From d33d81b9c1e3d0f8e933d07101ddd3c2e00fe2a9 Mon Sep 17 00:00:00 2001 From: Tanmay Satam Date: Fri, 8 Nov 2024 10:31:52 -0500 Subject: [PATCH] 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 --- .../openshiftcluster_convert.go | 27 +- pkg/cluster/install.go | 49 ++- pkg/cluster/workloadidentityresources.go | 14 + pkg/cluster/workloadidentityresources_test.go | 169 +++++++- .../openshiftcluster_putorpatch_test.go | 373 ++++++++++++++---- pkg/operator/deploy/deploy.go | 19 +- python/az/aro/azext_aro/custom.py | 5 +- 7 files changed, 536 insertions(+), 120 deletions(-) diff --git a/pkg/api/v20240812preview/openshiftcluster_convert.go b/pkg/api/v20240812preview/openshiftcluster_convert.go index 6359fa5aa..f63f9d1fd 100644 --- a/pkg/api/v20240812preview/openshiftcluster_convert.go +++ b/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 { - pwi := api.PlatformWorkloadIdentity{ - ClientID: oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k].ClientID, - ObjectID: oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k].ObjectID, - ResourceID: oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k].ResourceID, + for k, identity := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities { + if pwi, exists := out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k]; exists { + pwi.ResourceID = identity.ResourceID + out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k] = pwi + } else { + out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k] = api.PlatformWorkloadIdentity{ + ResourceID: identity.ResourceID, + } } - - out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k] = pwi } } diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go index fed54e876..eb5dc99cc 100644 --- a/pkg/cluster/install.go +++ b/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 - steps.Action(m.createOrUpdateClusterServicePrincipalRBAC), + if m.doc.OpenShiftCluster.UsesWorkloadIdentity() { + 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 { diff --git a/pkg/cluster/workloadidentityresources.go b/pkg/cluster/workloadidentityresources.go index d7989e6f4..df352dbc5 100644 --- a/pkg/cluster/workloadidentityresources.go +++ b/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 diff --git a/pkg/cluster/workloadidentityresources_test.go b/pkg/cluster/workloadidentityresources_test.go index e2e8bbbba..5e0d9d408 100644 --- a/pkg/cluster/workloadidentityresources_test.go +++ b/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 +} diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go index 423bffbb8..898185655 100644 --- a/pkg/frontend/openshiftcluster_putorpatch_test.go +++ b/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": { - ResourceID: mockMiResourceId, - }, - "CloudControllerManager": { - ResourceID: mockMiResourceId, - }, - "ClusterIngressOperator": { - ResourceID: mockMiResourceId, - }, - "ImageRegistryOperator": { - ResourceID: mockMiResourceId, - }, - "MachineApiOperator": { - ResourceID: mockMiResourceId, - }, - "NetworkOperator": { - ResourceID: mockMiResourceId, - }, - "ServiceOperator": { - ResourceID: mockMiResourceId, - }, - "StorageOperator": { - ResourceID: mockMiResourceId, - }, + "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}, }, } }, @@ -2214,30 +2198,14 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) { OperatorFlags: operator.DefaultOperatorFlags(), PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{ PlatformWorkloadIdentities: map[string]api.PlatformWorkloadIdentity{ - "AzureFilesStorageOperator": { - ResourceID: mockMiResourceId, - }, - "CloudControllerManager": { - ResourceID: mockMiResourceId, - }, - "ClusterIngressOperator": { - ResourceID: mockMiResourceId, - }, - "ImageRegistryOperator": { - ResourceID: mockMiResourceId, - }, - "MachineApiOperator": { - ResourceID: mockMiResourceId, - }, - "NetworkOperator": { - ResourceID: mockMiResourceId, - }, - "ServiceOperator": { - ResourceID: mockMiResourceId, - }, - "StorageOperator": { - ResourceID: mockMiResourceId, - }, + "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}, }, }, }, @@ -2278,30 +2246,14 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) { }, PlatformWorkloadIdentityProfile: &v20240812preview.PlatformWorkloadIdentityProfile{ PlatformWorkloadIdentities: map[string]v20240812preview.PlatformWorkloadIdentity{ - "AzureFilesStorageOperator": { - ResourceID: mockMiResourceId, - }, - "CloudControllerManager": { - ResourceID: mockMiResourceId, - }, - "ClusterIngressOperator": { - ResourceID: mockMiResourceId, - }, - "ImageRegistryOperator": { - ResourceID: mockMiResourceId, - }, - "MachineApiOperator": { - ResourceID: mockMiResourceId, - }, - "NetworkOperator": { - ResourceID: mockMiResourceId, - }, - "ServiceOperator": { - ResourceID: mockMiResourceId, - }, - "StorageOperator": { - ResourceID: mockMiResourceId, - }, + "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}, }, }, }, @@ -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, + }, + }, + }, }, }, }, diff --git a/pkg/operator/deploy/deploy.go b/pkg/operator/deploy/deploy.go index e818b269b..7e5f84a63 100644 --- a/pkg/operator/deploy/deploy.go +++ b/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 { - cloudcredentialobject.Annotations = map[string]string{} - } - - cloudcredentialobject.Annotations[upgradeableAnnotation] = upgradeableTo - - _, err = o.operatorcli.OperatorV1().CloudCredentials().Update(ctx, cloudcredentialobject, metav1.UpdateOptions{}) + _, err = o.operatorcli.OperatorV1().CloudCredentials().Patch(ctx, "cluster", types.MergePatchType, patchBytes, metav1.PatchOptions{}) if err != nil { return err } diff --git a/python/az/aro/azext_aro/custom.py b/python/az/aro/azext_aro/custom.py index 3ac04f8d4..18a4f1f62 100644 --- a/python/az/aro/azext_aro/custom.py +++ b/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()