Fix unexpected workload identity update in workload identity clusters

pkg/frontend/openshiftcluster_preflightvalidation.go

@@ -110,6 +110,7 @@ func (f *frontend) _preflightValidation(ctx context.Context, log *logrus.Entry,
 	converter := f.apis[apiVersion].OpenShiftClusterConverter
 	staticValidator := f.apis[apiVersion].OpenShiftClusterStaticValidator
 	ext := converter.ToExternal(oc)
+	converter.ExternalNoReadOnly(ext)
 	if err = json.Unmarshal(raw, &ext); err != nil {
 		log.Warning(err.Error())
 		return api.ValidationResult{
@@ -148,6 +149,19 @@ func (f *frontend) _preflightValidation(ctx context.Context, log *logrus.Entry,
 			}
 		}
 	}
+	converter.ToInternal(ext, oc)
+	if oc.UsesWorkloadIdentity() {
+		if err := f.validatePlatformWorkloadIdentities(oc); err != nil {
+			return api.ValidationResult{
+				Status: api.ValidationStatusFailed,
+				Error: &api.CloudErrorBody{
+					Code:    api.CloudErrorCodeInvalidParameter,
+					Message: err.Error(),
+				},
+			}
+		}
+	}
+
 	return validationSuccess
 }
 

pkg/frontend/openshiftcluster_putorpatch.go

