Support Update requirements for Workload Identity clusters (#3935)

* Do not clobber existing PlatformWorkloadIdentity readonly fields (clientid/objectid) on patch

* Ensure CSP-specific update steps only run for CSP clusters

* Ensure ClientId/ObjectIds are populated for all platform workload identities

* Ensure required federated identity credentials during cluster update

Note that additional work is still required to ensure this works as expected,
which will be handled in follow-up efforts.

* Add step to directly deploy platform workload identity credential secrets on-cluster during Update

* Refactor: extract mock platformWorkloadIdentityRolesByVersion setup to shared function

* Do not clobber existing ManagedServiceIdentity fields (IssuerURI) on patch

* Apply upgradeable-to annotation to cloudcredential resource via Patch

This avoids issues with e.g. having the wrong version of the resource struct
definition vendored into the RP.

* Fix az aro update request body handling

- Only pass in new/updated identities (RP will add these to the existing identity map during a patch operation)
- Only set the upgradeableTo property if it is explicitly set

pkg/api/v20240812preview/openshiftcluster_convert.go

@@ -212,7 +212,9 @@ func (c openShiftClusterConverter) ToInternal(_oc interface{}, out *api.OpenShif
 	}
 
 	if oc.Identity != nil {
-		out.Identity = &api.ManagedServiceIdentity{}
+		if out.Identity == nil {
+			out.Identity = &api.ManagedServiceIdentity{}
+		}
 		out.Identity.Type = api.ManagedServiceIdentityType(oc.Identity.Type)
 		out.Identity.UserAssignedIdentities = make(map[string]api.UserAssignedIdentity, len(oc.Identity.UserAssignedIdentities))
 		for k := range oc.Identity.UserAssignedIdentities {
@@ -239,23 +241,28 @@ func (c openShiftClusterConverter) ToInternal(_oc interface{}, out *api.OpenShif
 		}
 	}
 	if oc.Properties.PlatformWorkloadIdentityProfile != nil && oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities != nil {
-		out.Properties.PlatformWorkloadIdentityProfile = &api.PlatformWorkloadIdentityProfile{}
+		if out.Properties.PlatformWorkloadIdentityProfile == nil {
+			out.Properties.PlatformWorkloadIdentityProfile = &api.PlatformWorkloadIdentityProfile{}
+		}
 
 		if oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo != nil {
 			temp := api.UpgradeableTo(*oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo)
 			out.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo = &temp
 		}
 
-		out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities = make(map[string]api.PlatformWorkloadIdentity, len(oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities))
+		if out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities == nil {
+			out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities = make(map[string]api.PlatformWorkloadIdentity, len(oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities))
+		}
 
-		for k := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
+		for k, identity := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
-			pwi := api.PlatformWorkloadIdentity{
+			if pwi, exists := out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k]; exists {
-				ClientID:   oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k].ClientID,
+				pwi.ResourceID = identity.ResourceID
-				ObjectID:   oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k].ObjectID,
+				out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k] = pwi
-				ResourceID: oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k].ResourceID,
+			} else {
+				out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k] = api.PlatformWorkloadIdentity{
+					ResourceID: identity.ResourceID,
+				}
 			}
-
-			out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k] = pwi
 		}
 	}
 

pkg/cluster/install.go

@@ -214,12 +214,28 @@ func (m *manager) Update(ctx context.Context) error {
 		steps.AuthorizationRetryingAction(m.fpAuthorizer, m.validateResources),
 		steps.Action(m.initializeKubernetesClients), // All init steps are first
 		steps.Action(m.initializeOperatorDeployer),  // depends on kube clients
-		// Since ServicePrincipalProfile is now a pointer and our converters re-build the struct,
+	}
-		// our update path needs to enrich the doc with SPObjectID since it was overwritten by our API on put/patch.
-		steps.AuthorizationRetryingAction(m.fpAuthorizer, m.fixupClusterSPObjectID),
 
