ARO-10859 throw an error for create/update flow whenever an unexpected platform identity is found

Author: rajdeepc2792

pkg/cluster/deploybaseresources.go

@@ -28,6 +28,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/env"
 	"github.com/Azure/ARO-RP/pkg/util/arm"
 	"github.com/Azure/ARO-RP/pkg/util/oidcbuilder"
+	"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
 	"github.com/Azure/ARO-RP/pkg/util/pointerutils"
 	"github.com/Azure/ARO-RP/pkg/util/stringutils"
 )
@@ -485,7 +486,7 @@ func (m *manager) federateIdentityCredentials(ctx context.Context) error {
 
 		platformWIRole, exists := platformWIRolesByRoleName[name]
 		if !exists {
-			continue
+			return platformworkloadidentity.GetPlatformWorkloadIdentityMismatchError(m.doc.OpenShiftCluster, platformWIRolesByRoleName)
 		}
 
 		for _, sa := range platformWIRole.ServiceAccounts {

pkg/cluster/deploybaseresources_additional.go

@@ -17,6 +17,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/api"
 	"github.com/Azure/ARO-RP/pkg/util/arm"
 	"github.com/Azure/ARO-RP/pkg/util/azureclient"
+	"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
 	"github.com/Azure/ARO-RP/pkg/util/rbac"
 	"github.com/Azure/ARO-RP/pkg/util/stringutils"
 )
@@ -106,7 +107,7 @@ func (m *manager) platformWorkloadIdentityRBAC() ([]*arm.Resource, error) {
 	for name, identity := range platformWorkloadIdentities {
 		role, exists := platformWIRolesByRoleName[name]
 		if !exists {
-			continue
+			return nil, platformworkloadidentity.GetPlatformWorkloadIdentityMismatchError(m.doc.OpenShiftCluster, platformWIRolesByRoleName)
 		}
 
 		if strings.TrimSpace(identity.ObjectID) == "" {

pkg/cluster/deploybaseresources_test.go

@@ -1811,7 +1811,7 @@ func TestGenerateFederatedIdentityCredentials(t *testing.T) {
 			wantErr: "OIDCIssuer is nil",
 		},
 		{
-			name: "Success - Operator name do not exists in PlatformWorkloadIdentityProfile",
+			name: "Fail - Operator name do not exists in PlatformWorkloadIdentityProfile",
 			oc: &api.OpenShiftClusterDocument{
 				ID: docID,
 				OpenShiftCluster: &api.OpenShiftCluster{
@@ -1857,7 +1857,7 @@ func TestGenerateFederatedIdentityCredentials(t *testing.T) {
 					},
 				)
 			},
-			wantErr: "",
+			wantErr: fmt.Sprintf("400: %s: properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities: There's a mismatch between the required and expected set of platform workload identities for the requested OpenShift minor version '%s or %s'. The required platform workload identities are '[CloudControllerManager]'", api.CloudErrorCodePlatformWorkloadIdentityMismatch, "4.14", "4.15"),
 		},
 	} {
 		uuidGen := deterministicuuid.NewTestUUIDGenerator(deterministicuuid.OPENSHIFT_VERSIONS)

pkg/cluster/workloadidentityresources.go

@@ -81,33 +81,35 @@ func (m *manager) generatePlatformWorkloadIdentitySecrets() ([]*corev1.Secret, e
 
 	secrets := []*corev1.Secret{}
 	for name, identity := range m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
-		if role, ok := roles[name]; ok {
-			// Skip creating a secret for the ARO Operator. This will be
-			// generated by the ARO Operator deployer instead
-			// (see pkg/operator/deploy/deploy.go#generateOperatorIdentitySecret())
-			if role.OperatorName == pkgoperator.OperatorIdentityName {
-				continue
-			}
-
-			secrets = append(secrets, &corev1.Secret{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: corev1.SchemeGroupVersion.Identifier(),
-					Kind:       "Secret",
-				},
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: role.SecretLocation.Namespace,
-					Name:      role.SecretLocation.Name,
-				},
-				Type: corev1.SecretTypeOpaque,
-				StringData: map[string]string{
-					"azure_client_id":            identity.ClientID,
-					"azure_subscription_id":      subscriptionId,
-					"azure_tenant_id":            tenantId,
-					"azure_region":               region,
-					"azure_federated_token_file": azureFederatedTokenFileLocation,
-				},
-			})
+		role, exists := roles[name]
+		if !exists {
+			return nil, platformworkloadidentity.GetPlatformWorkloadIdentityMismatchError(m.doc.OpenShiftCluster, roles)
 		}
+		// Skip creating a secret for the ARO Operator. This will be
+		// generated by the ARO Operator deployer instead
+		// (see pkg/operator/deploy/deploy.go#generateOperatorIdentitySecret())
+		if role.OperatorName == pkgoperator.OperatorIdentityName {
+			continue
+		}
+
+		secrets = append(secrets, &corev1.Secret{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: corev1.SchemeGroupVersion.Identifier(),
+				Kind:       "Secret",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: role.SecretLocation.Namespace,
+				Name:      role.SecretLocation.Name,
+			},
+			Type: corev1.SecretTypeOpaque,
+			StringData: map[string]string{
+				"azure_client_id":            identity.ClientID,
+				"azure_subscription_id":      subscriptionId,
+				"azure_tenant_id":            tenantId,
+				"azure_region":               region,
+				"azure_federated_token_file": azureFederatedTokenFileLocation,
+			},
+		})
 	}
 
 	return secrets, nil

pkg/cluster/workloadidentityresources_test.go

@@ -321,6 +321,7 @@ func TestGeneratePlatformWorkloadIdentitySecrets(t *testing.T) {
 		identities map[string]api.PlatformWorkloadIdentity
 		roles      []api.PlatformWorkloadIdentityRole
 		want       []*corev1.Secret
+		wantErr    string
 	}{
 		{
 			name:       "no identities, no secrets",
@@ -394,17 +395,15 @@ func TestGeneratePlatformWorkloadIdentitySecrets(t *testing.T) {
 			},
 		},
 		{
-			name: "ignores identities with no role present",
+			name: "error - identities with unexpected operator name present",
 			identities: map[string]api.PlatformWorkloadIdentity{
 				"foo": {
 					ClientID: "00f00f00-0f00-0f00-0f00-f00f00f00f00",
 				},
-				"bar": {
-					ClientID: "00ba4ba4-0ba4-0ba4-0ba4-ba4ba4ba4ba4",
-				},
 			},
-			roles: []api.PlatformWorkloadIdentityRole{},
-			want:  []*corev1.Secret{},
+			roles:   []api.PlatformWorkloadIdentityRole{},
+			want:    []*corev1.Secret{},
+			wantErr: fmt.Sprintf("400: %s: properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities: There's a mismatch between the required and expected set of platform workload identities for the requested OpenShift minor version '%s'. The required platform workload identities are '[]'", api.CloudErrorCodePlatformWorkloadIdentityMismatch, "4.14"),
 		},
 		{
 			name: "skips ARO operator identity",
@@ -466,6 +465,9 @@ func TestGeneratePlatformWorkloadIdentitySecrets(t *testing.T) {
 							PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
 								PlatformWorkloadIdentities: tt.identities,
 							},
+							ClusterProfile: api.ClusterProfile{
+								Version: "4.14.40",
+							},
 						},
 					},
 				},
@@ -484,7 +486,7 @@ func TestGeneratePlatformWorkloadIdentitySecrets(t *testing.T) {
 			}
 			got, err := m.generatePlatformWorkloadIdentitySecrets()
 
-			utilerror.AssertErrorMessage(t, err, "")
+			utilerror.AssertErrorMessage(t, err, tt.wantErr)
 			assert.ElementsMatch(t, got, tt.want)
 		})
 	}

