OCPBUGS-59521: azure: fixes when setting DiskEncryptionSet on data disks by redhat-chai-bot · Pull Request #10776 · openshift/installer

redhat-chai-bot · 2026-08-19T19:32:15Z

Summary

Fixes a nil pointer dereference panic in pkg/asset/machines/azure/machines.go when configuring dataDisks.managedDisk.diskEncryptionSet on Azure.

Bug

OCPBUGS-59521

When a data disk is configured with a DiskEncryptionSet but without a SecurityProfile, the installer panics with a nil pointer dereference. The code incorrectly accesses disk.ManagedDisk.SecurityProfile.DiskEncryptionSet instead of using the correct path disk.ManagedDisk.DiskEncryptionSet.

Changes

pkg/asset/machines/azure/machines.go:

Replace the incorrect SecurityProfile.DiskEncryptionSet access with direct disk.ManagedDisk.DiskEncryptionSet.ID access
Add validation to return an error when the encryption set ID is empty

pkg/asset/machines/azure/machines_test.go:

Add TestProviderDataDiskEncryptionSet with 4 table-driven subtests:
- Data disk with valid DiskEncryptionSet (verifies ID is correctly passed through)
- Data disk with DiskEncryptionSet but empty ID (verifies error returned)
- Data disk without DiskEncryptionSet (verifies nil encryption set on output)
- Data disk with DiskEncryptionSet but nil SecurityProfile (the actual panic scenario)

Testing

All 24 tests in pkg/asset/machines/azure/ pass (20 existing + 4 new).

This change is isolated from the bundle PR #9947 which addresses multiple bugs.

AI-generated. Review for accuracy.

@mtulio requested in Slack thread

Summary by CodeRabbit

Bug Fixes
- Corrected handling of Azure data-disk encryption sets so valid encryption set IDs are preserved.
- Added clear validation errors when a data disk has an empty encryption set ID.
- Improved resilience when encryption metadata is incomplete, preventing unexpected failures.

…t on Azure The data disk encryption set code in provider() incorrectly accessed disk.ManagedDisk.SecurityProfile.DiskEncryptionSet, which panics when SecurityProfile is nil. The correct access path is disk.ManagedDisk.DiskEncryptionSet. This change also adds validation for an empty encryption set ID. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

openshift-merge-bot · 2026-08-19T19:32:18Z

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

coderabbitai · 2026-08-19T19:32:41Z

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: acd2aa2f-fc34-44ed-babf-396571bbf4ff

📥 Commits

Reviewing files that changed from the base of the PR and between b1249b5 and 5082f88.

📒 Files selected for processing (2)

pkg/asset/machines/azure/machines.go
pkg/asset/machines/azure/machines_test.go

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

📝 Walkthrough

Walkthrough

Changes

Azure data disk encryption

Layer / File(s)	Summary
Validate and map encryption set IDs `pkg/asset/machines/azure/machines.go`	The provider reads the encryption set ID from `ManagedDisk.DiskEncryptionSet`. It returns an error for an empty ID and identifies the affected data disk.
Cover data disk encryption set cases `pkg/asset/machines/azure/machines_test.go`	Tests cover valid, empty, missing, and nil-`SecurityProfile` encryption set values. Successful cases verify provider spec fields.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 5082f

This localized change prevents the Azure installer panic for data-disk encryption sets and rejects invalid empty encryption-set IDs, with focused tests covering the affected cases. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
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	The PR adds Go t.Run cases with literal, stable names; no Ginkgo title constructs or dynamic values appear in the changed test.
Test Structure And Quality	✅ Passed	The PR adds standard Go table tests, not Ginkgo It blocks. They create no cluster resources and use no waits; each subtest covers one disk-encryption behavior and follows package patterns.
Microshift Test Compatibility	✅ Passed	The added test is a standard Go testing.TestProviderDataDiskEncryptionSet unit test, not a Ginkgo e2e test, and it does not reference MicroShift-unavailable APIs or namespaces.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR adds a standard Go `TestProviderDataDiskEncryptionSet` unit test, not a Ginkgo e2e test; it contains no SNO-relevant multi-node assumptions.
Topology-Aware Scheduling Compatibility	✅ Passed	The diff changes only Azure machine asset conversion and tests; it adds no manifests, controllers, replicas, affinity, topology spread, node selectors, tolerations, or PDBs.
Ote Binary Stdout Contract	✅ Passed	The commit changes only Azure provider logic and a TestProviderDataDiskEncryptionSet test; no main/init/TestMain or suite setup stdout writes were added.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The diff adds a standard Go Test with t.Run, not Ginkgo e2e tests; it uses only local structs and provider calls, with no IPv4 assumptions or external connectivity.
No-Weak-Crypto	✅ Passed	The diff only maps Azure DiskEncryptionSet IDs and adds tests; it introduces no MD5, SHA1, DES/RC4/Blowfish/ECB, custom crypto, or secret comparisons.
Container-Privileges	✅ Passed	The commit changes Azure disk-encryption-set handling and tests only. It adds no container manifest or privileged, host namespace, SYS_ADMIN, root, or privilege-escalation setting.
No-Sensitive-Data-In-Logs	✅ Passed	The commit adds no logging calls or sensitive fields to logs. It only returns a validation error with the data-disk name; test resource IDs are not logged.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the Azure DiskEncryptionSet fix for data disks and includes the relevant issue identifier.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