-		// credentials rotation flow steps
+	if m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
-		steps.Action(m.createOrUpdateClusterServicePrincipalRBAC),
+		s = append(s,
+			steps.Action(m.ensureClusterMsiCertificate),
+			steps.Action(m.initializeClusterMsiClients),
+			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.clusterIdentityIDs),
+			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.platformWorkloadIdentityIDs),
+			steps.Action(m.federateIdentityCredentials),
+		)
+	} else {
+		s = append(s,
+			// Since ServicePrincipalProfile is now a pointer and our converters re-build the struct,
+			// our update path needs to enrich the doc with SPObjectID since it was overwritten by our API on put/patch.
+			steps.AuthorizationRetryingAction(m.fpAuthorizer, m.fixupClusterSPObjectID),
+
+			// CSP credentials rotation flow steps
+			steps.Action(m.createOrUpdateClusterServicePrincipalRBAC),
+		)
+	}
+
+	s = append(s,
 		steps.Action(m.createOrUpdateDenyAssignment),
 		steps.Action(m.startVMs),
 		steps.Condition(m.apiServersReady, 30*time.Minute, true),
@@ -229,14 +245,23 @@ func (m *manager) Update(ctx context.Context) error {
 		steps.Action(m.configureIngressCertificate),
 		steps.Action(m.renewMDSDCertificate),
 		steps.Action(m.fixUserAdminKubeconfig),
-		steps.Action(m.ensureCredentialsRequest),
-		steps.Action(m.updateOpenShiftSecret),
-		steps.Condition(m.aroCredentialsRequestReconciled, 3*time.Minute, true),
-		steps.Action(m.updateAROSecret),
-		steps.Action(m.restartAROOperatorMaster), // depends on m.updateOpenShiftSecret; the point of restarting is to pick up any changes made to the secret
-		steps.Condition(m.aroDeploymentReady, 5*time.Minute, true),
-		steps.Action(m.ensureUpgradeAnnotation),
 		steps.Action(m.reconcileLoadBalancerProfile),
+	)
+
+	if m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
+		s = append(s,
+			steps.Action(m.ensureUpgradeAnnotation),
+			steps.Action(m.deployPlatformWorkloadIdentitySecrets),
+		)
+	} else {
+		s = append(s,
+			steps.Action(m.ensureCredentialsRequest),
+			steps.Action(m.updateOpenShiftSecret),
+			steps.Condition(m.aroCredentialsRequestReconciled, 3*time.Minute, true),
+			steps.Action(m.updateAROSecret),
+			steps.Action(m.restartAROOperatorMaster), // depends on m.updateOpenShiftSecret; the point of restarting is to pick up any changes made to the secret
+			steps.Condition(m.aroDeploymentReady, 5*time.Minute, true),
+		)
 	}
 
 	if m.adoptViaHive {

pkg/cluster/workloadidentityresources.go

@@ -4,6 +4,7 @@ package cluster
 // Licensed under the Apache License 2.0.
 
 import (
+	"context"
 	"crypto/sha256"
 	"fmt"
 	"math/big"
@@ -63,6 +64,19 @@ func (m *manager) generateWorkloadIdentityResources() (map[string]kruntime.Objec
 	return resources, nil
 }
 
+func (m *manager) deployPlatformWorkloadIdentitySecrets(ctx context.Context) error {
+	secrets, err := m.generatePlatformWorkloadIdentitySecrets()
+	if err != nil {
+		return err
+	}
+	resources := make([]kruntime.Object, len(secrets))
+	for i, secret := range secrets {
+		resources[i] = secret
+	}
+
+	return m.ch.Ensure(ctx, resources...)
+}
+
 func (m *manager) generatePlatformWorkloadIdentitySecrets() ([]*corev1.Secret, error) {
 	subscriptionId := m.subscriptionDoc.ID
 	tenantId := m.subscriptionDoc.Subscription.Properties.TenantID

pkg/cluster/workloadidentityresources_test.go

@@ -4,19 +4,25 @@ package cluster
 // Licensed under the Apache License 2.0.
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
 	configv1 "github.com/openshift/api/config/v1"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"go.uber.org/mock/gomock"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
+	ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"github.com/Azure/ARO-RP/pkg/api"
+	"github.com/Azure/ARO-RP/pkg/util/clienthelper"
 	mock_platformworkloadidentity "github.com/Azure/ARO-RP/pkg/util/mocks/platformworkloadidentity"
+	"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
 	utilerror "github.com/Azure/ARO-RP/test/util/error"
 )
 
@@ -135,13 +141,6 @@ func TestGenerateWorkloadIdentityResources(t *testing.T) {
 			controller := gomock.NewController(t)
 			defer controller.Finish()
 
-			pwiRolesByVersion := mock_platformworkloadidentity.NewMockPlatformWorkloadIdentityRolesByVersion(controller)
-			platformWorkloadIdentityRolesByRoleName := map[string]api.PlatformWorkloadIdentityRole{}
-			for _, role := range tt.roles {
-				platformWorkloadIdentityRolesByRoleName[role.OperatorName] = role
-			}
-			pwiRolesByVersion.EXPECT().GetPlatformWorkloadIdentityRolesByRoleName().AnyTimes().Return(platformWorkloadIdentityRolesByRoleName)
-
 			m := manager{
 				doc: &api.OpenShiftClusterDocument{
 					OpenShiftCluster: &api.OpenShiftCluster{
@@ -164,7 +163,9 @@ func TestGenerateWorkloadIdentityResources(t *testing.T) {
 					},
 				},
 
-				platformWorkloadIdentityRolesByVersion: pwiRolesByVersion,
+				platformWorkloadIdentityRolesByVersion: mockPlatformWorkloadIdentityRolesByVersion(
+					controller, tt.roles,
+				),
 			}
 			if tt.usesWorkloadIdentity {
 				m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile = &api.PlatformWorkloadIdentityProfile{
@@ -180,6 +181,136 @@ func TestGenerateWorkloadIdentityResources(t *testing.T) {
 	}
 }
 
+func TestDeployPlatformWorkloadIdentitySecrets(t *testing.T) {
+	tenantId := "00000000-0000-0000-0000-000000000000"
+	subscriptionId := "ffffffff-ffff-ffff-ffff-ffffffffffff"
+	location := "eastus"
+
+	for _, tt := range []struct {
+		name       string
+		identities map[string]api.PlatformWorkloadIdentity
+		roles      []api.PlatformWorkloadIdentityRole
+		want       []*corev1.Secret
+	}{
+		{
+			name: "updates PWI secrets if a role definition is present",
+			identities: map[string]api.PlatformWorkloadIdentity{
+				"foo": {
+					ClientID: "00f00f00-0f00-0f00-0f00-f00f00f00f00",
+				},
+				"bar": {
+					ClientID: "00ba4ba4-0ba4-0ba4-0ba4-ba4ba4ba4ba4",
+				},
+			},
+			roles: []api.PlatformWorkloadIdentityRole{
+				{
+					OperatorName: "foo",
+					SecretLocation: api.SecretLocation{
+						Namespace: "openshift-foo",
+						Name:      "azure-cloud-credentials",
+					},
+				},
+				{
+					OperatorName: "bar",
+					SecretLocation: api.SecretLocation{
+						Namespace: "openshift-bar",
+						Name:      "azure-cloud-credentials",
+					},
+				},
+			},
+			want: []*corev1.Secret{
+				{
+					TypeMeta: metav1.TypeMeta{
+						APIVersion: "v1",
+						Kind:       "Secret",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace:       "openshift-foo",
+						Name:            "azure-cloud-credentials",
+						ResourceVersion: "1",
+					},
+					Type: corev1.SecretTypeOpaque,
+					StringData: map[string]string{
+						"azure_client_id":            "00f00f00-0f00-0f00-0f00-f00f00f00f00",
+						"azure_subscription_id":      subscriptionId,
+						"azure_tenant_id":            tenantId,
+						"azure_region":               location,
+						"azure_federated_token_file": azureFederatedTokenFileLocation,
+					},
+				},
+				{
+					TypeMeta: metav1.TypeMeta{
+						APIVersion: "v1",
+						Kind:       "Secret",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace:       "openshift-bar",
+						Name:            "azure-cloud-credentials",
+						ResourceVersion: "1",
+					},
+					Type: corev1.SecretTypeOpaque,
+					StringData: map[string]string{
+						"azure_client_id":            "00ba4ba4-0ba4-0ba4-0ba4-ba4ba4ba4ba4",
+						"azure_subscription_id":      subscriptionId,
+						"azure_tenant_id":            tenantId,
+						"azure_region":               location,
+						"azure_federated_token_file": azureFederatedTokenFileLocation,
+					},
+				},
+			},
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			clientFake := ctrlfake.NewClientBuilder().Build()
+
+			ch := clienthelper.NewWithClient(logrus.NewEntry(logrus.StandardLogger()), clientFake)
+
+			m := manager{
+				doc: &api.OpenShiftClusterDocument{
+					OpenShiftCluster: &api.OpenShiftCluster{
+						Location: location,
+						Properties: api.OpenShiftClusterProperties{
+							PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
+								PlatformWorkloadIdentities: tt.identities,
+							},
+						},
+					},
+				},
+				subscriptionDoc: &api.SubscriptionDocument{
+					ID: subscriptionId,
+					Subscription: &api.Subscription{
+						Properties: &api.SubscriptionProperties{
+							TenantID: tenantId,
+						},
+					},
+				},
+
+				ch: ch,
+
+				platformWorkloadIdentityRolesByVersion: mockPlatformWorkloadIdentityRolesByVersion(
+					controller, tt.roles,
+				),
+			}
+			err := m.deployPlatformWorkloadIdentitySecrets(ctx)
+
+			utilerror.AssertErrorMessage(t, err, "")
+
+			for _, wantSecret := range tt.want {
+				gotSecret := &corev1.Secret{}
+				key := types.NamespacedName{Name: wantSecret.Name, Namespace: wantSecret.Namespace}
+				if err := clientFake.Get(ctx, key, gotSecret); err != nil {
+					t.Error(err)
+				}
+				assert.Equal(t, wantSecret, gotSecret)
+			}
+		})
+	}
+}
+
 func TestGeneratePlatformWorkloadIdentitySecrets(t *testing.T) {
 	tenantId := "00000000-0000-0000-0000-000000000000"
 	subscriptionId := "ffffffff-ffff-ffff-ffff-ffffffffffff"
@@ -327,13 +458,6 @@ func TestGeneratePlatformWorkloadIdentitySecrets(t *testing.T) {
 			controller := gomock.NewController(t)
 			defer controller.Finish()
 
-			pwiRolesByVersion := mock_platformworkloadidentity.NewMockPlatformWorkloadIdentityRolesByVersion(controller)
-			platformWorkloadIdentityRolesByRoleName := map[string]api.PlatformWorkloadIdentityRole{}
-			for _, role := range tt.roles {
-				platformWorkloadIdentityRolesByRoleName[role.OperatorName] = role
-			}
-			pwiRolesByVersion.EXPECT().GetPlatformWorkloadIdentityRolesByRoleName().AnyTimes().Return(platformWorkloadIdentityRolesByRoleName)
-
 			m := manager{
 				doc: &api.OpenShiftClusterDocument{
 					OpenShiftCluster: &api.OpenShiftCluster{
@@ -354,7 +478,9 @@ func TestGeneratePlatformWorkloadIdentitySecrets(t *testing.T) {
 					},
 				},
 
-				platformWorkloadIdentityRolesByVersion: pwiRolesByVersion,
+				platformWorkloadIdentityRolesByVersion: mockPlatformWorkloadIdentityRolesByVersion(
+					controller, tt.roles,
+				),
 			}
 			got, err := m.generatePlatformWorkloadIdentitySecrets()
 
@@ -558,3 +684,14 @@ func TestGetPlatformWorkloadIdentityFederatedCredName(t *testing.T) {
 		})
 	}
 }
+
+func mockPlatformWorkloadIdentityRolesByVersion(controller *gomock.Controller, roles []api.PlatformWorkloadIdentityRole) platformworkloadidentity.PlatformWorkloadIdentityRolesByVersion {
+	pwiRolesByVersion := mock_platformworkloadidentity.NewMockPlatformWorkloadIdentityRolesByVersion(controller)
+	platformWorkloadIdentityRolesByRoleName := map[string]api.PlatformWorkloadIdentityRole{}
+	for _, role := range roles {
+		platformWorkloadIdentityRolesByRoleName[role.OperatorName] = role
+	}
+	pwiRolesByVersion.EXPECT().GetPlatformWorkloadIdentityRolesByRoleName().AnyTimes().Return(platformWorkloadIdentityRolesByRoleName)
+
+	return pwiRolesByVersion
+}

pkg/frontend/openshiftcluster_putorpatch_test.go

@@ -2122,30 +2122,14 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 				}
 				oc.Properties.PlatformWorkloadIdentityProfile = &v20240812preview.PlatformWorkloadIdentityProfile{
 					PlatformWorkloadIdentities: map[string]v20240812preview.PlatformWorkloadIdentity{
-						"AzureFilesStorageOperator": {
+						"file-csi-driver":          {ResourceID: mockMiResourceId},
-							ResourceID: mockMiResourceId,
+						"cloud-controller-manager": {ResourceID: mockMiResourceId},
-						},
+						"ingress":                  {ResourceID: mockMiResourceId},
-						"CloudControllerManager": {
+						"image-registry":           {ResourceID: mockMiResourceId},
-							ResourceID: mockMiResourceId,
+						"machine-api":              {ResourceID: mockMiResourceId},
-						},
+						"cloud-network-config":     {ResourceID: mockMiResourceId},
-						"ClusterIngressOperator": {
+						"aro-operator":             {ResourceID: mockMiResourceId},
-							ResourceID: mockMiResourceId,
+						"disk-csi-driver":          {ResourceID: mockMiResourceId},
-						},
-						"ImageRegistryOperator": {
-							ResourceID: mockMiResourceId,
-						},
-						"MachineApiOperator": {
-							ResourceID: mockMiResourceId,
-						},
-						"NetworkOperator": {
-							ResourceID: mockMiResourceId,
-						},
-						"ServiceOperator": {
-							ResourceID: mockMiResourceId,
-						},
-						"StorageOperator": {
-							ResourceID: mockMiResourceId,
-						},
 					},
 				}
 			},
@@ -2214,30 +2198,14 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 							OperatorFlags: operator.DefaultOperatorFlags(),
 							PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
 								PlatformWorkloadIdentities: map[string]api.PlatformWorkloadIdentity{
-									"AzureFilesStorageOperator": {
+									"file-csi-driver":          {ResourceID: mockMiResourceId},
-										ResourceID: mockMiResourceId,
+									"cloud-controller-manager": {ResourceID: mockMiResourceId},
-									},
+									"ingress":                  {ResourceID: mockMiResourceId},
-									"CloudControllerManager": {
+									"image-registry":           {ResourceID: mockMiResourceId},
-										ResourceID: mockMiResourceId,
+									"machine-api":              {ResourceID: mockMiResourceId},
-									},
+									"cloud-network-config":     {ResourceID: mockMiResourceId},
-									"ClusterIngressOperator": {
+									"aro-operator":             {ResourceID: mockMiResourceId},
-										ResourceID: mockMiResourceId,
+									"disk-csi-driver":          {ResourceID: mockMiResourceId},
-									},
-									"ImageRegistryOperator": {
-										ResourceID: mockMiResourceId,
-									},
-									"MachineApiOperator": {
-										ResourceID: mockMiResourceId,
-									},
-									"NetworkOperator": {
-										ResourceID: mockMiResourceId,
-									},
-									"ServiceOperator": {
-										ResourceID: mockMiResourceId,
-									},
-									"StorageOperator": {
-										ResourceID: mockMiResourceId,
-									},
 								},
 							},
 						},
