ARO-13916 persist user assigned identities with client and object id …

…after dynamic validation
Azure · Jan 14, 2025 · 9d8ecb9 · 9d8ecb9
1 parent 02a7ba7
commit 9d8ecb9
Show file tree

Hide file tree

Showing 11 changed files with 95 additions and 146 deletions.
diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go
@@ -137,6 +137,7 @@ type manager struct {
 	openShiftClusterDocumentVersioner openShiftClusterDocumentVersioner
 
 	platformWorkloadIdentityRolesByVersion platformworkloadidentity.PlatformWorkloadIdentityRolesByVersion
+	platformWorkloadIdentities             map[string]api.PlatformWorkloadIdentity
 }
 
 // New returns a cluster manager

diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go
@@ -220,6 +220,7 @@ func (m *manager) Update(ctx context.Context) error {
 			steps.Action(m.fixupClusterMsiTenantID),
 			steps.Action(m.ensureClusterMsiCertificate),
 			steps.Action(m.initializeClusterMsiClients),
+			steps.Action(m.platformWorkloadIdentityIDs),
 		)
 	}
 
@@ -228,7 +229,7 @@ func (m *manager) Update(ctx context.Context) error {
 	if m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
 		s = append(s,
 			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.clusterIdentityIDs),
-			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.platformWorkloadIdentityIDs),
+			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.persistPlatformWorkloadIdentityIDs),
 			steps.Action(m.federateIdentityCredentials),
 		)
 	} else {
@@ -346,6 +347,7 @@ func (m *manager) bootstrap() []steps.Step {
 		s = append(s,
 			steps.Action(m.ensureClusterMsiCertificate),
 			steps.Action(m.initializeClusterMsiClients),
+			steps.Action(m.platformWorkloadIdentityIDs),
 		)
 	}
 
@@ -354,7 +356,7 @@ func (m *manager) bootstrap() []steps.Step {
 	if m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
 		s = append(s,
 			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.clusterIdentityIDs),
-			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.platformWorkloadIdentityIDs),
+			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.persistPlatformWorkloadIdentityIDs),
 		)
 	} else {
 		s = append(s,

diff --git a/pkg/cluster/platformworkloadidentities.go b/pkg/cluster/platformworkloadidentities.go
@@ -7,26 +7,51 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/msi/armmsi"
+
 	"github.com/Azure/ARO-RP/pkg/api"
-	"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
 )
 
-func (m *manager) platformWorkloadIdentityIDs(ctx context.Context) error {
-	var err error
+func (m *manager) persistPlatformWorkloadIdentityIDs(ctx context.Context) (err error) {
 	if !m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
-		return fmt.Errorf("platformWorkloadIdentityIDs called for CSP cluster")
-	}
-
-	identities := m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities
-	updatedIdentities, err := platformworkloadidentity.GetPlatformWorkloadIdentityIDs(ctx, identities, m.userAssignedIdentities)
-	if err != nil {
-		return err
+		return fmt.Errorf("persistPlatformWorkloadIdentityIDs called for CSP cluster")
 	}
 
 	m.doc, err = m.db.PatchWithLease(ctx, m.doc.Key, func(doc *api.OpenShiftClusterDocument) error {
-		doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities = updatedIdentities
+		doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities = m.platformWorkloadIdentities
 		return nil
 	})
 
 	return err
 }
+
+func (m *manager) platformWorkloadIdentityIDs(ctx context.Context) error {
+	if !m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
+		return fmt.Errorf("platformWorkloadIdentityIDs called for CSP cluster")
+	}
+
+	identities := m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities
+	updatedIdentities := make(map[string]api.PlatformWorkloadIdentity, len(identities))
+
+	for operatorName, identity := range identities {
+		resourceId, err := arm.ParseResourceID(identity.ResourceID)
+		if err != nil {
+			return fmt.Errorf("platform workload identity '%s' invalid: %w", operatorName, err)
+		}
+
+		identityDetails, err := m.userAssignedIdentities.Get(ctx, resourceId.ResourceGroupName, resourceId.Name, &armmsi.UserAssignedIdentitiesClientGetOptions{})
+		if err != nil {
+			return fmt.Errorf("error occured when retrieving platform workload identity '%s' details: %w", operatorName, err)
+		}
+
+		updatedIdentities[operatorName] = api.PlatformWorkloadIdentity{
+			ResourceID: identity.ResourceID,
+			ClientID:   *identityDetails.Properties.ClientID,
+			ObjectID:   *identityDetails.Properties.PrincipalID,
+		}
+	}
+
+	m.platformWorkloadIdentities = updatedIdentities
+	return nil
+}
diff --git a/pkg/cluster/platformworkloadidentities_test.go b/pkg/cluster/platformworkloadidentities_test.go
@@ -186,6 +186,11 @@ func TestPlatformWorkloadIdentityIDs(t *testing.T) {
 			err := m.platformWorkloadIdentityIDs(ctx)
 			utilerror.AssertErrorMessage(t, err, tt.wantErr)
 
+			if err == nil {
+				err = m.persistPlatformWorkloadIdentityIDs(ctx)
+				utilerror.AssertErrorMessage(t, err, tt.wantErr)
+			}
+
 			if tt.wantIdentities != nil {
 				assert.Equal(t, *tt.wantIdentities, m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities)
 			}

diff --git a/pkg/cluster/validate.go b/pkg/cluster/validate.go
@@ -16,6 +16,6 @@ func (m *manager) validateResources(ctx context.Context) error {
 		clusterMSICredential = m.userAssignedIdentities.GetClusterMSICredential()
 	}
 	return validate.NewOpenShiftClusterDynamicValidator(
-		m.log, m.env, m.doc.OpenShiftCluster, m.subscriptionDoc, m.fpAuthorizer, m.armRoleDefinitions, m.clusterMsiFederatedIdentityCredentials, m.userAssignedIdentities, m.platformWorkloadIdentityRolesByVersion, clusterMSICredential,
+		m.log, m.env, m.doc.OpenShiftCluster, m.subscriptionDoc, m.fpAuthorizer, m.armRoleDefinitions, m.clusterMsiFederatedIdentityCredentials, m.platformWorkloadIdentities, m.platformWorkloadIdentityRolesByVersion, clusterMSICredential,
 	).Dynamic(ctx)
 }
