ARO-24514: Make identityRef optional in AROControlPlane for ASO credential-based authentication and make AROControlPlane identityRef independent#79
Merged
mzazrivec merged 1 commit intostolostron:backplane-2.17from Feb 27, 2026
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: marek-veber The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
04c7bf9 to
5aa47df
Compare
e14357b to
6066418
Compare
cfdf1ea to
2c05a35
Compare
…hinePool This change makes spec.identityRef optional for both AROControlPlane and AROMachinePool to support ASO credential-based authentication, and refactors AROMachinePool to eliminate the need for Azure SDK credentials entirely. Changes: 1. AROControlPlane: - Made spec.identityRef optional in API types - Added webhook validation to ensure identityRef is set when not using ASO - Updated CRD with optional identityRef field - Updated scope to handle missing identityRef gracefully 2. AROMachinePool: - Removed Azure SDK dependencies and credential initialization - Removed AzureClients embedding from AROMachinePoolScope - Removed CredentialCache parameter from reconciler - Replaced Azure VM API calls with workload cluster node listing - Added ClusterTracker support for accessing workload cluster clients - Updated tests to use FakeClusterTracker mock instead of CredentialCache 3. main.go: - Created newWorkloadClusterCache() helper function to share cluster cache - Added shared workload cluster cache for both ASOAPI and ARO features - Ensures only one cluster cache is created regardless of feature gate order - Prevents "controller with name clustercache already exists" error 4. Documentation: - Updated ARO HCP proposal with implementation details - Documented new node listing approach for AROMachinePool - Added implementation history entries Rationale: - ASO resources use serviceoperator.azure.com/credential-from annotations for authentication, making identityRef redundant when using ASO mode - AROMachinePool no longer needs Azure credentials as it gets providerIDList from workload cluster nodes instead of calling Azure VM API - This simplifies the authentication model and aligns with ASO patterns - Follows the same pattern as ASOManagedMachinePool for node discovery
2c05a35 to
98893b1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements ARO-24514 to make
identityRefoptional in AROControlPlane and AROMachinePool, enabling customers to use ASO credential-based authentication without managing separate AzureClusterIdentity resources.Additionally, AROMachinePool has been completely refactored to remove Azure SDK dependencies - it now populates
providerIDListby reading nodes from the workload cluster instead of calling Azure VM API, making it fully ASO-native and eliminating the need for Azure credentials entirely.Problem
Currently, AROControlPlane and AROMachinePool require
identityRefto initialize Azure credentials. For AROControlPlane, this is needed for Key Vault operations (encryption key creation and version propagation). For AROMachinePool, credentials were required to call Azure VM API to populate providerIDList, even though all other Azure operations are handled by ASO. Customers like Adobe who want to use ASO'sserviceoperator.azure.com/credential-fromannotations are forced to maintain separate AzureClusterIdentity resources, adding unnecessary complexity.When using ASO resources with credential annotations, customers want to avoid managing identityRef separately while maintaining full control over Key Vault and encryption key management.
Solution
Core Changes
Skip Azure operations when identityRef is not set
activeKey.versionin HcpOpenShiftCluster specAROMachinePool refactored to use workload cluster nodes
Two-layer validation for encryption key version
Enhanced documentation
Authentication Modes
kms.activeKey.versionkms.activeKey.versionChanges Made
Code Changes
Controller Logic (
exp/controllers/arocontrolplane_reconciler.go)spec.properties.etcd.dataEncryption.customerManaged.kms.activeKey.versionwhen encryption configuredEncryptionKeyReadyConditionappropriately for each mode:Truewhen identityRef is set and key is readyUnknownwhen identityRef is not set (manual key management)Falsewhen validation failsWebhook Validation (
exp/api/controlplane/v1beta2/arocontrolplane_webhook.go)validateEncryptionKeyVersion()functionvalidateResources()to checkkms.activeKey.versionwhen identityRef is nilspec.properties.etcd.dataEncryption.customerManaged.kms.activeKey.versionScope Initialization
azure/scope/arocontrolplane.go)azure/scope/aromachinepool.go)identityRefor Azure SDK accessAROMachinePool Reconciler (
exp/controllers/aromachinepool_reconciler.go)tracker.GetClient()and filters by node pool name pattern:<cluster-name>-<nodepool-name>-<suffix>CredentialCache, addedClusterTrackerMain Controller Setup (
main.go)newWorkloadClusterCache()helper function to prevent duplicate cluster cache creationnewWorkloadClusterCache()with nil checkAROMachinePool Tests (
exp/controllers/aromachinepool_controller_test.go)FakeClusterTrackermock implementingClusterTrackerinterfaceCredentialCachewithClusterTrackerin all test setupClusterTrackerinterfaceKey Vault Service (
azure/services/keyvaults/keyvault.go)API Types (
exp/api/controlplane/v1beta2/arocontrolplane_types.go)Documentation Changes
ARO HCP Proposal (
docs/proposals/20250425-aro-hcp.md)CRD Manifest (
config/crd/bases/controlplane.cluster.x-k8s.io_arocontrolplanes.yaml)AROMachinePool Implementation Details
Workload Cluster Node Listing
AROMachinePool now populates
providerIDListby reading nodes directly from the workload cluster instead of calling Azure VM API:ClusterTracker.GetClient(ctx, clusterKey)client.List(ctx, &corev1.NodeList{})<cluster-name>-<nodepool-name>-<random-suffix>node.Spec.ProviderIDBenefits
Node Pool Name Pattern
ARO HCP node names follow the pattern:
{clusterName}-{nodePoolName}-{randomSuffix}Example:
my-clusterworkersmy-cluster-workers-abc123,my-cluster-workers-def456The reconciler filters nodes by checking if the node name contains the pattern
my-cluster-workers-.Validation Logic
Webhook Validation (Layer 1)
Runtime Validation (Layer 2)
Example Configuration
With identityRef (existing behavior)
Without identityRef (new capability)
Testing
AROControlPlane Tests
activeKey.versionwhen identityRef is nilactiveKey.versionis missingactiveKey.versionis manually specifiedAROMachinePool Tests
Integration Tests
make lintpassesArchitecture Changes
Before
After
Breaking Changes
None. This change is backward compatible:
identityRefcontinue to work unchangedidentityRefremains optional (was already optional in schema)identityRef(no Azure credentials needed either way)Files Changed Summary
exp/api/controlplane/v1beta2/arocontrolplane_types.goexp/api/controlplane/v1beta2/arocontrolplane_webhook.goexp/controllers/arocontrolplane_reconciler.goazure/scope/arocontrolplane.goazure/scope/aromachinepool.goexp/controllers/aromachinepool_reconciler.goexp/controllers/aromachinepool_controller.goexp/controllers/aromachinepool_controller_test.gomain.godocs/proposals/20250425-aro-hcp.mdconfig/crd/bases/*.yamlRelated Issues