@@ -2278,30 +2246,14 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 					PlatformWorkloadIdentityProfile: &v20240812preview.PlatformWorkloadIdentityProfile{
 						PlatformWorkloadIdentities: map[string]v20240812preview.PlatformWorkloadIdentity{
-							"AzureFilesStorageOperator": {
+							"file-csi-driver":          {ResourceID: mockMiResourceId},
-								ResourceID: mockMiResourceId,
+							"cloud-controller-manager": {ResourceID: mockMiResourceId},
-							},
+							"ingress":                  {ResourceID: mockMiResourceId},
-							"CloudControllerManager": {
+							"image-registry":           {ResourceID: mockMiResourceId},
-								ResourceID: mockMiResourceId,
+							"machine-api":              {ResourceID: mockMiResourceId},
-							},
+							"cloud-network-config":     {ResourceID: mockMiResourceId},
-							"ClusterIngressOperator": {
+							"aro-operator":             {ResourceID: mockMiResourceId},
-								ResourceID: mockMiResourceId,
+							"disk-csi-driver":          {ResourceID: mockMiResourceId},
-							},
-							"ImageRegistryOperator": {
-								ResourceID: mockMiResourceId,
-							},
-							"MachineApiOperator": {
-								ResourceID: mockMiResourceId,
-							},
-							"NetworkOperator": {
-								ResourceID: mockMiResourceId,
-							},
-							"ServiceOperator": {
-								ResourceID: mockMiResourceId,
-							},
-							"StorageOperator": {
-								ResourceID: mockMiResourceId,
-							},
 						},
 					},
 				},