diff --git a/pkg/util/mocks/dynamic/dynamic.go b/pkg/util/mocks/dynamic/dynamic.go
diff --git a/pkg/util/platformworkloadidentity/platformworkloadidentities.go b/pkg/util/platformworkloadidentity/platformworkloadidentities.go
diff --git a/pkg/validate/dynamic/dynamic.go b/pkg/validate/dynamic/dynamic.go
@@ -88,7 +88,7 @@ type Dynamic interface {
 		platformWorkloadIdentityRolesByRoleName map[string]api.PlatformWorkloadIdentityRole,
 		roleDefinitions armauthorization.RoleDefinitionsClient,
 		clusterMsiFederatedIdentityCredentials armmsi.FederatedIdentityCredentialsClient,
-		userAssignedIdentityClient armmsi.UserAssignedIdentitiesClient,
+		platformWorkloadIdentities map[string]api.PlatformWorkloadIdentity,
 	) error
 }
 

diff --git a/pkg/validate/dynamic/platformworkloadidentityprofile.go b/pkg/validate/dynamic/platformworkloadidentityprofile.go
@@ -30,15 +30,12 @@ func (dv *dynamic) ValidatePlatformWorkloadIdentityProfile(
 	platformWorkloadIdentityRolesByRoleName map[string]api.PlatformWorkloadIdentityRole,
 	roleDefinitions armauthorization.RoleDefinitionsClient,
 	clusterMsiFederatedIdentityCredentials armmsi.FederatedIdentityCredentialsClient,
-	userAssignedIdentitiesClient armmsi.UserAssignedIdentitiesClient,
+	platformWorkloadIdentities map[string]api.PlatformWorkloadIdentity, // Platform Workload Identities with object and client IDs
 ) (err error) {
 	dv.log.Print("ValidatePlatformWorkloadIdentityProfile")
 
 	dv.platformIdentitiesActionsMap = map[string][]string{}
-	dv.platformIdentities, err = platformworkloadidentity.GetPlatformWorkloadIdentityIDs(ctx, oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities, userAssignedIdentitiesClient)
-	if err != nil {
-		return err
-	}
+	dv.platformIdentities = platformWorkloadIdentities
 
 	// Check if any required platform identity is missing
 	if len(dv.platformIdentities) != len(platformWorkloadIdentityRolesByRoleName) {