OCPBUGS-59743: azure: reject data disks on Azure Stack Hub

[mfbonfigli]

Summary

Data disks are not currently supported or tested on Azure Stack Hub. When configured, the MAPI machineset spec serializes a zero-value managedDisk struct with an empty storageAccountType, which Azure rejects at provisioning time.

Rather than failing at provisioning time, this PR adds install-config validation to reject dataDisks on Azure Stack Hub upfront with a clear error message, consistent with how other unsupported features (security profiles, managed boot diagnostics) are already blocked on that platform.

Changes

• pkg/types/azure/validation/machinepool.go: Add field.Forbidden guard for dataDisks when platform.CloudName == azure.StackCloud
• pkg/types/azure/validation/machinepool_test.go: Add test case verifying data disks on Azure Stack Hub are rejected

Summary by CodeRabbit

• Bug Fixes
  • Azure Stack now clearly rejects machine pool configurations that include data disks.
  • Other Azure cloud environments continue to validate data disks as before.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@mfbonfigli: This pull request references Jira Issue OCPBUGS-59743, which is invalid:

• expected the bug to target either version "5.1.0." or "openshift-5.1.0.", but it targets "4.21" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

Data disks are not currently supported or tested on Azure Stack Hub. When configured, the MAPI machineset spec serializes a zero-value managedDisk struct with an empty storageAccountType, which Azure rejects at provisioning time.

Rather than failing at provisioning time, this PR adds install-config validation to reject dataDisks on Azure Stack Hub upfront with a clear error message, consistent with how other unsupported features (security profiles, managed boot diagnostics) are already blocked on that platform.

Changes

• pkg/types/azure/validation/machinepool.go: Add field.Forbidden guard for dataDisks when platform.CloudName == azure.StackCloud
• pkg/types/azure/validation/machinepool_test.go: Add test case verifying data disks on Azure Stack Hub are rejected

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 3f815d67-6741-4f9d-bbdc-972015056132

📥 Commits

Reviewing files that changed from the base of the PR and between 12787b2 and 9a4611f.

📒 Files selected for processing (2)

• pkg/types/azure/validation/machinepool.go
• pkg/types/azure/validation/machinepool_test.go

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

Azure Stack machine pool validation now rejects configured data disks with a forbidden error. Other Azure cloud environments retain the existing data-disk validation path. A test covers the Azure Stack restriction and expected error.

Changes

Azure Stack data disk validation

Layer / File(s)	Summary
Reject data disks on Azure Stack pkg/types/azure/validation/machinepool.go, pkg/types/azure/validation/machinepool_test.go	ValidateMachinePool returns a forbidden error for data disks on Azure Stack. Other cloud environments continue to call validateDataDisk. The test verifies the Azure Stack error.
Estimated code review effort: 2 (Simple)	~10 minutes

Merge Risk: ⚪ Minimal · up to 9a461

The change rejects unsupported data disks on Azure Stack Hub during validation instead of allowing a later provisioning failure; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: rejecting data disks on Azure Stack Hub.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PASS. The pull request adds a Go subtest named "data disks on Azure Stack Hub are rejected". The title is a stable descriptive string. It contains no generated identifier, timestamp, node name, namesp…
Test Structure And Quality	✅ Passed	PASS. The added case is a single table-driven unit-test scenario for Azure Stack data-disk rejection. It creates no cluster resources, so setup/cleanup and wait-timeout requirements do not apply. The …
Microshift Test Compatibility	✅ Passed	PASS: The pull request adds a standard Go unit-test case in TestValidateMachinePool, not a Ginkgo e2e test. The changed test uses Azure validation structs and testify, and it references no MicroSh…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The pull request adds a standard Go table-test case inside TestValidateMachinePool, using testing.T and testify/assert. It does not add a Ginkgo It, Describe, Context, or When e2e …
Topology-Aware Scheduling Compatibility	✅ Passed	PASS — The change modifies only Azure machine-pool validation and its unit test. It adds no deployment manifest, operator/controller logic, replica setting, affinity, topology spread, node selector, t…
Ote Binary Stdout Contract	✅ Passed	PASS: The pull request changes only Azure validation logic and a unit-test case. The added code uses fmt.Sprintf to build a validation error and does not write to stdout, configure logging, or alter…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PASS. The pull request adds a standard Go TestValidateMachinePool table case, not a new Ginkgo e2e test. The test uses only in-memory Azure machine-pool data and validation; it has no IPv4 assumptio…
No-Weak-Crypto	✅ Passed	PASS: The pull request only adds an Azure Stack dataDisks validation guard and its test. The changed lines introduce no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB mode, custom cryptography, or secret/…
Container-Privileges	✅ Passed	PASS: The pull request changes only Azure validation Go code and its Go test. The diff adds a field.Forbidden error for Azure Stack data disks and adds test data. It introduces no container or Kuber…
No-Sensitive-Data-In-Logs	✅ Passed	PASS: The commit changes only Azure validation and its test. The added code returns a constant field.Forbidden message using azure.StackCloud; it adds no logging, logger calls, or sensitive-data o…