@@ -2898,6 +2850,236 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 							EncryptionAtHost: v20240812preview.EncryptionAtHostDisabled,
 						},
 					},
+					MasterProfile: v20240812preview.MasterProfile{
+						EncryptionAtHost: v20240812preview.EncryptionAtHostDisabled,
+					},
+					NetworkProfile: v20240812preview.NetworkProfile{
+						OutboundType:     v20240812preview.OutboundTypeLoadbalancer,
+						PreconfiguredNSG: v20240812preview.PreconfiguredNSGDisabled,
+						LoadBalancerProfile: &v20240812preview.LoadBalancerProfile{
+							ManagedOutboundIPs: &v20240812preview.ManagedOutboundIPs{
+								Count: 1,
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "patch a workload identity cluster succeeded",
+			request: func(oc *v20240812preview.OpenShiftCluster) {
+				oc.Properties.PlatformWorkloadIdentityProfile = &v20240812preview.PlatformWorkloadIdentityProfile{
+					PlatformWorkloadIdentities: map[string]v20240812preview.PlatformWorkloadIdentity{
+						"file-csi-driver":          {ResourceID: mockMiResourceId},
+						"cloud-controller-manager": {ResourceID: mockMiResourceId},
+						"ingress":                  {ResourceID: mockMiResourceId},
+						"image-registry":           {ResourceID: mockMiResourceId},
+						"machine-api":              {ResourceID: mockMiResourceId},
+						"cloud-network-config":     {ResourceID: mockMiResourceId},
+						"aro-operator":             {ResourceID: mockMiResourceId},
+						"disk-csi-driver":          {ResourceID: mockMiResourceId},
+						"new":                      {ResourceID: mockMiResourceId},
+					},
+				}
+			},
+			isPatch: true,
+			fixture: func(f *testdatabase.Fixture) {
+				f.AddSubscriptionDocuments(&api.SubscriptionDocument{
+					ID: mockGuid,
+					Subscription: &api.Subscription{
+						State: api.SubscriptionStateRegistered,
+						Properties: &api.SubscriptionProperties{
+							TenantID: "11111111-1111-1111-1111-111111111111",
+						},
+					},
+				})
+				f.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{
+					Key: strings.ToLower(testdatabase.GetResourcePath(mockGuid, "resourceName")),
+					OpenShiftCluster: &api.OpenShiftCluster{
+						ID:   testdatabase.GetResourcePath(mockGuid, "resourceName"),
+						Name: "resourceName",
+						Type: "Microsoft.RedHatOpenShift/openShiftClusters",
+						Tags: map[string]string{"tag": "will-be-kept"},
+						Properties: api.OpenShiftClusterProperties{
+							ProvisioningState: api.ProvisioningStateSucceeded,
+							IngressProfiles:   []api.IngressProfile{{Name: "default"}},
+							WorkerProfiles: []api.WorkerProfile{
+								{
+									Name:             "default",
+									EncryptionAtHost: api.EncryptionAtHostDisabled,
+								},
+							},
+							NetworkProfile: api.NetworkProfile{
+								SoftwareDefinedNetwork: api.SoftwareDefinedNetworkOpenShiftSDN,
+								OutboundType:           api.OutboundTypeLoadbalancer,
+							},
+							MasterProfile: api.MasterProfile{
+								EncryptionAtHost: api.EncryptionAtHostDisabled,
+							},
+							OperatorFlags: api.OperatorFlags{},
+							PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
+								PlatformWorkloadIdentities: map[string]api.PlatformWorkloadIdentity{
+									"file-csi-driver": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"cloud-controller-manager": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"ingress": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"image-registry": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"machine-api": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"cloud-network-config": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"aro-operator": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"disk-csi-driver": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+								},
+							},
+						},
+					},
+				})
+			},
+			wantSystemDataEnriched: true,
+			wantDocuments: func(c *testdatabase.Checker) {
+				c.AddAsyncOperationDocuments(&api.AsyncOperationDocument{
+					OpenShiftClusterKey: strings.ToLower(testdatabase.GetResourcePath(mockGuid, "resourceName")),
+					AsyncOperation: &api.AsyncOperation{
+						InitialProvisioningState: api.ProvisioningStateUpdating,
+						ProvisioningState:        api.ProvisioningStateUpdating,
+					},
+				})
+				c.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{
+					Key: strings.ToLower(testdatabase.GetResourcePath(mockGuid, "resourceName")),
+					OpenShiftCluster: &api.OpenShiftCluster{
+						ID:   testdatabase.GetResourcePath(mockGuid, "resourceName"),
+						Name: "resourceName",
+						Type: "Microsoft.RedHatOpenShift/openShiftClusters",
+						Tags: map[string]string{"tag": "will-be-kept"},
+						Properties: api.OpenShiftClusterProperties{
+							ProvisioningState:     api.ProvisioningStateUpdating,
+							LastProvisioningState: api.ProvisioningStateSucceeded,
+							ClusterProfile: api.ClusterProfile{
+								FipsValidatedModules: api.FipsValidatedModulesDisabled,
+							},
+							IngressProfiles: []api.IngressProfile{{Name: "default"}},
+							WorkerProfiles: []api.WorkerProfile{
+								{
+									Name:             "default",
+									EncryptionAtHost: api.EncryptionAtHostDisabled,
+								},
+							},
+							NetworkProfile: api.NetworkProfile{
+								SoftwareDefinedNetwork: api.SoftwareDefinedNetworkOpenShiftSDN,
+								OutboundType:           api.OutboundTypeLoadbalancer,
+								PreconfiguredNSG:       api.PreconfiguredNSGDisabled,
+								LoadBalancerProfile: &api.LoadBalancerProfile{
+									ManagedOutboundIPs: &api.ManagedOutboundIPs{
+										Count: 1,
+									},
+								},
+							},
+							MasterProfile: api.MasterProfile{
+								EncryptionAtHost: api.EncryptionAtHostDisabled,
+							},
+							OperatorFlags: api.OperatorFlags{},
+							PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
+								PlatformWorkloadIdentities: map[string]api.PlatformWorkloadIdentity{
+									"file-csi-driver": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"cloud-controller-manager": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"ingress": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"image-registry": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"machine-api": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"cloud-network-config": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"aro-operator": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"disk-csi-driver": {
+										ResourceID: mockMiResourceId,
+										ClientID:   mockGuid,
+										ObjectID:   mockGuid,
+									},
+									"new": {
+										ResourceID: mockMiResourceId,
+									},
+								},
+							},
+						},
+					},
+				})
+			},
+			wantEnriched:   []string{testdatabase.GetResourcePath(mockGuid, "resourceName")},
+			wantAsync:      true,
+			wantStatusCode: http.StatusOK,
+			wantResponse: &v20240812preview.OpenShiftCluster{
+				ID:         testdatabase.GetResourcePath(mockGuid, "resourceName"),
+				Name:       "resourceName",
+				Type:       "Microsoft.RedHatOpenShift/openShiftClusters",
+				SystemData: &v20240812preview.SystemData{},
+				Tags:       map[string]string{"tag": "will-be-kept"},
+				Properties: v20240812preview.OpenShiftClusterProperties{
+					ProvisioningState: v20240812preview.ProvisioningStateUpdating,
+					ClusterProfile: v20240812preview.ClusterProfile{
+						FipsValidatedModules: v20240812preview.FipsValidatedModulesDisabled,
+					},
+					IngressProfiles: []v20240812preview.IngressProfile{{Name: "default"}},
+					WorkerProfiles: []v20240812preview.WorkerProfile{
+						{
+							Name:             "default",
+							EncryptionAtHost: v20240812preview.EncryptionAtHostDisabled,
+						},
+					},
 
 					MasterProfile: v20240812preview.MasterProfile{
 						EncryptionAtHost: v20240812preview.EncryptionAtHostDisabled,
@@ -2911,6 +3093,53 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 							},
 						},
 					},
