Skip to content

Commit

Permalink
ARO-4376 add dynamic validation for platformworkloadidentityprofile
Browse files Browse the repository at this point in the history
  • Loading branch information
rajdeepc2792 committed Jun 13, 2024
1 parent 7964440 commit 072ad43
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 18 deletions.
2 changes: 2 additions & 0 deletions pkg/api/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ const (
CloudErrorCodeRequestDisallowedByPolicy = "RequestDisallowedByPolicy"
CloudErrorCodeInvalidNetworkAddress = "InvalidNetworkAddress"
CloudErrorCodeThrottlingLimitExceeded = "ThrottlingLimitExceeded"
CloudErrorCodePlatformWorkloadIdentityMismatch = "CloudErrorCodePlatformWorkloadIdentityMismatch"
CloudErrorCodeInvalidClusterMSICount = "CloudErrorCodeInvalidClusterMSICount"
)

// NewCloudError returns a new CloudError
Expand Down
14 changes: 14 additions & 0 deletions pkg/util/mocks/dynamic/dynamic.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 10 additions & 9 deletions pkg/util/rbac/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@ import (
)

const (
RoleACRPull = "7f951dda-4ed3-4680-a7ca-43fe172d538d"
RoleContributor = "b24988ac-6180-42a0-ab88-20f7382dd24c"
RoleDocumentDBAccountContributor = "5bd9cd88-fe45-4216-938b-f97437e15450"
RoleDocumentDBDataContributor = "00000000-0000-0000-0000-000000000002"
RoleDNSZoneContributor = "befefa01-2a29-4197-83a8-272ff33ce314"
RoleNetworkContributor = "4d97b98b-1d4f-4787-a291-c67834d212e7"
RoleOwner = "8e3af657-a8ff-443c-a75c-2fe8c4bcb635"
RoleReader = "acdd72a7-3385-48ef-bd42-f606fba81ae7"
RoleStorageBlobDataContributor = "ba92f5b4-2d11-453d-a403-e96b0029c9fe"
RoleACRPull = "7f951dda-4ed3-4680-a7ca-43fe172d538d"
RoleContributor = "b24988ac-6180-42a0-ab88-20f7382dd24c"
RoleDocumentDBAccountContributor = "5bd9cd88-fe45-4216-938b-f97437e15450"
RoleDocumentDBDataContributor = "00000000-0000-0000-0000-000000000002"
RoleDNSZoneContributor = "befefa01-2a29-4197-83a8-272ff33ce314"
RoleNetworkContributor = "4d97b98b-1d4f-4787-a291-c67834d212e7"
RoleOwner = "8e3af657-a8ff-443c-a75c-2fe8c4bcb635"
RoleReader = "acdd72a7-3385-48ef-bd42-f606fba81ae7"
RoleStorageBlobDataContributor = "ba92f5b4-2d11-453d-a403-e96b0029c9fe"
RoleAzureRedHatOpenShiftFederatedCredentialRole = "ef318e2a-8334-4a05-9e4a-295a196c6a6e"
)

// ResourceRoleAssignment returns a Resource granting roleID on the resource of
Expand Down
34 changes: 29 additions & 5 deletions pkg/validate/dynamic/dynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ import (
"github.com/Azure/ARO-RP/pkg/env"
"github.com/Azure/ARO-RP/pkg/util/azureclient"
"github.com/Azure/ARO-RP/pkg/util/azureclient/authz/remotepdp"
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armauthorization"
"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/compute"
"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/network"
"github.com/Azure/ARO-RP/pkg/util/stringutils"
"github.com/Azure/ARO-RP/pkg/util/token"
)

Expand Down Expand Up @@ -73,16 +75,19 @@ type Dynamic interface {
ValidateEncryptionAtHost(ctx context.Context, oc *api.OpenShiftCluster) error
ValidateLoadBalancerProfile(ctx context.Context, oc *api.OpenShiftCluster) error
ValidatePreConfiguredNSGs(ctx context.Context, oc *api.OpenShiftCluster, subnets []Subnet) error
ValidatePlatformWorkloadIdentityProfile(ctx context.Context, oc *api.OpenShiftCluster, platformWorkloadIdentityRoles []api.PlatformWorkloadIdentityRole, roleDefinitions armauthorization.RoleDefinitionsClient) error
}