@@ -239,6 +239,12 @@ func (f *frontend) _putOrPatchOpenShiftCluster(ctx context.Context, log *logrus.
 	// is not provided in the header must be preserved
 	f.systemDataClusterDocEnricher(doc, putOrPatchClusterParameters.systemData)
 
+	if doc.OpenShiftCluster.UsesWorkloadIdentity() {
+		if err := f.validatePlatformWorkloadIdentities(doc.OpenShiftCluster); err != nil {
+			return nil, err
+		}
+	}
+
 	if isCreate {
 		err = f.validateInstallVersion(ctx, doc.OpenShiftCluster)
 		if err != nil {

pkg/frontend/platformworkloadidentityrolesets_list.go

@@ -4,7 +4,6 @@ package frontend
 // Licensed under the Apache License 2.0.
 
 import (
-	"context"
 	"encoding/json"
 	"net/http"
 
@@ -25,14 +24,14 @@ func (f *frontend) listPlatformWorkloadIdentityRoleSets(w http.ResponseWriter, r
 		return
 	}
 
-	roleSets := f.getAvailablePlatformWorkloadIdentityRoleSets(ctx)
+	roleSets := f.getAvailablePlatformWorkloadIdentityRoleSets()
 	converter := f.apis[apiVersion].PlatformWorkloadIdentityRoleSetConverter
 
 	b, err := json.MarshalIndent(converter.ToExternalList(roleSets), "", "    ")
 	reply(log, w, nil, b, err)
 }
 
-func (f *frontend) getAvailablePlatformWorkloadIdentityRoleSets(ctx context.Context) []*api.PlatformWorkloadIdentityRoleSet {
+func (f *frontend) getAvailablePlatformWorkloadIdentityRoleSets() []*api.PlatformWorkloadIdentityRoleSet {
 	roleSets := make([]*api.PlatformWorkloadIdentityRoleSet, 0)
 
 	f.platformWorkloadIdentityRoleSetsMu.RLock()

pkg/frontend/platformworkloadidentityrolesets_validate.go

@@ -0,0 +1,27 @@
+package frontend
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"github.com/Azure/ARO-RP/pkg/api"
+	"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
+)
+
+// validatePlatformWorkloadIdentities validates that customer provided platform workload identities are expected
+func (f *frontend) validatePlatformWorkloadIdentities(oc *api.OpenShiftCluster) error {
+	roleSets := f.getAvailablePlatformWorkloadIdentityRoleSets()
+
+	platformWorkloadIdentityRolesByVersionService := platformworkloadidentity.NewPlatformWorkloadIdentityRolesByVersionService()
+	err := platformWorkloadIdentityRolesByVersionService.PopulatePlatformWorkloadIdentityRolesByVersionUsingRoleSets(oc, roleSets)
+	if err != nil {
+		return err
+	}
+	matches := platformWorkloadIdentityRolesByVersionService.MatchesPlatformWorkloadIdentityRoles(oc)
+
+	if !matches {
+		return platformworkloadidentity.GetPlatformWorkloadIdentityMismatchError(oc, platformWorkloadIdentityRolesByVersionService.GetPlatformWorkloadIdentityRolesByRoleName())
+	}
+
+	return nil
+}

pkg/frontend/platformworkloadidentityrolesets_validate_test.go

@@ -0,0 +1,190 @@
+package frontend
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"testing"
+
+	"github.com/Azure/ARO-RP/pkg/api"
+	"github.com/Azure/ARO-RP/pkg/util/pointerutils"
+	utilerror "github.com/Azure/ARO-RP/test/util/error"
+)
+
+func TestValidatePlatformWorkloadIdentities(t *testing.T) {
+	mockMiResourceId := "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/not-a-real-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/not-a-real-mi"
+
+	for _, tt := range []struct {
+		test          string
+		pwi           map[string]api.PlatformWorkloadIdentity
+		version       string
+		upgradeableTo *api.UpgradeableTo
+		wantErr       string
+	}{
+		{
+			test:    "Success - Valid platform workload identities provided",
+			version: defaultVersion,
+			pwi: map[string]api.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},
+			},
+		},
+		{
+			test:    "Success - Valid platform workload identities provided with UpgradeableTo",
+			version: defaultVersion,
+			pwi: map[string]api.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},
+				"extra-new-operator":       {ResourceID: mockMiResourceId},
+			},
+			upgradeableTo: pointerutils.ToPtr(api.UpgradeableTo(getMIWIUpgradeableToVersion().String())),
+		},
+		{
+			test:    "Success - Valid platform workload identities provided with UpgradeableTo smaller than current version",
+			version: defaultVersion,
+			pwi: map[string]api.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},
+			},
+			upgradeableTo: pointerutils.ToPtr(api.UpgradeableTo("4.10.25")),
+		},
+		{
+			test:    "Fail - Not a MIWI Cluster",
+			version: defaultVersion,
+			wantErr: "PopulatePlatformWorkloadIdentityRolesByVersionUsingRoleSets called for a Cluster Service Principal cluster",
+		},
+		{
+			test:    "Fail - Invalid Cluster Version",
+			version: "4.1052",
+			pwi: map[string]api.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},
+			},
+			wantErr: `could not parse version "4.1052"`,
+		},
+		{
+			test:    "Fail - Invalid upgradeableTo version",
+			version: defaultVersion,
+			pwi: map[string]api.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},
+			},
+			upgradeableTo: pointerutils.ToPtr(api.UpgradeableTo("4.1052")),
+			wantErr:       `could not parse version "4.1052"`,
+		},
+		{
+			test:    "Fail - No roleset exists for the older version",
+			version: "4.10.14",
+			pwi: map[string]api.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},
+			},
+			wantErr: "400: InvalidParameter: : No PlatformWorkloadIdentityRoleSet found for the requested or upgradeable OpenShift minor version '4.10'. Please retry with different OpenShift version, and if the issue persists, raise an Azure support ticket",
+		},
+		{
+			test:    "Fail - Unexpected platform workload identity provided",
+			version: defaultVersion,
+			pwi: map[string]api.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},
+				"unexpected-identity":      {ResourceID: mockMiResourceId},
+			},
+			wantErr: unexpectedWorkloadIdentitiesError,
+		},
+		{
+			test:    "Fail - Missing platform workload identity",
+			version: defaultVersion,
+			pwi: map[string]api.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},
+			},
+			wantErr: unexpectedWorkloadIdentitiesError,
+		},
+		{
+			test:    "Fail - Extra platform workload identity provided",
+			version: defaultVersion,
+			pwi: map[string]api.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},
+				"extra-identity":           {ResourceID: mockMiResourceId},
+			},
+			wantErr: unexpectedWorkloadIdentitiesError,
+		},
+	} {
+		t.Run(tt.test, func(t *testing.T) {
+			f := frontend{
+				availablePlatformWorkloadIdentityRoleSets: getPlatformWorkloadIdentityRolesChangeFeed(),
+			}
+
+			oc := &api.OpenShiftCluster{
+				Properties: api.OpenShiftClusterProperties{
+					ClusterProfile: api.ClusterProfile{
+						Version: tt.version,
+					},
+				},
+			}
+
+			if tt.pwi != nil {
+				oc.Properties.PlatformWorkloadIdentityProfile = &api.PlatformWorkloadIdentityProfile{
+					PlatformWorkloadIdentities: tt.pwi,
+					UpgradeableTo:              tt.upgradeableTo,
+				}
+			}
+
+			err := f.validatePlatformWorkloadIdentities(oc)
+			utilerror.AssertErrorMessage(t, err, tt.wantErr)
+		})
+	}
+}