Skip to content

Commit da1decc

Browse files
rajdeepc2792rajdeepc2792
committed
ARO-10859 update dynamic validation to reject the create/update flow for unexpected platform workload identity
1 parent a1dd014 commit da1decc

File tree

2 files changed

+63
-82
lines changed

2 files changed

+63
-82
lines changed

‎pkg/validate/dynamic/platformworkloadidentityprofile.go

Lines changed: 50 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ import (
1212
"github.com/Azure/ARO-RP/pkg/api"
1313
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armauthorization"
1414
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armmsi"
15+
"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
1516
"github.com/Azure/ARO-RP/pkg/util/rbac"
1617
"github.com/Azure/ARO-RP/pkg/util/stringutils"
17-
"github.com/Azure/ARO-RP/pkg/util/version"
1818
)
1919

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

3636
dv.platformIdentitiesActionsMap = map[string][]string{}
37-
dv.platformIdentities = map[string]api.PlatformWorkloadIdentity{}
38-
39-
for k, pwi := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
40-
_, ok := platformWorkloadIdentityRolesByRoleName[k]
41-
if ok {
42-
dv.platformIdentitiesActionsMap[k] = nil
43-
dv.platformIdentities[k] = pwi
44-
45-
identityResourceId, err := azure.ParseResourceID(pwi.ResourceID)
46-
if err != nil {
47-
return err
48-
}
49-
50-
// validate federated identity credentials
51-
federatedCredentials, err := clusterMsiFederatedIdentityCredentials.List(ctx, identityResourceId.ResourceGroup, identityResourceId.ResourceName, &sdkmsi.FederatedIdentityCredentialsClientListOptions{})
52-
if err != nil {
53-
return err
54-
}
55-
56-
for _, federatedCredential := range federatedCredentials {
57-
switch {
58-
case federatedCredential == nil,
59-
federatedCredential.Name == nil,
60-
federatedCredential.Properties == nil:
61-
return fmt.Errorf("received invalid federated credential")
62-
case oc.Properties.ProvisioningState == api.ProvisioningStateCreating:
63-
return api.NewCloudError(
64-
http.StatusBadRequest,
65-
api.CloudErrorCodePlatformWorkloadIdentityContainsInvalidFederatedCredential,
66-
fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities.%s.resourceId", k),
67-
"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.",
68-
*federatedCredential.Name,
69-
pwi.ResourceID,
70-
k,
71-
)
72-
case len(federatedCredential.Properties.Audiences) != 1,
73-
*federatedCredential.Properties.Audiences[0] != expectedAudience,
74-
federatedCredential.Properties.Issuer == nil,
75-
*federatedCredential.Properties.Issuer != string(*oc.Properties.ClusterProfile.OIDCIssuer):
76-
return api.NewCloudError(
77-
http.StatusBadRequest,
78-
api.CloudErrorCodePlatformWorkloadIdentityContainsInvalidFederatedCredential,
79-
fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities.%s.resourceId", k),
80-
"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.",
81-
*federatedCredential.Name,
82-
pwi.ResourceID,
83-
k,
84-
)
85-
}
86-
}
87-
}
88-
}
37+
dv.platformIdentities = oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities
8938