pkg/util/platformworkloadidentity/platformworkloadidentityrolesbyversion.go

@@ -85,3 +85,28 @@ func (service *PlatformWorkloadIdentityRolesByVersionService) GetPlatformWorkloa
 	}
 	return platformWorkloadIdentityRolesByRoleName
 }
+
+func GetPlatformWorkloadIdentityMismatchError(oc *api.OpenShiftCluster, platformWorkloadIdentityRolesByRoleName map[string]api.PlatformWorkloadIdentityRole) error {
+	requiredOperatorIdentities := []string{}
+	for _, role := range platformWorkloadIdentityRolesByRoleName {
+		requiredOperatorIdentities = append(requiredOperatorIdentities, role.OperatorName)
+	}
+	currentOpenShiftVersion, err := version.ParseVersion(oc.Properties.ClusterProfile.Version)
+	if err != nil {
+		return err
+	}
+	currentMinorVersion := currentOpenShiftVersion.MinorVersion()
+	v := currentMinorVersion
+	if oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo != nil {
+		upgradeableVersion, err := version.ParseVersion(string(*oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo))
+		if err != nil {
+			return err
+		}
+		upgradeableMinorVersion := upgradeableVersion.MinorVersion()
+		if currentMinorVersion != upgradeableMinorVersion && currentOpenShiftVersion.Lt(upgradeableVersion) {
+			v = fmt.Sprintf("%s or %s", v, upgradeableMinorVersion)
+		}
+	}
+	return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePlatformWorkloadIdentityMismatch,
+		"properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities", "There's a mismatch between the required and expected set of platform workload identities for the requested OpenShift minor version '%s'. The required platform workload identities are '%v'", v, requiredOperatorIdentities)
+}