diff --git a/pkg/cluster/clustermsi.go b/pkg/cluster/clustermsi.go index 882e10f0dd8..6adb01305d9 100644 --- a/pkg/cluster/clustermsi.go +++ b/pkg/cluster/clustermsi.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net/http" + "strings" "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" @@ -187,13 +188,20 @@ func (m *manager) clusterIdentityIDs(ctx context.Context) error { principalId := *msiCredObj.CredentialsObject.ExplicitIdentities[0].ObjectID m.doc, err = m.db.PatchWithLease(ctx, m.doc.Key, func(doc *api.OpenShiftClusterDocument) error { - identity := doc.OpenShiftCluster.Identity.UserAssignedIdentities[clusterMsiResourceId.String()] - identity.ClientID = clientId - identity.PrincipalID = principalId - - doc.OpenShiftCluster.Identity.UserAssignedIdentities[clusterMsiResourceId.String()] = identity + // we iterate through the existing identities to find the identity matching + // the expected resourceID with casefolding, to ensure we preserve the + // passed-in casing on IDs even if it may be incorrect + for k, v := range doc.OpenShiftCluster.Identity.UserAssignedIdentities { + if strings.EqualFold(k, clusterMsiResourceId.String()) { + v.ClientID = clientId + v.PrincipalID = principalId + + doc.OpenShiftCluster.Identity.UserAssignedIdentities[k] = v + return nil + } + } - return nil + return fmt.Errorf("no entries found matching clusterMsiResourceId") }) return err diff --git a/pkg/cluster/clustermsi_test.go b/pkg/cluster/clustermsi_test.go index 0e99d2b5f81..ce4d3bcd0e8 100644 --- a/pkg/cluster/clustermsi_test.go +++ b/pkg/cluster/clustermsi_test.go @@ -316,6 +316,7 @@ func TestClusterIdentityIDs(t *testing.T) { miName := "aro-cluster-msi" miResourceId := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.ManagedIdentity/userAssignedIdentities/%s", mockGuid, clusterRGName, miName) + miResourceIdIncorrectCasing := fmt.Sprintf("/subscriptions/%s/resourcegroups/%s/providers/Microsoft.ManagedIdentity/userAssignedIdentities/%s", mockGuid, clusterRGName, miName) msiDataPlaneNotFoundError := `failed to get credentials: Request information not available -------------------------------------------------------------------------------- @@ -329,6 +330,36 @@ Response contained no body miObjectId := "99999999-9999-9999-9999-999999999999" placeholderString := "placeholder" + msiDataPlaneValidStub := policy.ClientOptions{ + Transport: dataplane.NewStub([]*dataplane.CredentialsObject{ + { + CredentialsObject: swagger.CredentialsObject{ + ExplicitIdentities: []*swagger.NestedCredentialsObject{ + { + ClientID: &miClientId, + ObjectID: &miObjectId, + ResourceID: &miResourceId, + + ClientSecret: &placeholderString, + TenantID: &placeholderString, + AuthenticationEndpoint: &placeholderString, + CannotRenewAfter: &placeholderString, + ClientSecretURL: &placeholderString, + MtlsAuthenticationEndpoint: &placeholderString, + NotAfter: &placeholderString, + NotBefore: &placeholderString, + RenewAfter: &placeholderString, + CustomClaims: &swagger.CustomClaims{ + XMSAzNwperimid: []*string{&placeholderString}, + XMSAzTm: &placeholderString, + }, + }, + }, + }, + }, + }), + } + for _, tt := range []struct { name string doc *api.OpenShiftClusterDocument @@ -415,35 +446,46 @@ Response contained no body }, }, }, - msiDataplaneStub: policy.ClientOptions{ - Transport: dataplane.NewStub([]*dataplane.CredentialsObject{ - { - CredentialsObject: swagger.CredentialsObject{ - ExplicitIdentities: []*swagger.NestedCredentialsObject{ - { - ClientID: &miClientId, - ObjectID: &miObjectId, - ResourceID: &miResourceId, - - ClientSecret: &placeholderString, - TenantID: &placeholderString, - AuthenticationEndpoint: &placeholderString, - CannotRenewAfter: &placeholderString, - ClientSecretURL: &placeholderString, - MtlsAuthenticationEndpoint: &placeholderString, - NotAfter: &placeholderString, - NotBefore: &placeholderString, - RenewAfter: &placeholderString, - CustomClaims: &swagger.CustomClaims{ - XMSAzNwperimid: []*string{&placeholderString}, - XMSAzTm: &placeholderString, - }, - }, + msiDataplaneStub: msiDataPlaneValidStub, + wantDoc: &api.OpenShiftClusterDocument{ + ID: clusterResourceId, + Key: clusterResourceId, + OpenShiftCluster: &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{}, + }, + Identity: &api.Identity{ + IdentityURL: middleware.MockIdentityURL, + TenantID: mockGuid, + UserAssignedIdentities: api.UserAssignedIdentities{ + miResourceId: api.ClusterUserAssignedIdentity{ + ClientID: miClientId, + PrincipalID: miObjectId, }, }, }, - }), + }, + }, + }, + { + name: "success - existing identity resourceID casing is preserved even if it differs from ARM resourceID parsing", + doc: &api.OpenShiftClusterDocument{ + ID: clusterResourceId, + Key: clusterResourceId, + OpenShiftCluster: &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{}, + }, + Identity: &api.Identity{ + IdentityURL: middleware.MockIdentityURL, + TenantID: mockGuid, + UserAssignedIdentities: api.UserAssignedIdentities{ + miResourceIdIncorrectCasing: api.ClusterUserAssignedIdentity{}, + }, + }, + }, }, + msiDataplaneStub: msiDataPlaneValidStub, wantDoc: &api.OpenShiftClusterDocument{ ID: clusterResourceId, Key: clusterResourceId, @@ -455,7 +497,7 @@ Response contained no body IdentityURL: middleware.MockIdentityURL, TenantID: mockGuid, UserAssignedIdentities: api.UserAssignedIdentities{ - miResourceId: api.ClusterUserAssignedIdentity{ + miResourceIdIncorrectCasing: api.ClusterUserAssignedIdentity{ ClientID: miClientId, PrincipalID: miObjectId, },