From 8fbbbffa0a77dd26a4519e678fb9dcd95cb4b8cd Mon Sep 17 00:00:00 2001 From: Miguel Soriano Date: Tue, 28 Jul 2026 12:48:01 +0200 Subject: [PATCH] feat: add controller that calculates Cluster Data Plane Identities extra information We add a controller that retrieves the Client ID and Principal ID associated to the Data Plane operators identities associated to the ARO-HCP Cluster. We leverage the Service Managed Identity associated to the ARO-HCP Cluster to retrieve the Data Plane operators identities information. We use Azure Go SDK's UserAssignedIdentities API to retrieve it. This is a different method than what's done for MSI based identities where the Managed Identities Data Plane service is used instead. This is because for the MSI based identities, on the environments where the managed identities data plane service is not available, we use the mi mock identity instead, which includes its clientid+principalid instead of the ones associated to the identities passed in the cluster payload. By using the mock managed identities data plane client we retrieve that transparently. We do that also because that identity/information is the one that needs to be used by the control plane operators themselves on the control plane side. --- backend/pkg/app/backend.go | 9 + ...plane_operators_managed_identities_info.go | 303 ++++++++++++++++++ ..._operators_managed_identities_info_test.go | 227 +++++++++++++ .../coreapi/types_serviceprovider_cluster.go | 54 ++++ internal/api/coreapi/zz_generated.deepcopy.go | 66 ++++ 5 files changed, 659 insertions(+) create mode 100644 backend/pkg/controllers/cluster/identity/fetch_data_plane_operators_managed_identities_info.go create mode 100644 backend/pkg/controllers/cluster/identity/fetch_data_plane_operators_managed_identities_info_test.go diff --git a/backend/pkg/app/backend.go b/backend/pkg/app/backend.go index e85bbd3e29b..b2829e29b31 100644 --- a/backend/pkg/app/backend.go +++ b/backend/pkg/app/backend.go @@ -47,6 +47,7 @@ import ( credentialrevocationdeletion "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/credentialrevocation/deletion" credentialrevocationoperations "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/credentialrevocation/operations" clusterdeletion "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/deletion" + clusteridentity "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/identity" "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/legacycredentialrequest" clusteroperations "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/operations" clusterplacement "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/placement" @@ -1006,6 +1007,13 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, backendInformers, ) + fetchDataPlaneOperatorsManagedIdentitiesInfoController := clusteridentity.NewFetchDataPlaneOperatorsManagedIdentitiesInfoController( + b.clock, + b.options.ResourcesDBClient, + backendInformers, + b.options.SMIClientBuilder, + ) + leaderElectionConfig := leaderelection.LeaderElectionConfig{ Lock: b.options.LeaderElectionLock, LeaseDuration: sharedleaderelection.RecommendedLeaseDuration, @@ -1117,6 +1125,7 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, go cosmosMigrationController.Run(ctx, 5) go virtualMachineResourceSKUsCachedReaderController.Run(ctx, 20) go backupScheduleController.Run(ctx, 20) + go fetchDataPlaneOperatorsManagedIdentitiesInfoController.Run(ctx, 20) }, OnStoppedLeading: func() { // This needs to be defined even though it does nothing. diff --git a/backend/pkg/controllers/cluster/identity/fetch_data_plane_operators_managed_identities_info.go b/backend/pkg/controllers/cluster/identity/fetch_data_plane_operators_managed_identities_info.go new file mode 100644 index 00000000000..70213d5d910 --- /dev/null +++ b/backend/pkg/controllers/cluster/identity/fetch_data_plane_operators_managed_identities_info.go @@ -0,0 +1,303 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package identity + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + utilsclock "k8s.io/utils/clock" + + azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + + azureclient "github.com/Azure/ARO-HCP/backend/pkg/azure/client" + "github.com/Azure/ARO-HCP/backend/pkg/utils/controllerutils" + "github.com/Azure/ARO-HCP/internal/api/coreapi" + "github.com/Azure/ARO-HCP/internal/database/cosmosstorage/corecosmosstorage" + "github.com/Azure/ARO-HCP/internal/database/cosmosstorage/cosmosstorageutils" + "github.com/Azure/ARO-HCP/internal/database/informers/coreinformers" + "github.com/Azure/ARO-HCP/internal/utils" +) + +const ( + fetchDataPlaneOperatorsManagedIdentitiesInfoControllerName = "FetchDataPlaneOperatorsManagedIdentitiesInfo" + + // dataPlaneOperatorsManagedIdentitiesRecheckInterval is the base interval + // before re-querying Azure for ClientID/PrincipalID when the desired set of + // identities is already fully resolved. Combined with + // dataPlaneOperatorsManagedIdentitiesRecheckJitter via wait.Jitter. + dataPlaneOperatorsManagedIdentitiesRecheckInterval = 12 * time.Hour + dataPlaneOperatorsManagedIdentitiesRecheckJitter = 0.5 +) + +// fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer reconciles +// ServiceProviderCluster.Status.DataPlaneOperatorsManagedIdentities from the +// cluster's configured data plane operator managed identities. +type fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer struct { + clock utilsclock.PassiveClock + resourcesDBClient corecosmosstorage.ResourcesDBClient + + smiClientBuilder azureclient.ServiceManagedIdentityClientBuilder +} + +var _ controllerutils.ClusterSyncer = (*fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer)(nil) + +// NewFetchDataPlaneOperatorsManagedIdentitiesInfoController creates a cluster-watching +// controller that keeps ServiceProviderCluster.Status.DataPlaneOperatorsManagedIdentities +// in sync with the cluster's CustomerProperties data plane operator managed identities. +// +// On each sync it: +// 1. Reads every operator -> ResourceID entry from +// Cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators +// and deduplicates by lowercased ResourceID (multiple operators may share one +// identity). +// 2. Via needsWork, skips Azure calls when EarliestRecheckTime is still in the +// future AND the unique ResourceIDs stored on SPC still match that desired +// set. If the desired ResourceIDs have changed, EarliestRecheckTime is +// ignored so Azure is queried immediately. EarliestRecheckTime is shared +// across every entry in the Identities map. +// 3. Otherwise uses the cluster's Service Managed Identity to call Azure +// UserAssignedIdentitiesClient Get once per unique ResourceID and resolve +// ClientID and PrincipalID. +// 4. Rebuilds Status.DataPlaneOperatorsManagedIdentities.Identities as a full +// desired map keyed by lowercased ResourceID (ResourceID, ClientID, +// PrincipalID). Entries that are no longer present on the cluster are pruned. +// Every desired ResourceID is written into the map: +// - ParseResourceID of a set key failing returns immediately without writing. +// That cannot happen for keys produced from ResourceID.String(). +// - ResourceNotFound keeps the entry and sets ClientID and PrincipalID to +// nil, so the SPC still lists the customer-configured identity while +// signaling that Azure does not currently have it. +// - Any other Get failure is accumulated and processing continues. The entry +// keeps any previously resolved ClientID/PrincipalID from the existing SPC +// when present, otherwise leaves them unset. A successful Get with nil +// Properties fails the whole sync immediately without writing. +// - Otherwise ClientID and PrincipalID are written as returned by Azure, +// including nil or empty values. +// 5. After every identity is processed without a failing Get, sets +// EarliestRecheckTime on the in-memory replacement to now plus a jittered +// interval (including when some identities were ResourceNotFound). When any +// Get failures were accumulated, the existing EarliestRecheckTime is left +// unchanged (nil or already past) and the accumulated error is returned so +// the workqueue retries. +// 6. Writes the ServiceProviderCluster when the desired status differs, then +// returns any accumulated Get errors. needsWork observes EarliestRecheckTime +// and the desired-vs-stored ResourceID match from Cosmos, so a wait is +// introduced only after a successful Replace persists a matching set with a +// future EarliestRecheckTime. If Replace fails (or hits a precondition +// failure), the new EarliestRecheckTime is not stored; the workqueue requeues +// and the next needsWork still sees the previously persisted value (typically +// nil or already past, or a mismatched identity set), so the controller does +// not wait out the recheck interval after write failures either. +func NewFetchDataPlaneOperatorsManagedIdentitiesInfoController( + clock utilsclock.PassiveClock, + resourcesDBClient corecosmosstorage.ResourcesDBClient, + backendInformers coreinformers.BackendInformers, + smiClientBuilder azureclient.ServiceManagedIdentityClientBuilder, +) controllerutils.Controller { + if clock == nil { + clock = utilsclock.RealClock{} + } + + syncer := &fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer{ + clock: clock, + resourcesDBClient: resourcesDBClient, + smiClientBuilder: smiClientBuilder, + } + + controller := controllerutils.NewClusterWatchingController( + fetchDataPlaneOperatorsManagedIdentitiesInfoControllerName, + resourcesDBClient, + backendInformers, + nil, + 1*time.Minute, + syncer, + ) + + return controller +} + +// needsWork reports whether Azure should be queried for data plane operator +// managed identity metadata. desiredDataPlaneOperatorIdentities must already be +// the unique lowercased ResourceID set from CustomerProperties. EarliestRecheckTime +// is honored only when those ResourceIDs still match SPC; on mismatch it returns +// true immediately. When identities match, it returns false while +// EarliestRecheckTime is in the future, and true when EarliestRecheckTime is +// nil or already past. Callers must skip needsWork entirely when the cluster +// is deleting. +func (c *fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer) needsWork(spc *coreapi.ServiceProviderCluster, desiredDataPlaneOperatorsResourceIDStrs map[string]struct{}) bool { + // Only honor EarliestRecheckTime when the desired identity set still matches + // SPC. Any mismatch should fall through to return true and query Azure. + if c.desiredDataPlaneOperatorResourceIDsMatchSPC(desiredDataPlaneOperatorsResourceIDStrs, spc) { + earliestRecheckTime := spc.Status.DataPlaneOperatorsManagedIdentities.EarliestRecheckTime + if earliestRecheckTime != nil && c.clock.Now().Before(earliestRecheckTime.Time) { + return false + } + } + + return true +} + +func (c *fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer) SyncOnce(ctx context.Context, key controllerutils.HCPClusterKey) error { + existingCluster, err := c.resourcesDBClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName).Get(ctx, key.HCPClusterName) + if cosmosstorageutils.IsNotFoundError(err) { + return nil // cluster doesn't exist, no work to do + } + if err != nil { + return utils.TrackError(fmt.Errorf("failed to get Cluster: %w", err)) + } + + if existingCluster.ServiceProviderProperties.DeletionTimestamp != nil { + return nil + } + + existingServiceProviderCluster, err := corecosmosstorage.GetOrCreateServiceProviderCluster(ctx, c.resourcesDBClient, key.GetResourceID()) + if err != nil { + return utils.TrackError(fmt.Errorf("failed to get or create ServiceProviderCluster: %w", err)) + } + + desiredDataPlaneOperators := existingCluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators + identitiesToSync := c.uniqueDataPlaneOperatorResourceIDs(desiredDataPlaneOperators) + if identitiesToSync == nil { + return utils.TrackError(fmt.Errorf("data plane operator managed identity ResourceID is nil")) + } + if !c.needsWork(existingServiceProviderCluster, identitiesToSync) { + return nil + } + + replacement := existingServiceProviderCluster.DeepCopy() + replacement.Status.DataPlaneOperatorsManagedIdentities = coreapi.ServiceProviderClusterDataPlaneOperatorsManagedIdentities{ + Identities: make(map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity, len(identitiesToSync)), + EarliestRecheckTime: existingServiceProviderCluster.Status.DataPlaneOperatorsManagedIdentities.EarliestRecheckTime.DeepCopy(), + } + + smiResourceID := existingCluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ServiceManagedIdentity + userAssignedIdentitiesClient, err := c.smiClientBuilder.UserAssignedIdentitiesClient(ctx, existingCluster.ServiceProviderProperties.ManagedIdentitiesDataPlaneIdentityURL, smiResourceID, existingCluster.ID.SubscriptionID) + if err != nil { + return utils.TrackError(fmt.Errorf("failed to get User Assigned Identities Client: %w", err)) + } + + errs := []error{} + for identityResourceIDStr := range identitiesToSync { + resourceID, err := azcorearm.ParseResourceID(identityResourceIDStr) + if err != nil { + // We should never get a nil ResourceID from uniqueDataPlaneOperatorResourceIDs because it's built from + // the Cluster's customer properties which should have been validated beforehand. Because of this, we return an error instead of accumulating. + return utils.TrackError(fmt.Errorf("failed to parse Data Plane Operator Managed Identity ResourceID %s: %w", identityResourceIDStr, err)) + } + + replacementIdentity := &coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity{ + ResourceID: resourceID, + } + replacement.Status.DataPlaneOperatorsManagedIdentities.Identities[identityResourceIDStr] = replacementIdentity + + currentMI, err := userAssignedIdentitiesClient.Get(ctx, resourceID.ResourceGroupName, resourceID.Name, nil) + if azureclient.IsResourceNotFoundErr(err) { + // If the identity is not found, we still keep the identity in the resource but we set the ClientID and PrincipalID to nil. In this way, we keep + // the same set of data plane operator managed identities resource IDs in the customer properties but we signal that the identity is missing + // by setting the ClientID and PrincipalID to nil. + replacementIdentity.ClientID = nil + replacementIdentity.PrincipalID = nil + continue + } + if err != nil { + // Accumulate Get failures and keep going so successfully resolved identities + // can still be persisted. Preserve any previously resolved ClientID/PrincipalID + // so a transient failure does not wipe known values. + if existingIdentity := existingServiceProviderCluster.Status.DataPlaneOperatorsManagedIdentities.Identities[identityResourceIDStr]; existingIdentity != nil { + replacementIdentity.ClientID = existingIdentity.ClientID + replacementIdentity.PrincipalID = existingIdentity.PrincipalID + } + errs = append(errs, utils.TrackError(fmt.Errorf("failed to get Data Plane Operator Managed Identity %s: %w", identityResourceIDStr, err))) + continue + } + + if currentMI.Properties == nil { + // The identity should always have properties. If it doesn't, we return an error instead of accumulating it, as this is unexpected and should not happen. + return utils.TrackError(fmt.Errorf("unexpected Data Plane Operator Managed Identity %s Properties is nil", identityResourceIDStr)) + } + + // For ClientID and PrincipalID of the identity, we set the value returned from the Azure API as is. This includes the cases where the + // value is nil or empty. + replacementIdentity.ClientID = currentMI.Properties.ClientID + replacementIdentity.PrincipalID = currentMI.Properties.PrincipalID + } + + if len(errs) == 0 { + // Set an earliest recheck time for the controller so we do not hit the Azure API too often. + // The value below is only honored once Replace persists it. A Replace failure leaves Cosmos unchanged, so needsWork will still see the + // previously persisted value (if any). + // On Get failures we skip this and keep the DeepCopied existing EarliestRecheckTime + // (nil or already past), then return the accumulated error so the workqueue retries. + // needsWork ignores EarliestRecheckTime when desired DataPlaneOperators ResourceIDs + // no longer match SPC, so identity replacement is detected without waiting out the gate. + recheckAt := metav1.NewTime(c.clock.Now().Add(wait.Jitter( + dataPlaneOperatorsManagedIdentitiesRecheckInterval, + dataPlaneOperatorsManagedIdentitiesRecheckJitter, + ))) + replacement.Status.DataPlaneOperatorsManagedIdentities.EarliestRecheckTime = &recheckAt + } + + if !equality.Semantic.DeepEqual(replacement.Status.DataPlaneOperatorsManagedIdentities, existingServiceProviderCluster.Status.DataPlaneOperatorsManagedIdentities) { + _, err = c.resourcesDBClient.ServiceProviderClusters(existingCluster.ID.SubscriptionID, existingCluster.ID.ResourceGroupName, existingCluster.ID.Name).Replace(ctx, replacement, nil) + if cosmosstorageutils.IsPreconditionFailedError(err) { + // Status (including any new DataPlaneOperatorsManagedIdentitiesEarliestRecheckTime) was not written. + // needsWork will still see the previously persisted value. + return errors.Join(errs...) + } + if err != nil { + // Same as precondition failure: DataPlaneOperatorsManagedIdentitiesEarliestRecheckTime was not + // persisted, so needsWork will still see the previously persisted value. + return errors.Join(append(errs, utils.TrackError(fmt.Errorf("failed to replace ServiceProviderCluster: %w", err)))...) + } + } + + return errors.Join(errs...) +} + +// uniqueDataPlaneOperatorResourceIDs returns the unique lowercased ResourceID +// strings from desiredDataPlaneOperators. It returns nil if any ResourceID is nil. +func (c *fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer) uniqueDataPlaneOperatorResourceIDs(desiredDataPlaneOperators map[string]*azcorearm.ResourceID) map[string]struct{} { + unique := make(map[string]struct{}, len(desiredDataPlaneOperators)) + for _, resourceID := range desiredDataPlaneOperators { + unique[strings.ToLower(resourceID.String())] = struct{}{} + } + return unique +} + +// desiredDataPlaneOperatorResourceIDsMatchSPC reports whether the unique data +// plane operator managed identity ResourceIDs stored on SPC match +// desiredDataPlaneOperatorIdentities. desiredDataPlaneOperatorIdentities must +// already be keyed by lowercased ResourceID. Comparison is by ResourceID +// presence only; ClientID/PrincipalID are ignored. +func (c *fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer) desiredDataPlaneOperatorResourceIDsMatchSPC(desiredDataPlaneOperatorsResourceIDStrs map[string]struct{}, spc *coreapi.ServiceProviderCluster) bool { + spcIdentities := spc.Status.DataPlaneOperatorsManagedIdentities.Identities + if len(desiredDataPlaneOperatorsResourceIDStrs) != len(spcIdentities) { + return false + } + + for resourceIDKey := range desiredDataPlaneOperatorsResourceIDStrs { + if _, ok := spcIdentities[resourceIDKey]; !ok { + return false + } + } + + return true +} diff --git a/backend/pkg/controllers/cluster/identity/fetch_data_plane_operators_managed_identities_info_test.go b/backend/pkg/controllers/cluster/identity/fetch_data_plane_operators_managed_identities_info_test.go new file mode 100644 index 00000000000..b2fdf20a99d --- /dev/null +++ b/backend/pkg/controllers/cluster/identity/fetch_data_plane_operators_managed_identities_info_test.go @@ -0,0 +1,227 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package identity + +import ( + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clocktesting "k8s.io/utils/clock/testing" + + azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + + "github.com/Azure/ARO-HCP/internal/api/coreapi" + "github.com/Azure/ARO-HCP/internal/api/metadataapi" +) + +func TestDesiredDataPlaneOperatorResourceIDsMatchSPC(t *testing.T) { + t.Parallel() + + identityA := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/identity-a")) + identityB := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/identity-b")) + mixedCaseIdentityA := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/Test-RG/providers/Microsoft.ManagedIdentity/userAssignedIdentities/Identity-A")) + + testCases := []struct { + name string + desiredResourceIDs map[string]struct{} + spcIdentities map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity + expectedMatch bool + }{ + { + name: "both empty match", + desiredResourceIDs: map[string]struct{}{}, + spcIdentities: map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity{}, + expectedMatch: true, + }, + { + name: "matching resource ID", + desiredResourceIDs: map[string]struct{}{ + strings.ToLower(identityA.String()): {}, + }, + spcIdentities: map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity{ + strings.ToLower(identityA.String()): { + ResourceID: identityA, + }, + }, + expectedMatch: true, + }, + { + name: "matching ignores resource ID casing when already lowercased as key", + desiredResourceIDs: map[string]struct{}{ + strings.ToLower(mixedCaseIdentityA.String()): {}, + }, + spcIdentities: map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity{ + strings.ToLower(identityA.String()): { + ResourceID: identityA, + }, + }, + expectedMatch: true, + }, + { + name: "unique identity count mismatch", + desiredResourceIDs: map[string]struct{}{ + strings.ToLower(identityA.String()): {}, + }, + spcIdentities: map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity{ + strings.ToLower(identityA.String()): { + ResourceID: identityA, + }, + strings.ToLower(identityB.String()): { + ResourceID: identityB, + }, + }, + expectedMatch: false, + }, + { + name: "resource ID mismatch", + desiredResourceIDs: map[string]struct{}{ + strings.ToLower(identityA.String()): {}, + }, + spcIdentities: map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity{ + strings.ToLower(identityB.String()): { + ResourceID: identityB, + }, + }, + expectedMatch: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + syncer := &fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer{} + spc := &coreapi.ServiceProviderCluster{} + spc.Status.DataPlaneOperatorsManagedIdentities.Identities = tc.spcIdentities + + assert.Equal(t, tc.expectedMatch, syncer.desiredDataPlaneOperatorResourceIDsMatchSPC(tc.desiredResourceIDs, spc)) + }) + } +} + +func TestUniqueDataPlaneOperatorResourceIDs(t *testing.T) { + t.Parallel() + + identityA := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/identity-a")) + mixedCaseIdentityA := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/Test-RG/providers/Microsoft.ManagedIdentity/userAssignedIdentities/Identity-A")) + + syncer := &fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer{} + + t.Run("dedupes shared identity across operators", func(t *testing.T) { + t.Parallel() + unique := syncer.uniqueDataPlaneOperatorResourceIDs(map[string]*azcorearm.ResourceID{ + "operator-a": identityA, + "operator-b": identityA, + }) + require.NotNil(t, unique) + assert.Equal(t, map[string]struct{}{ + strings.ToLower(identityA.String()): {}, + }, unique) + }) + + t.Run("lowercases resource ID keys", func(t *testing.T) { + t.Parallel() + unique := syncer.uniqueDataPlaneOperatorResourceIDs(map[string]*azcorearm.ResourceID{ + "operator-a": mixedCaseIdentityA, + }) + require.NotNil(t, unique) + assert.Equal(t, map[string]struct{}{ + strings.ToLower(identityA.String()): {}, + }, unique) + }) + + t.Run("nil resource ID returns nil", func(t *testing.T) { + t.Parallel() + unique := syncer.uniqueDataPlaneOperatorResourceIDs(map[string]*azcorearm.ResourceID{ + "operator-a": nil, + }) + assert.Nil(t, unique) + }) +} + +func TestFetchDataPlaneOperatorsManagedIdentitiesInfoNeedsWork(t *testing.T) { + t.Parallel() + + now := time.Date(2026, 8, 14, 12, 0, 0, 0, time.UTC) + identityA := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/identity-a")) + identityB := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/identity-b")) + + matchingDesired := map[string]struct{}{ + strings.ToLower(identityA.String()): {}, + } + matchingSPCIdentities := map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity{ + strings.ToLower(identityA.String()): { + ResourceID: identityA, + }, + } + + testCases := []struct { + name string + desiredResourceIDs map[string]struct{} + spcIdentities map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity + earliestRecheckTime *metav1.Time + expectedNeedsWork bool + }{ + { + name: "matching identities with future recheck skips work", + desiredResourceIDs: matchingDesired, + spcIdentities: matchingSPCIdentities, + earliestRecheckTime: &metav1.Time{Time: now.Add(time.Hour)}, + expectedNeedsWork: false, + }, + { + name: "matching identities with past recheck needs work", + desiredResourceIDs: matchingDesired, + spcIdentities: matchingSPCIdentities, + earliestRecheckTime: &metav1.Time{Time: now.Add(-time.Hour)}, + expectedNeedsWork: true, + }, + { + name: "matching identities with nil recheck needs work", + desiredResourceIDs: matchingDesired, + spcIdentities: matchingSPCIdentities, + earliestRecheckTime: nil, + expectedNeedsWork: true, + }, + { + name: "mismatched identities ignore future recheck", + desiredResourceIDs: map[string]struct{}{ + strings.ToLower(identityB.String()): {}, + }, + spcIdentities: matchingSPCIdentities, + earliestRecheckTime: &metav1.Time{Time: now.Add(time.Hour)}, + expectedNeedsWork: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + syncer := &fetchDataPlaneOperatorsManagedIdentitiesInfoSyncer{ + clock: clocktesting.NewFakePassiveClock(now), + } + spc := &coreapi.ServiceProviderCluster{} + spc.Status.DataPlaneOperatorsManagedIdentities.Identities = tc.spcIdentities + spc.Status.DataPlaneOperatorsManagedIdentities.EarliestRecheckTime = tc.earliestRecheckTime + + require.Equal(t, tc.expectedNeedsWork, syncer.needsWork(spc, tc.desiredResourceIDs)) + }) + } +} diff --git a/internal/api/coreapi/types_serviceprovider_cluster.go b/internal/api/coreapi/types_serviceprovider_cluster.go index cf32c5ea39e..b2c99177d3a 100644 --- a/internal/api/coreapi/types_serviceprovider_cluster.go +++ b/internal/api/coreapi/types_serviceprovider_cluster.go @@ -232,6 +232,42 @@ type ServiceProviderClusterStatus struct { // AzureResources tracks the lifecycle of Azure resources associated with // the cluster, including deny assignments and the managed resource group. AzureResources AzureResources `json:"azureResources,omitempty"` + + // DataPlaneOperatorsManagedIdentities tracks resolved ClientID/PrincipalID for + // the Azure User Assigned Managed Identities associated to the cluster's data + // plane operators, plus when Azure should next be re-queried for that info. + // A cluster's data plane operator is a kubernetes operator associated to the + // cluster that runs in the cluster's data plane. + // For example, the Cluster's CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators map + // contains the set of required data plane operators associated to a Cluster. + // Written by: FetchDataPlaneOperatorsManagedIdentitiesInfoController + DataPlaneOperatorsManagedIdentities ServiceProviderClusterDataPlaneOperatorsManagedIdentities `json:"dataPlaneOperatorsManagedIdentities,omitempty"` +} + +// ServiceProviderClusterDataPlaneOperatorsManagedIdentities holds the resolved +// managed-identity metadata for all data plane operators on a cluster, together +// with a single EarliestRecheckTime that applies to every entry in Identities. +type ServiceProviderClusterDataPlaneOperatorsManagedIdentities struct { + // Identities is a map containing resolved ClientID/PrincipalID for the Azure + // User Assigned Managed Identities associated to the cluster's data plane + // operators. The key is the fully lowercased Azure Resource ID of the + // identity. Which operators reference each identity is tracked on + // Cluster.CustomerProperties, not here. Multiple operators may share one + // identity entry. + // Written by: FetchDataPlaneOperatorsManagedIdentitiesInfoController + Identities map[string]*ServiceProviderClusterDataPlaneOperatorManagedIdentity `json:"identities,omitempty"` + // EarliestRecheckTime is the earliest time at which the controller should + // re-query Azure for ClientID/PrincipalID of Identities. Nil means recheck + // immediately. The same recheck time applies across all elements of Identities. + // This allows the controller to avoid repeatedly hitting an Azure API to + // recheck that the desired state is true. + // Controllers should set this field with substantial jitter: without another + // concern, jitter of 50% is considered normal so that any storms are quickly + // dissipated. Additionally, long recheck times are recommended for resources + // outside of their active phases. Order of at least six hours is, with + // durations up to 24 hours considered normal. + // Written by: FetchDataPlaneOperatorsManagedIdentitiesInfoController + EarliestRecheckTime *metav1.Time `json:"earliestRecheckTime,omitempty"` } // AzureResources groups the Azure resource references associated with a cluster. @@ -280,6 +316,24 @@ type AzureReference struct { EarliestRecheckTime *metav1.Time `json:"earliestRecheckTime,omitempty"` } +// ServiceProviderClusterDataPlaneOperatorManagedIdentity contains resolved +// ClientID/PrincipalID for an Azure User Assigned Managed Identity used by one +// or more of a cluster's data plane operators. +// A cluster's data plane operator is a customer operator associated to the cluster that runs in the cluster's data plane. +// Which operators reference this identity is tracked on +// Cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators. +type ServiceProviderClusterDataPlaneOperatorManagedIdentity struct { + // ResourceID is the Azure Resource ID of the Azure User Assigned Managed Identity. + // Its value comes from the Cluster's CustomerProperties. + ResourceID *azcorearm.ResourceID `json:"resourceID,omitempty"` + // ClientID is Client ID of the Azure User Assigned Managed Identity represented by ResourceID. + // Fetched from Azure and written here by the FetchDataPlaneOperatorsManagedIdentitiesInfoController. + ClientID *string `json:"clientID,omitempty"` + // PrincipalID Principal ID of the Azure User Assigned Managed Identity represented by ResourceID. + // Fetched from Azure and written here by the FetchDataPlaneOperatorsManagedIdentitiesInfoController. + PrincipalID *string `json:"principalID,omitempty"` +} + // ServiceProviderClusterStatusVersion contains the actual version information. type ServiceProviderClusterStatusVersion struct { // ActiveVersions is an array of versions currently active in the control plane, ordered with the most recent first. diff --git a/internal/api/coreapi/zz_generated.deepcopy.go b/internal/api/coreapi/zz_generated.deepcopy.go index 335fa228d8f..c1100e9d699 100644 --- a/internal/api/coreapi/zz_generated.deepcopy.go +++ b/internal/api/coreapi/zz_generated.deepcopy.go @@ -1991,6 +1991,71 @@ func (in *ServiceProviderCluster) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ServiceProviderClusterDataPlaneOperatorManagedIdentity) DeepCopyInto(out *ServiceProviderClusterDataPlaneOperatorManagedIdentity) { + *out = *in + if in.ResourceID != nil { + in, out := &in.ResourceID, &out.ResourceID + *out = DeepCopyResourceID(*in) + } + if in.ClientID != nil { + in, out := &in.ClientID, &out.ClientID + *out = new(string) + **out = **in + } + if in.PrincipalID != nil { + in, out := &in.PrincipalID, &out.PrincipalID + *out = new(string) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceProviderClusterDataPlaneOperatorManagedIdentity. +func (in *ServiceProviderClusterDataPlaneOperatorManagedIdentity) DeepCopy() *ServiceProviderClusterDataPlaneOperatorManagedIdentity { + if in == nil { + return nil + } + out := new(ServiceProviderClusterDataPlaneOperatorManagedIdentity) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ServiceProviderClusterDataPlaneOperatorsManagedIdentities) DeepCopyInto(out *ServiceProviderClusterDataPlaneOperatorsManagedIdentities) { + *out = *in + if in.Identities != nil { + in, out := &in.Identities, &out.Identities + *out = make(map[string]*ServiceProviderClusterDataPlaneOperatorManagedIdentity, len(*in)) + for key, val := range *in { + var outVal *ServiceProviderClusterDataPlaneOperatorManagedIdentity + if val == nil { + (*out)[key] = nil + } else { + in, out := &val, &outVal + *out = new(ServiceProviderClusterDataPlaneOperatorManagedIdentity) + (*in).DeepCopyInto(*out) + } + (*out)[key] = outVal + } + } + if in.EarliestRecheckTime != nil { + in, out := &in.EarliestRecheckTime, &out.EarliestRecheckTime + *out = (*in).DeepCopy() + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceProviderClusterDataPlaneOperatorsManagedIdentities. +func (in *ServiceProviderClusterDataPlaneOperatorsManagedIdentities) DeepCopy() *ServiceProviderClusterDataPlaneOperatorsManagedIdentities { + if in == nil { + return nil + } + out := new(ServiceProviderClusterDataPlaneOperatorsManagedIdentities) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServiceProviderClusterList) DeepCopyInto(out *ServiceProviderClusterList) { *out = *in @@ -2116,6 +2181,7 @@ func (in *ServiceProviderClusterStatus) DeepCopyInto(out *ServiceProviderCluster **out = **in } in.AzureResources.DeepCopyInto(&out.AzureResources) + in.DataPlaneOperatorsManagedIdentities.DeepCopyInto(&out.DataPlaneOperatorsManagedIdentities) return }