Skip to content

Commit

Permalink
ARO-10859 update dynamic validation to reject the create/update flow …
Browse files Browse the repository at this point in the history 
…for unexpected platform workload identity
  • Loading branch information
@rajdeepc2792
rajdeepc2792 committed Nov 22, 2024
1 parent 6bf634b commit 305400e
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 82 deletions.
125 changes: 50 additions & 75 deletions pkg/validate/dynamic/platformworkloadidentityprofile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armauthorization"
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armmsi"
"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
"github.com/Azure/ARO-RP/pkg/util/rbac"
"github.com/Azure/ARO-RP/pkg/util/stringutils"
"github.com/Azure/ARO-RP/pkg/util/version"
)

// Copyright (c) Microsoft Corporation.
Expand All @@ -34,93 +34,68 @@ func (dv *dynamic) ValidatePlatformWorkloadIdentityProfile(
dv.log.Print("ValidatePlatformWorkloadIdentityProfile")

dv.platformIdentitiesActionsMap = map[string][]string{}
dv.platformIdentities = map[string]api.PlatformWorkloadIdentity{}

for k, pwi := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
_, ok := platformWorkloadIdentityRolesByRoleName[k]
if ok {
dv.platformIdentitiesActionsMap[k] = nil
dv.platformIdentities[k] = pwi

identityResourceId, err := azure.ParseResourceID(pwi.ResourceID)
if err != nil {
return err
}

// validate federated identity credentials
federatedCredentials, err := clusterMsiFederatedIdentityCredentials.List(ctx, identityResourceId.ResourceGroup, identityResourceId.ResourceName, &sdkmsi.FederatedIdentityCredentialsClientListOptions{})
if err != nil {
return err
}

for _, federatedCredential := range federatedCredentials {
switch {
case federatedCredential == nil,
federatedCredential.Name == nil,
federatedCredential.Properties == nil:
return fmt.Errorf("received invalid federated credential")
case oc.Properties.ProvisioningState == api.ProvisioningStateCreating:
return api.NewCloudError(
http.StatusBadRequest,
api.CloudErrorCodePlatformWorkloadIdentityContainsInvalidFederatedCredential,
fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities.%s.resourceId", k),
"Unexpected federated credential '%s' found on platform workload identity '%s' used for role '%s'. Please ensure this identity is only used for this cluster and does not have any existing federated identity credentials.",
*federatedCredential.Name,
pwi.ResourceID,
k,
)
case len(federatedCredential.Properties.Audiences) != 1,
*federatedCredential.Properties.Audiences[0] != expectedAudience,
federatedCredential.Properties.Issuer == nil,
*federatedCredential.Properties.Issuer != string(*oc.Properties.ClusterProfile.OIDCIssuer):
return api.NewCloudError(
http.StatusBadRequest,
api.CloudErrorCodePlatformWorkloadIdentityContainsInvalidFederatedCredential,
fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities.%s.resourceId", k),
"Unexpected federated credential '%s' found on platform workload identity '%s' used for role '%s'. Please ensure only federated credentials provisioned by the ARO service for this cluster are present.",
*federatedCredential.Name,
pwi.ResourceID,
k,
)
}
}
}
}
dv.platformIdentities = oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities

// Check if any required platform identity is missing
if len(dv.platformIdentities) != len(platformWorkloadIdentityRolesByRoleName) {
requiredOperatorIdentities := []string{}
for _, role := range platformWorkloadIdentityRolesByRoleName {
requiredOperatorIdentities = append(requiredOperatorIdentities, role.OperatorName)
return platformworkloadidentity.GetPlatformWorkloadIdentityMismatchError(oc, platformWorkloadIdentityRolesByRoleName)
}

for k, pwi := range dv.platformIdentities {
role, exists := platformWorkloadIdentityRolesByRoleName[k]
if !exists {
return platformworkloadidentity.GetPlatformWorkloadIdentityMismatchError(oc, platformWorkloadIdentityRolesByRoleName)
}
currentOpenShiftVersion, err := version.ParseVersion(oc.Properties.ClusterProfile.Version)

roleDefinitionID := stringutils.LastTokenByte(role.RoleDefinitionID, '/')
actions, err := getActionsForRoleDefinition(ctx, roleDefinitionID, roleDefinitions)
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)
}
dv.platformIdentitiesActionsMap[role.OperatorName] = actions

identityResourceId, err := azure.ParseResourceID(pwi.ResourceID)
if err != nil {
return err
}
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)
}

