ARO-9382 prevent updating existing platform identities (#3786)

* prevent updating existing platform identities

This adds a check to v20240812preview static validation that raises an
error if either the name or resource ID of an existing platform identity

* allow changing operator identity order

This allows changing the order of platform identities while still
preventing the resource ID and operator name from being changed

* additional platform identity update validation

This prevents removal of a platform identity or changing the identity's
OperatorName and ResourceID at the same time

* detect duplicate operator names in platform workload identity profiles

* use a map instead of a slice

pkg/api/v20240812preview/openshiftcluster.go

@@ -41,6 +41,11 @@ type OpenShiftCluster struct {
 	Identity *Identity `json:"identity,omitempty"`
 }
 
+// UsesWorkloadIdentity checks whether a cluster is a Workload Identity cluster or a Service Principal cluster
+func (oc *OpenShiftCluster) UsesWorkloadIdentity() bool {
+	return oc.Properties.PlatformWorkloadIdentityProfile != nil && oc.Properties.ServicePrincipalProfile == nil
+}
+
 // Tags represents an OpenShift cluster's tags.
 type Tags map[string]string
 

pkg/api/v20240812preview/openshiftcluster_test.go

@@ -0,0 +1,67 @@
+package v20240812preview
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestIsWorkloadIdentity(t *testing.T) {
+	tests := []*struct {
+		name string
+		oc   OpenShiftCluster
+		want bool
+	}{
+		{
+			name: "Cluster is Workload Identity",
+			oc: OpenShiftCluster{
+				Properties: OpenShiftClusterProperties{
+					PlatformWorkloadIdentityProfile: &PlatformWorkloadIdentityProfile{},
+					ServicePrincipalProfile:         nil,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "Cluster is Service Principal",
+			oc: OpenShiftCluster{
+				Properties: OpenShiftClusterProperties{
+					PlatformWorkloadIdentityProfile: nil,
+					ServicePrincipalProfile:         &ServicePrincipalProfile{},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "Cluster is Service Principal",
+			oc: OpenShiftCluster{
+				Properties: OpenShiftClusterProperties{
+					PlatformWorkloadIdentityProfile: nil,
+					ServicePrincipalProfile:         nil,
+				},
+			},
+			want: false,
+		},
+		{
+			name: "Cluster is Service Principal",
+			oc: OpenShiftCluster{
+				Properties: OpenShiftClusterProperties{
+					PlatformWorkloadIdentityProfile: &PlatformWorkloadIdentityProfile{},
+					ServicePrincipalProfile:         &ServicePrincipalProfile{},
+				},
+			},
+			want: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := test.oc.UsesWorkloadIdentity()
+			if got != test.want {
+				t.Error(fmt.Errorf("got != want: %v != %v", got, test.want))
+			}
+		})
+	}
+}

pkg/api/v20240812preview/openshiftcluster_validatestatic.go

@@ -446,6 +446,29 @@ func (sv openShiftClusterStaticValidator) validateDelta(oc, current *OpenShiftCl
 		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, err.Target, err.Message)
 	}
 
+	if current.UsesWorkloadIdentity() {
+		currentIdentities := map[string]PlatformWorkloadIdentity{}
+		for _, i := range current.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
+			currentIdentities[i.OperatorName] = i
+		}
+
+		updateIdentities := map[string]PlatformWorkloadIdentity{}
+		for _, i := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
+			updateIdentities[i.OperatorName] = i
+		}
+
+		for name, currentIdentity := range currentIdentities {
+			updateIdentity, present := updateIdentities[name]
+			// this also validates that existing identities' names haven't changed
+			if !present {
+				return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, "properties.platformWorkloadIdentityProfile.platformWorkloadIdentities", "Operator identity cannot be removed or have its name changed.")
+			}
+			if currentIdentity.ResourceID != updateIdentity.ResourceID {
+				return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, "properties.platformWorkloadIdentityProfile.platformWorkloadIdentities", "Operator identity resource ID cannot be changed.")
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -455,6 +478,9 @@ func (sv openShiftClusterStaticValidator) validatePlatformWorkloadIdentityProfil
 		return nil
 	}
 
+	// collect operator names to check for duplicates
+	operators := map[string]struct{}{}
+
 	// Validate the PlatformWorkloadIdentities
 	for n, p := range pwip.PlatformWorkloadIdentities {
 		resource, err := azcorearm.ParseResourceID(p.ResourceID)
@@ -466,6 +492,11 @@ func (sv openShiftClusterStaticValidator) validatePlatformWorkloadIdentityProfil
 			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("%s.PlatformWorkloadIdentities[%d].resourceID", path, n), "Operator name is empty.")
 		}
 
+		if _, found := operators[p.OperatorName]; found {
+			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("%s.platformWorkloadIdentities", path), "Operator identities cannot have duplicate names.")
+		}
+		operators[p.OperatorName] = struct{}{}
+
 		if resource.ResourceType.Type != "userAssignedIdentities" {
 			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("%s.PlatformWorkloadIdentities[%d].resourceID", path, n), "Resource must be a user assigned identity.")
 		}