Full details: Stable And Deterministic Test Names

Explanation

PASS. The pull request adds a Go subtest named "data disks on Azure Stack Hub are rejected". The title is a stable descriptive string. It contains no generated identifier, timestamp, node name, namespace, IP address, or runtime value. The affected file uses testing.T subtests and contains no Ginkgo It, Describe, Context, or When declarations.

Full details: Test Structure And Quality

Explanation

PASS. The added case is a single table-driven unit-test scenario for Azure Stack data-disk rejection. It creates no cluster resources, so setup/cleanup and wait-timeout requirements do not apply. The test uses the existing package pattern: t.Run with assert.Regexp against ValidateMachinePool; no Ginkgo, Eventually, or Consistently code is present in this package. The subtest name and assertion output identify the expected error and scenario.

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request adds a standard Go unit-test case in TestValidateMachinePool, not a Ginkgo e2e test. The changed test uses Azure validation structs and testify, and it references no MicroShift-unavailable OpenShift API, namespace, or unsupported cluster feature.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request adds a standard Go table-test case inside TestValidateMachinePool, using testing.T and testify/assert. It does not add a Ginkgo It, Describe, Context, or When e2e test. Therefore, the SNO multi-node compatibility check does not apply.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS — The change modifies only Azure machine-pool validation and its unit test. It adds no deployment manifest, operator/controller logic, replica setting, affinity, topology spread, node selector, toleration, or PDB. The test's Name: "master" is a machine-pool fixture and does not add a scheduling constraint.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The pull request changes only Azure validation logic and a unit-test case. The added code uses fmt.Sprintf to build a validation error and does not write to stdout, configure logging, or alter main, init, TestMain, or suite setup. No explicit OTE stdout-contract failure was introduced.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS. The pull request adds a standard Go TestValidateMachinePool table case, not a new Ginkgo e2e test. The test uses only in-memory Azure machine-pool data and validation; it has no IPv4 assumptions or external connectivity requirements. The committed diff contains no hardcoded IPs, URLs, network objects, or public service access.

Full details: No-Weak-Crypto

Explanation

PASS: The pull request only adds an Azure Stack dataDisks validation guard and its test. The changed lines introduce no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB mode, custom cryptography, or secret/token comparisons. The only des search hit is the pre-existing diskEncryptionSets test path, not cryptographic implementation usage.

Full details: Container-Privileges

Explanation

PASS: The pull request changes only Azure validation Go code and its Go test. The diff adds a field.Forbidden error for Azure Stack data disks and adds test data. It introduces no container or Kubernetes manifest settings, and no added lines contain privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or root execution settings.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS: The commit changes only Azure validation and its test. The added code returns a constant field.Forbidden message using azure.StackCloud; it adds no logging, logger calls, or sensitive-data output. The added test contains only static configuration values and no passwords, tokens, API keys, PII, session IDs, hostnames, or customer data.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

---

_{Comment @coderabbitai help to get the list of available commands.}

[mfbonfigli]

/jira refresh

[openshift-ci-robot]

@mfbonfigli: This pull request references Jira Issue OCPBUGS-59743, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.1.0) matches configured target version for branch (5.1.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jinyunma

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: jinyunma.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

@mfbonfigli: This pull request references Jira Issue OCPBUGS-59743, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.1.0) matches configured target version for branch (5.1.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jinyunma

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[mfbonfigli]

/test golint

[mfbonfigli]

/test e2e-azure-ovn
/test e2e-azure-ovn-multidisk-techpreview

[mfbonfigli]

/test images

[vr4manta]

/lgtm

[openshift-merge-bot]

Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.

[openshift-ci]

@mfbonfigli: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/azure-private	9a4611f	link	false	/test azure-private
ci/prow/e2e-azure-ovn-multidisk-techpreview	9a4611f	link	false	/test e2e-azure-ovn-multidisk-techpreview

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[mtulio]

/lgtm
/cc @tthvo

[tthvo]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tthvo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• pkg/types/azure/OWNERS [tthvo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sadasu]

/pipeline required

[openshift-merge-bot]

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-azure-default-config
/test e2e-azure-ovn
/test e2e-azure-ovn-shared-vpc
/test e2e-azurestack
/test e2e-aws-ovn