for _, role := range platformWorkloadIdentityRolesByRoleName {
roleDefinitionID := stringutils.LastTokenByte(role.RoleDefinitionID, '/')
actions, err := getActionsForRoleDefinition(ctx, roleDefinitionID, roleDefinitions)
// validate federated identity credentials
federatedCredentials, err := clusterMsiFederatedIdentityCredentials.List(ctx, identityResourceId.ResourceGroup, identityResourceId.ResourceName, &sdkmsi.FederatedIdentityCredentialsClientListOptions{})
if err != nil {
return err
}

dv.platformIdentitiesActionsMap[role.OperatorName] = actions
for _, federatedCredential := range federatedCredentials {
switch {
case federatedCredential == nil,
federatedCredential.Name == nil,
federatedCredential.Properties == nil:
return fmt.Errorf("received invalid federated credential")
case oc.Properties.ProvisioningState == api.ProvisioningStateCreating:
return api.NewCloudError(
http.StatusBadRequest,
api.CloudErrorCodePlatformWorkloadIdentityContainsInvalidFederatedCredential,
fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities.%s.resourceId", k),
"Unexpected federated credential '%s' found on platform workload identity '%s' used for role '%s'. Please ensure this identity is only used for this cluster and does not have any existing federated identity credentials.",
*federatedCredential.Name,
pwi.ResourceID,
k,
)
case len(federatedCredential.Properties.Audiences) != 1,
*federatedCredential.Properties.Audiences[0] != expectedAudience,
federatedCredential.Properties.Issuer == nil,
*federatedCredential.Properties.Issuer != string(*oc.Properties.ClusterProfile.OIDCIssuer):
return api.NewCloudError(
http.StatusBadRequest,
api.CloudErrorCodePlatformWorkloadIdentityContainsInvalidFederatedCredential,
fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities.%s.resourceId", k),
"Unexpected federated credential '%s' found on platform workload identity '%s' used for role '%s'. Please ensure only federated credentials provisioned by the ARO service for this cluster are present.",
*federatedCredential.Name,
pwi.ResourceID,
k,
)
}
}
}

return nil
Expand Down
20 changes: 13 additions & 7 deletions pkg/validate/dynamic/platformworkloadidentityprofile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,6 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
expectedPlatformIdentity1FederatedCredName := platformworkloadidentity.GetPlatformWorkloadIdentityFederatedCredName(clusterResourceId, platformIdentity1ResourceId, platformIdentity1SAName)
expectedOIDCIssuer := "https://fakeissuer.fakedomain/fakecluster"
platformWorkloadIdentities := map[string]api.PlatformWorkloadIdentity{
"Dummy2": {
ResourceID: platformIdentity1,
},
"Dummy1": {
ResourceID: platformIdentity1,
},
Expand Down Expand Up @@ -532,6 +529,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
},
},
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
Return(nil, fmt.Errorf("something unexpected occurred"))
},
Expand Down Expand Up @@ -560,6 +558,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
},
},
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
Return([]*sdkmsi.FederatedIdentityCredential{
{
Expand Down Expand Up @@ -604,6 +603,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
},
},
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
Return([]*sdkmsi.FederatedIdentityCredential{
{
Expand Down Expand Up @@ -648,6 +648,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
},
},
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
Return([]*sdkmsi.FederatedIdentityCredential{
{
Expand Down Expand Up @@ -692,6 +693,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
},
},
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
Return([]*sdkmsi.FederatedIdentityCredential{
{
Expand Down Expand Up @@ -935,7 +937,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
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 '[Dummy3]'", api.CloudErrorCodePlatformWorkloadIdentityMismatch, "4.14"),
},
{
name: "Fail - Mismatch between desired and provided platform Identities - count mismatch 2",
name: "Fail - Mismatch between desired and provided platform Identities - count mismatch and operator missing",
platformIdentityRoles: validRolesForVersion,
oc: &api.OpenShiftCluster{
ID: clusterID,
Expand Down Expand Up @@ -1013,19 +1015,23 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
Properties: api.OpenShiftClusterProperties{
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
PlatformWorkloadIdentities: map[string]api.PlatformWorkloadIdentity{
"Dummy2": {
ResourceID: "Invalid UUID",
},
"Dummy1": {
ResourceID: "Invalid UUID",
},
},
},
ClusterProfile: api.ClusterProfile{
Version: openShiftVersion,
OIDCIssuer: pointerutils.ToPtr(api.OIDCIssuer(expectedOIDCIssuer)),
},
},
Identity: &api.ManagedServiceIdentity{
UserAssignedIdentities: clusterMSI,
},
},
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
},
wantErr: "parsing failed for Invalid UUID. Invalid resource Id format",
},
{
Expand Down

0 comments on commit 305400e

Please sign in to comment.