openshift-ci · 2026-08-19T19:32:48Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sadasu for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

pkg/asset/machines/azure/OWNERS

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

mtulio · 2026-08-19T19:35:35Z

/assign

mtulio · 2026-08-19T19:35:53Z

/test e2e-azure-ovn

mtulio · 2026-08-19T19:47:39Z

/retitle OCPBUGS-59521: azure: fixes when setting DiskEncryptionSet on data disks

openshift-ci-robot · 2026-08-19T19:47:51Z

@redhat-chai-bot: This pull request references Jira Issue OCPBUGS-59521, 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

Fixes a nil pointer dereference panic in pkg/asset/machines/azure/machines.go when configuring dataDisks.managedDisk.diskEncryptionSet on Azure.

Bug

OCPBUGS-59521

When a data disk is configured with a DiskEncryptionSet but without a SecurityProfile, the installer panics with a nil pointer dereference. The code incorrectly accesses disk.ManagedDisk.SecurityProfile.DiskEncryptionSet instead of using the correct path disk.ManagedDisk.DiskEncryptionSet.

Changes

pkg/asset/machines/azure/machines.go:

Replace the incorrect SecurityProfile.DiskEncryptionSet access with direct disk.ManagedDisk.DiskEncryptionSet.ID access

Add validation to return an error when the encryption set ID is empty

pkg/asset/machines/azure/machines_test.go:

Add TestProviderDataDiskEncryptionSet with 4 table-driven subtests:

Data disk with valid DiskEncryptionSet (verifies ID is correctly passed through)

Data disk with DiskEncryptionSet but empty ID (verifies error returned)

Data disk without DiskEncryptionSet (verifies nil encryption set on output)

Data disk with DiskEncryptionSet but nil SecurityProfile (the actual panic scenario)

Testing

All 24 tests in pkg/asset/machines/azure/ pass (20 existing + 4 new).

This change is isolated from the bundle PR #9947 which addresses multiple bugs.

AI-generated. Review for accuracy.

@mtulio requested in Slack thread

Summary by CodeRabbit

Bug Fixes

Corrected handling of Azure data-disk encryption sets so valid encryption set IDs are preserved.

Added clear validation errors when a data disk has an empty encryption set ID.

Improved resilience when encryption metadata is incomplete, preventing unexpected failures.

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.

mtulio · 2026-08-19T21:00:12Z

/test e2e-azure-ovn-multidisk-techpreview

openshift-ci · 2026-08-20T00:52:22Z

@redhat-chai-bot: The following test 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/e2e-azure-ovn-multidisk-techpreview	`5082f88`	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.

openshift-ci Bot requested review from jhixson74 and rna-afk August 19, 2026 19:32

openshift-ci Bot assigned mtulio Aug 19, 2026

openshift-ci Bot changed the title ~~Bug 59521: Fix nil pointer panic when setting data disk encryption set on Azure~~ OCPBUGS-59521: azure: fixes when setting DiskEncryptionSet on data disks Aug 19, 2026

redhat-chai-bot mentioned this pull request Aug 21, 2026

OCPBUGS-59522: azure: reject securityEncryptionType on data disks #10777

Open

mtulio mentioned this pull request Aug 21, 2026

OCPBUGS-61892, OCPBUGS-59743, OCPBUGS-59520, OCPBUGS-63147: Resolve issues with azure data disk #9947

Open

Navigation Menu

Uh oh!

OCPBUGS-59521: azure: fixes when setting DiskEncryptionSet on data disks - #10776

OCPBUGS-59521: azure: fixes when setting DiskEncryptionSet on data disks#10776
redhat-chai-bot wants to merge 1 commit into
openshift:mainfrom
redhat-chai-bot:fix/OCPBUGS-59521-data-disk-encryption-set-panic

redhat-chai-bot commented Aug 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

openshift-merge-bot Bot commented Aug 19, 2026

Uh oh!

coderabbitai Bot commented Aug 19, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

openshift-ci Bot commented Aug 19, 2026

Uh oh!

mtulio commented Aug 19, 2026

Uh oh!

mtulio commented Aug 19, 2026

Uh oh!

mtulio commented Aug 19, 2026

Uh oh!

openshift-ci-robot commented Aug 19, 2026

Summary

Bug

Changes

Testing

Summary by CodeRabbit

Uh oh!

mtulio commented Aug 19, 2026

Uh oh!

openshift-ci Bot commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

redhat-chai-bot commented Aug 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bug

Changes

Testing

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Aug 19, 2026

Uh oh!

coderabbitai Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

openshift-ci Bot commented Aug 19, 2026

Uh oh!

mtulio commented Aug 19, 2026

Uh oh!

mtulio commented Aug 19, 2026

Uh oh!

mtulio commented Aug 19, 2026

Uh oh!

openshift-ci-robot commented Aug 19, 2026

Summary

Bug

Changes

Testing

Summary by CodeRabbit

Uh oh!

mtulio commented Aug 19, 2026

Uh oh!

openshift-ci Bot commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

redhat-chai-bot commented Aug 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 19, 2026 •

edited

Loading