Skip to content

Commit

Permalink
disallow duplicate operator identity resource IDs
Browse files Browse the repository at this point in the history
This adds a step to static validation that checks to see if operator
identities use the same resource ID as any other operator identities and
causes the cluster installation to fail if so. Each operator should have
its own identity.
  • Loading branch information
yithian authored and cadenmarchese committed Nov 1, 2024
1 parent 25eef93 commit 517da11
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
7 changes: 7 additions & 0 deletions pkg/api/v20240812preview/openshiftcluster_validatestatic.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,14 @@ func (sv openShiftClusterStaticValidator) validatePlatformWorkloadIdentityProfil
}

// Validate the PlatformWorkloadIdentities
foundIdentityResourceIDs := map[string]string{}

for name, p := range pwip.PlatformWorkloadIdentities {
if _, present := foundIdentityResourceIDs[strings.ToLower(p.ResourceID)]; present {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("%s.PlatformWorkloadIdentities", path), "ResourceID %s used by multiple identities.", p.ResourceID)
}
foundIdentityResourceIDs[strings.ToLower(p.ResourceID)] = ""

resource, err := azcorearm.ParseResourceID(p.ResourceID)
if err != nil {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("%s.PlatformWorkloadIdentities[%s].resourceID", path, name), "ResourceID %s formatted incorrectly.", p.ResourceID)
Expand Down
38 changes: 38 additions & 0 deletions pkg/api/v20240812preview/openshiftcluster_validatestatic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1379,6 +1379,44 @@ func TestOpenShiftClusterStaticValidatePlatformWorkloadIdentityProfile(t *testin
},
wantErr: "400: InvalidParameter: properties.servicePrincipalProfile: Must provide either an identity or service principal credentials.",
},
{
name: "duplicate operator identities",
modify: func(oc *OpenShiftCluster) {
oc.Identity = &ManagedServiceIdentity{
UserAssignedIdentities: map[string]UserAssignedIdentity{
"first": clusterIdentity1,
},
}
oc.Properties.PlatformWorkloadIdentityProfile = &PlatformWorkloadIdentityProfile{
PlatformWorkloadIdentities: map[string]PlatformWorkloadIdentity{
"FAKE-OPERATOR": platformIdentity1,
"ANOTHER-FAKE-OPERATOR": platformIdentity1,
},
}
oc.Properties.ServicePrincipalProfile = nil
},
wantErr: "400: InvalidParameter: properties.platformWorkloadIdentityProfile.PlatformWorkloadIdentities: ResourceID /subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/a-fake-group/providers/Microsoft.RedHatOpenShift/userAssignedIdentities/fake-cluster-name used by multiple identities.",
},
{
name: "duplicate operator identities, different cases",
modify: func(oc *OpenShiftCluster) {
oc.Identity = &ManagedServiceIdentity{
UserAssignedIdentities: map[string]UserAssignedIdentity{
"first": clusterIdentity1,
},
}
oc.Properties.PlatformWorkloadIdentityProfile = &PlatformWorkloadIdentityProfile{
PlatformWorkloadIdentities: map[string]PlatformWorkloadIdentity{
"FAKE-OPERATOR": platformIdentity1,
"ANOTHER-FAKE-OPERATOR": {
ResourceID: "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/a-fake-group/providers/Microsoft.RedHatOpenShift/userAssignedIdentities/FAKE-CLUSTER-NAME",
},
},
}
oc.Properties.ServicePrincipalProfile = nil
},
wantErr: "400: InvalidParameter: properties.platformWorkloadIdentityProfile.PlatformWorkloadIdentities: ResourceID /subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/a-fake-group/providers/Microsoft.RedHatOpenShift/userAssignedIdentities/fake-cluster-name used by multiple identities.",
},
{
name: "valid UpgradeableTo value",
modify: func(oc *OpenShiftCluster) {
Expand Down

0 comments on commit 517da11

Please sign in to comment.