type dynamic struct {
log *logrus.Entry
appID string // for use when reporting an error
authorizerType AuthorizerType
// This represents the Subject for CheckAccess. Could be either FP or SP.
checkAccessSubjectInfoCred azcore.TokenCredential
env env.Interface
azEnv *azureclient.AROEnvironment
checkAccessSubjectInfoCred azcore.TokenCredential
env env.Interface
azEnv *azureclient.AROEnvironment
platformIdentities []api.PlatformWorkloadIdentity
platformIdentitiesActionsMap map[string][]string

virtualNetworks virtualNetworksGetClient
diskEncryptionSets compute.DiskEncryptionSetsClient
Expand All @@ -91,6 +96,7 @@ type dynamic struct {
spNetworkUsage network.UsageClient
loadBalancerBackendAddressPoolsClient network.LoadBalancerBackendAddressPoolsClient
pdpClient remotepdp.RemotePDPClient
roleDefinitions armauthorization.RoleDefinitionsClient
}

type AuthorizerType string
Expand Down Expand Up @@ -386,12 +392,12 @@ func (dv *dynamic) validateNatGatewayPermissions(ctx context.Context, s Subnet)
return err
}

func (dv *dynamic) validateActions(ctx context.Context, r *azure.Resource, actions []string) error {
func (dv *dynamic) validateActionsByOID(ctx context.Context, r *azure.Resource, actions []string, oid *string) error {
// ARM has a 5 minute cache around role assignment creation, so wait one minute longer
timeoutCtx, cancel := context.WithTimeout(ctx, 6*time.Minute)
defer cancel()

c := closure{dv: dv, ctx: ctx, resource: r, actions: actions}
c := closure{dv: dv, ctx: ctx, resource: r, actions: actions, oid: oid}

return wait.PollImmediateUntil(30*time.Second, c.usingCheckAccessV2, timeoutCtx.Done())
}
Expand Down Expand Up @@ -780,6 +786,24 @@ func (dv *dynamic) ValidatePreConfiguredNSGs(ctx context.Context, oc *api.OpenSh
return nil
}

func (dv *dynamic) validateActions(ctx context.Context, r *azure.Resource, actions []string) error {
if dv.platformIdentities != nil {
for _, platformIdentity := range dv.platformIdentities {
actionsToValidate := stringutils.GroupsIntersect(actions, dv.platformIdentitiesActionsMap[platformIdentity.OperatorName])
if len(actionsToValidate) > 0 {
if err := dv.validateActionsByOID(ctx, r, actionsToValidate, &platformIdentity.ObjectID); err != nil {
return err
}
}
}
} else {
if err := dv.validateActionsByOID(ctx, r, actions, nil); err != nil {
return err
}
}
return nil
}

func (dv *dynamic) validateNSGPermissions(ctx context.Context, nsgID string) error {
nsg, err := azure.ParseResourceID(nsgID)
if err != nil {
Expand Down
108 changes: 108 additions & 0 deletions pkg/validate/dynamic/platformworkloadidentityprofile.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package dynamic

import (
"context"
"net/http"

sdkauthorization "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v3"
"github.com/Azure/go-autorest/autorest/azure"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armauthorization"
"github.com/Azure/ARO-RP/pkg/util/rbac"
"github.com/Azure/ARO-RP/pkg/util/stringutils"
)

// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

func (dv *dynamic) ValidatePlatformWorkloadIdentityProfile(ctx context.Context, oc *api.OpenShiftCluster, platformWorkloadIdentityRoles []api.PlatformWorkloadIdentityRole, roleDefinitions armauthorization.RoleDefinitionsClient) error {
dv.log.Print("ValidatePlatformWorkloadIdentityProfile")

platformIdentitiesActionsMap := map[string][]string{}

requiredOperatorIdentities := []string{}
recievedOperatorIdentities := []string{}
for _, role := range platformWorkloadIdentityRoles {
requiredOperatorIdentities = append(requiredOperatorIdentities, role.OperatorName)
}
for _, role := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
recievedOperatorIdentities = append(recievedOperatorIdentities, role.OperatorName)
}
ok := stringutils.ElementsMatch(requiredOperatorIdentities, recievedOperatorIdentities)
if !ok {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePlatformWorkloadIdentityMismatch,
"properties.ValidatePlatformWorkloadIdentityProfile.PlatformWorkloadIdentities", "There's a mismatch between the required and expected set of platform workload identities for the requested OpenShift version '%s'", oc.Properties.ClusterProfile.Version)
}

err := dv.validateClusterMSI(ctx, oc)
if err != nil {
return err
}

for _, role := range platformWorkloadIdentityRoles {
definition, err := dv.roleDefinitions.GetByID(ctx, role.RoleDefinitionID, &sdkauthorization.RoleDefinitionsClientGetByIDOptions{})
if err != nil {
return err
}

if len(definition.Properties.Permissions) <= 0 {
return api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInvalidClusterMSICount,
"dynamic.ValidatePlatformWorkloadIdentityProfile", "No Permissions exists for the role %s", role.RoleDefinitionID)
}

actions := []string{}
for _, action := range definition.Properties.Permissions[0].Actions {
actions = append(actions, *action)
}
platformIdentitiesActionsMap[role.OperatorName] = actions
}

dv.platformIdentities = oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities
dv.platformIdentitiesActionsMap = platformIdentitiesActionsMap
dv.roleDefinitions = roleDefinitions
return nil
}

