Preserve passed-in casing on cluster identity resource IDs

Azure · Oct 1, 2024 · 7744d31 · 7744d31
1 parent 2c87af8
commit 7744d31
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 32 deletions.
diff --git 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
@@ -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,
 							},