9039
// Check if any required platform identity is missing
9140
if len(dv.platformIdentities) != len(platformWorkloadIdentityRolesByRoleName) {
92-
requiredOperatorIdentities := []string{}
93-
for _, role := range platformWorkloadIdentityRolesByRoleName {
94-
requiredOperatorIdentities = append(requiredOperatorIdentities, role.OperatorName)
41+
return platformworkloadidentity.GetPlatformWorkloadIdentityMismatchError(oc, platformWorkloadIdentityRolesByRoleName)
42+
}
43+
44+
for k, pwi := range dv.platformIdentities {
45+
role, exists := platformWorkloadIdentityRolesByRoleName[k]
46+
if !exists {
47+
return platformworkloadidentity.GetPlatformWorkloadIdentityMismatchError(oc, platformWorkloadIdentityRolesByRoleName)
9548
}
96-
currentOpenShiftVersion, err := version.ParseVersion(oc.Properties.ClusterProfile.Version)
49+
50+
roleDefinitionID := stringutils.LastTokenByte(role.RoleDefinitionID, '/')
51+
actions, err := getActionsForRoleDefinition(ctx, roleDefinitionID, roleDefinitions)
9752
if err != nil {
9853
return err
9954
}
100-
currentMinorVersion := currentOpenShiftVersion.MinorVersion()
101-
v := currentMinorVersion
102-
if oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo != nil {
103-
upgradeableVersion, err := version.ParseVersion(string(*oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo))
104-
if err != nil {
105-
return err
106-
}
107-
upgradeableMinorVersion := upgradeableVersion.MinorVersion()
108-
if currentMinorVersion != upgradeableMinorVersion && currentOpenShiftVersion.Lt(upgradeableVersion) {
109-
v = fmt.Sprintf("%s or %s", v, upgradeableMinorVersion)
110-
}
55+
dv.platformIdentitiesActionsMap[role.OperatorName] = actions
56+
57+
identityResourceId, err := azure.ParseResourceID(pwi.ResourceID)
58+
if err != nil {
59+
return err
11160
}
112-
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePlatformWorkloadIdentityMismatch,
113-
"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)
114-
}
11561

116-
for _, role := range platformWorkloadIdentityRolesByRoleName {
117-
roleDefinitionID := stringutils.LastTokenByte(role.RoleDefinitionID, '/')
118-
actions, err := getActionsForRoleDefinition(ctx, roleDefinitionID, roleDefinitions)
62+
// validate federated identity credentials
63+
federatedCredentials, err := clusterMsiFederatedIdentityCredentials.List(ctx, identityResourceId.ResourceGroup, identityResourceId.ResourceName, &sdkmsi.FederatedIdentityCredentialsClientListOptions{})
11964
if err != nil {
12065
return err
12166
}
12267

123-
dv.platformIdentitiesActionsMap[role.OperatorName] = actions
68+
for _, federatedCredential := range federatedCredentials {
69+
switch {
70+
case federatedCredential == nil,
71+
federatedCredential.Name == nil,
72+
federatedCredential.Properties == nil:
73+
return fmt.Errorf("received invalid federated credential")
74+
case oc.Properties.ProvisioningState == api.ProvisioningStateCreating:
75+
return api.NewCloudError(
76+
http.StatusBadRequest,
77+
api.CloudErrorCodePlatformWorkloadIdentityContainsInvalidFederatedCredential,
78+
fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities.%s.resourceId", k),
79+
"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.",
80+
*federatedCredential.Name,
81+
pwi.ResourceID,
82+
k,
83+
)
84+
case len(federatedCredential.Properties.Audiences) != 1,
85+
*federatedCredential.Properties.Audiences[0] != expectedAudience,
86+
federatedCredential.Properties.Issuer == nil,
87+
*federatedCredential.Properties.Issuer != string(*oc.Properties.ClusterProfile.OIDCIssuer):
88+
return api.NewCloudError(
89+
http.StatusBadRequest,
90+
api.CloudErrorCodePlatformWorkloadIdentityContainsInvalidFederatedCredential,
91+
fmt.Sprintf("properties.platformWorkloadIdentityProfile.platformWorkloadIdentities.%s.resourceId", k),
92+
"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.",
93+
*federatedCredential.Name,
94+
pwi.ResourceID,
95+
k,
96+
)
97+
}
98+
}
12499
}
125100

126101
return nil

‎pkg/validate/dynamic/platformworkloadidentityprofile_test.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,6 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
263263
expectedPlatformIdentity1FederatedCredName := platformworkloadidentity.GetPlatformWorkloadIdentityFederatedCredName(clusterResourceId, platformIdentity1ResourceId, platformIdentity1SAName)
264264
expectedOIDCIssuer := "https://fakeissuer.fakedomain/fakecluster"
265265
platformWorkloadIdentities := map[string]api.PlatformWorkloadIdentity{
266-
"Dummy2": {
267-
ResourceID: platformIdentity1,
268-
},
269266
"Dummy1": {
270267
ResourceID: platformIdentity1,
271268
},
@@ -532,6 +529,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
532529
},
533530
},
534531
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
532+
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
535533
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
536534
Return(nil, fmt.Errorf("something unexpected occurred"))
537535
},
@@ -560,6 +558,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
560558
},
561559
},
562560
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
561+
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
563562
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
564563
Return([]*sdkmsi.FederatedIdentityCredential{
565564
{
@@ -604,6 +603,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
604603
},
605604
},
606605
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
606+
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
607607
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
608608
Return([]*sdkmsi.FederatedIdentityCredential{
609609
{
@@ -648,6 +648,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
648648
},
649649
},
650650
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
651+
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
651652
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
652653
Return([]*sdkmsi.FederatedIdentityCredential{
653654
{
@@ -692,6 +693,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
692693
},
693694
},
694695
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
696+
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
695697
federatedIdentityCredentials.EXPECT().List(gomock.Any(), gomock.Eq(platformIdentity1ResourceId.ResourceGroup), gomock.Eq(platformIdentity1ResourceId.ResourceName), gomock.Any()).
696698
Return([]*sdkmsi.FederatedIdentityCredential{
697699
{
@@ -935,7 +937,7 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
935937
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"),
936938
},
937939
{
938-
name: "Fail - Mismatch between desired and provided platform Identities - count mismatch 2",
940+
name: "Fail - Mismatch between desired and provided platform Identities - count mismatch and operator missing",
939941
platformIdentityRoles: validRolesForVersion,
940942
oc: &api.OpenShiftCluster{
941943
ID: clusterID,
@@ -1013,19 +1015,23 @@ func TestValidatePlatformWorkloadIdentityProfile(t *testing.T) {
10131015
Properties: api.OpenShiftClusterProperties{
10141016
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
10151017
PlatformWorkloadIdentities: map[string]api.PlatformWorkloadIdentity{
1016-
"Dummy2": {
1017-
ResourceID: "Invalid UUID",
1018-
},
10191018
"Dummy1": {
10201019
ResourceID: "Invalid UUID",
10211020
},
10221021
},
10231022
},
1023+
ClusterProfile: api.ClusterProfile{
1024+
Version: openShiftVersion,
1025+
OIDCIssuer: pointerutils.ToPtr(api.OIDCIssuer(expectedOIDCIssuer)),
1026+
},
10241027
},
10251028
Identity: &api.ManagedServiceIdentity{
10261029
UserAssignedIdentities: clusterMSI,
10271030
},
10281031
},
1032+
mocks: func(roleDefinitions *mock_armauthorization.MockRoleDefinitionsClient, federatedIdentityCredentials *mock_armmsi.MockFederatedIdentityCredentialsClient) {
1033+
roleDefinitions.EXPECT().GetByID(ctx, gomock.Any(), &sdkauthorization.RoleDefinitionsClientGetByIDOptions{}).AnyTimes().Return(platformIdentityRequiredPermissions, nil)
1034+
},
10291035
wantErr: "parsing failed for Invalid UUID. Invalid resource Id format",
10301036
},
10311037
{

0 commit comments

Comments
 (0)