+					PlatformWorkloadIdentityProfile: &v20240812preview.PlatformWorkloadIdentityProfile{
+						PlatformWorkloadIdentities: map[string]v20240812preview.PlatformWorkloadIdentity{
+							"file-csi-driver": {
+								ResourceID: mockMiResourceId,
+								ClientID:   mockGuid,
+								ObjectID:   mockGuid,
+							},
+							"cloud-controller-manager": {
+								ResourceID: mockMiResourceId,
+								ClientID:   mockGuid,
+								ObjectID:   mockGuid,
+							},
+							"ingress": {
+								ResourceID: mockMiResourceId,
+								ClientID:   mockGuid,
+								ObjectID:   mockGuid,
+							},
+							"image-registry": {
+								ResourceID: mockMiResourceId,
+								ClientID:   mockGuid,
+								ObjectID:   mockGuid,
+							},
+							"machine-api": {
+								ResourceID: mockMiResourceId,
+								ClientID:   mockGuid,
+								ObjectID:   mockGuid,
+							},
+							"cloud-network-config": {
+								ResourceID: mockMiResourceId,
+								ClientID:   mockGuid,
+								ObjectID:   mockGuid,
+							},
+							"aro-operator": {
+								ResourceID: mockMiResourceId,
+								ClientID:   mockGuid,
+								ObjectID:   mockGuid,
+							},
+							"disk-csi-driver": {
+								ResourceID: mockMiResourceId,
+								ClientID:   mockGuid,
+								ObjectID:   mockGuid,
+							},
+							"new": {
+								ResourceID: mockMiResourceId,
+							},
+						},
+					},
 				},
 			},
 		},

