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
Azure · Aug 21, 2024 · db93126 · db93126
1 parent 92a8bfc
commit db93126
Show file tree

Hide file tree

Showing 4 changed files with 168 additions and 0 deletions.
diff --git a/pkg/api/v20240812preview/openshiftcluster.go b/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
 

diff --git a/pkg/api/v20240812preview/openshiftcluster_test.go b/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))
+			}
+		})
+	}
+}
diff --git a/pkg/api/v20240812preview/openshiftcluster_validatestatic.go b/pkg/api/v20240812preview/openshiftcluster_validatestatic.go
@@ -446,6 +446,18 @@ func (sv openShiftClusterStaticValidator) validateDelta(oc, current *OpenShiftCl
 		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, err.Target, err.Message)
 	}
 
+	if current.UsesWorkloadIdentity() {
+		for i := range current.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
+			if current.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].OperatorName != oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].OperatorName {
+				return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities[%d].operatorName", i), "Operator identity name cannot be changed.")
+			}
+
+			if current.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].ResourceID != oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].ResourceID {
+				return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities[%d].resourceID", i), "Operator identity resource ID cannot be changed.")
+			}
+		}
+	}
+
 	return nil
 }
 

diff --git a/pkg/api/v20240812preview/openshiftcluster_validatestatic_test.go b/pkg/api/v20240812preview/openshiftcluster_validatestatic_test.go
@@ -1405,5 +1405,89 @@ func TestOpenShiftClusterStaticValidatePlatformWorkloadIdentityProfile(t *testin
 		},
 	}
 
+	updateTests := []*validateTest{
+		{
+			name: "valid addition of operator identity",
+			current: func(oc *OpenShiftCluster) {
+				oc.Properties.PlatformWorkloadIdentityProfile = &PlatformWorkloadIdentityProfile{
+					PlatformWorkloadIdentities: []PlatformWorkloadIdentity{
+						{
+							OperatorName: "FAKE-OPERATOR",
+							ResourceID:   "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/a-fake-group/providers/Microsoft.RedHatOpenShift/userAssignedIdentities/fake-cluster-name",
+						},
+					},
+				}
+				oc.Identity = &Identity{
+					UserAssignedIdentities: UserAssignedIdentities{
+						"first": {
+							ClientID:    "11111111-1111-1111-1111-111111111111",
+							PrincipalID: "SOMETHING",
+						},
+					},
+				}
+				oc.Properties.ServicePrincipalProfile = nil
+			},
+			modify: func(oc *OpenShiftCluster) {
+				oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities = append(oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities, PlatformWorkloadIdentity{
+					OperatorName: "ANOTHER-FAKE-OPERATOR",
+					ResourceID:   "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/a-fake-group/providers/Microsoft.RedHatOpenShift/userAssignedIdentities/fake-cluster-name-two",
+				})
+			},
+		},
+		{
+			name: "invalid change of operator identity name",
+			current: func(oc *OpenShiftCluster) {
+				oc.Properties.PlatformWorkloadIdentityProfile = &PlatformWorkloadIdentityProfile{
+					PlatformWorkloadIdentities: []PlatformWorkloadIdentity{
+						{
+							OperatorName: "FAKE-OPERATOR",
+							ResourceID:   "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/a-fake-group/providers/Microsoft.RedHatOpenShift/userAssignedIdentities/fake-cluster-name",
+						},
+					},
+				}
+				oc.Identity = &Identity{
+					UserAssignedIdentities: UserAssignedIdentities{
+						"first": {
+							ClientID:    "11111111-1111-1111-1111-111111111111",
+							PrincipalID: "SOMETHING",
+						},
+					},
+				}
+				oc.Properties.ServicePrincipalProfile = nil
+			},
+			modify: func(oc *OpenShiftCluster) {
+				oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[0].OperatorName = "FAKE-OPERATOR-OTHER"
+			},
+			wantErr: "400: PropertyChangeNotAllowed: properties.platformWorkloadIdentityProfile.platformWorkloadIdentities[0].operatorName: Operator identity name cannot be changed.",
+		},
+		{
+			name: "invalid change of operator identity resource ID",
+			current: func(oc *OpenShiftCluster) {
+				oc.Properties.PlatformWorkloadIdentityProfile = &PlatformWorkloadIdentityProfile{
+					PlatformWorkloadIdentities: []PlatformWorkloadIdentity{
+						{
+							OperatorName: "FAKE-OPERATOR",
+							ResourceID:   "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/a-fake-group/providers/Microsoft.RedHatOpenShift/userAssignedIdentities/fake-cluster-name",
+						},
+					},
+				}
+				oc.Identity = &Identity{
+					UserAssignedIdentities: UserAssignedIdentities{
+						"first": {
+							ClientID:    "11111111-1111-1111-1111-111111111111",
+							PrincipalID: "SOMETHING",
+						},
+					},
+				}
+				oc.Properties.ServicePrincipalProfile = nil
+			},
+			modify: func(oc *OpenShiftCluster) {
+				oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[0].ResourceID = "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/a-fake-group/providers/Microsoft.RedHatOpenShift/userAssignedIdentities/fake-cluster-name-other"
+			},
+			wantErr: "400: PropertyChangeNotAllowed: properties.platformWorkloadIdentityProfile.platformWorkloadIdentities[0].resourceID: Operator identity resource ID cannot be changed.",
+		},
+	}
+
 	runTests(t, testModeCreate, createTests)
+	runTests(t, testModeUpdate, updateTests)
 }