func (dv *dynamic) validateClusterMSI(ctx context.Context, oc *api.OpenShiftCluster) error {
if len(oc.Identity.UserAssignedIdentities) != 1 {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidClusterMSICount,
"identity.userAssignedIdentities", "Unexpected number of OpenShift Cluster associated User Assigned Identity are provided for the Workload Identity OpenShift cluster, expected one User Assigned Identity")
}

for _, identity := range oc.Identity.UserAssignedIdentities {
return dv.validateClusterMSIPermissions(ctx, identity.PrincipalID, oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities)
}

return nil
}

func (dv *dynamic) validateClusterMSIPermissions(ctx context.Context, oid string, platformIdentities []api.PlatformWorkloadIdentity) error {
definition, err := dv.roleDefinitions.GetByID(ctx, rbac.RoleAzureRedHatOpenShiftFederatedCredentialRole, &sdkauthorization.RoleDefinitionsClientGetByIDOptions{})
if err != nil {
return err
}

if len(definition.Properties.Permissions) <= 0 {
return api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInvalidClusterMSICount,
"dynamic.validateClusterMSIPermissions", "No Permissions exists for the role %s", rbac.RoleAzureRedHatOpenShiftFederatedCredentialRole)
}

actions := []string{}
for _, action := range definition.Properties.Permissions[0].Actions {
actions = append(actions, *action)
}

for _, platformIdentity := range platformIdentities {
pid, err := azure.ParseResourceID(platformIdentity.ResourceID)
if err != nil {
return err
}

err = dv.validateActionsByOID(ctx, &pid, actions, &oid)
if err != nil {
return err
}
}
return nil
}
21 changes: 17 additions & 4 deletions pkg/validate/openshiftcluster_validatedynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/env"
"github.com/Azure/ARO-RP/pkg/util/azureclient/authz/remotepdp"
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armauthorization"
"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
"github.com/Azure/ARO-RP/pkg/validate/dynamic"
)
Expand Down Expand Up @@ -163,10 +164,22 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error {
pdpClient,
)

// SP validation
err = spDynamic.ValidateServicePrincipal(ctx, spClientCred)
if err != nil {
return err
if dv.oc.Properties.PlatformWorkloadIdentityProfile == nil || dv.oc.Properties.ServicePrincipalProfile != nil {
// SP validation
err = spDynamic.ValidateServicePrincipal(ctx, spClientCred)
if err != nil {
return err
}
} else {
// PlatformWorkloadIdentity and ClusterMSIIdentity Validation
roleDefinitions, err := armauthorization.NewRoleDefinitionsClient(dv.env.Environment(), fpClientCred)
if err != nil {
return err
}
err = spDynamic.ValidatePlatformWorkloadIdentityProfile(ctx, dv.oc, dv.platformWorkloadIdentityRolesByVersion.GetPlatformWorkloadIdentityRoles(), roleDefinitions)
if err != nil {
return err
}
}

err = spDynamic.ValidateVnet(
Expand Down

0 comments on commit 072ad43

Please sign in to comment.