pkg/operator/deploy/deploy.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -544,18 +545,20 @@ func (o *operator) EnsureUpgradeAnnotation(ctx context.Context) error {
 	upgradeableTo := string(*o.oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo)
 	upgradeableAnnotation := "cloudcredential.openshift.io/upgradeable-to"
 
-	cloudcredentialobject, err := o.operatorcli.OperatorV1().CloudCredentials().Get(ctx, "cluster", metav1.GetOptions{})
+	patch := &metav1.PartialObjectMetadata{
+		ObjectMeta: metav1.ObjectMeta{
+			Annotations: map[string]string{
+				upgradeableAnnotation: upgradeableTo,
+			},
+		},
+	}
+
+	patchBytes, err := json.Marshal(patch)
 	if err != nil {
 		return err
 	}
 
-	if cloudcredentialobject.Annotations == nil {
+	_, err = o.operatorcli.OperatorV1().CloudCredentials().Patch(ctx, "cluster", types.MergePatchType, patchBytes, metav1.PatchOptions{})
-		cloudcredentialobject.Annotations = map[string]string{}
-	}
-
-	cloudcredentialobject.Annotations[upgradeableAnnotation] = upgradeableTo
-
-	_, err = o.operatorcli.OperatorV1().CloudCredentials().Update(ctx, cloudcredentialobject, metav1.UpdateOptions{})
 	if err != nil {
 		return err
 	}

python/az/aro/azext_aro/custom.py

@@ -485,9 +485,10 @@ def aro_update(cmd,
             oc_update.platform_workload_identity_profile = openshiftcluster.PlatformWorkloadIdentityProfile()
 
         if platform_workload_identities is not None:
-            oc_update.platform_workload_identity_profile.platform_workload_identities.update(platform_workload_identities)  # pylint: disable=line-too-long
+            oc_update.platform_workload_identity_profile.platform_workload_identities = dict(platform_workload_identities)  # pylint: disable=line-too-long
 
-        oc_update.platform_workload_identity_profile.upgradeable_to = upgradeable_to
+        if upgradeable_to is not None:
+            oc_update.platform_workload_identity_profile.upgradeable_to = upgradeable_to
 
     if load_balancer_managed_outbound_ip_count is not None:
         oc_update.network_profile = openshiftcluster.NetworkProfile()