Skip to content

Commit

Permalink
Validate required customer service principal permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
rhamitarora committed Jan 19, 2024
1 parent 232d3bc commit d719881
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 4 deletions.
8 changes: 6 additions & 2 deletions pkg/monitor/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
aroclient "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned"
"github.com/Azure/ARO-RP/pkg/util/steps"
"github.com/Azure/ARO-RP/pkg/validate/dynamic"
)

var _ monitoring.Monitor = (*Monitor)(nil)
Expand Down Expand Up @@ -60,10 +61,11 @@ type Monitor struct {
arodl *appsv1.DeploymentList
}

wg *sync.WaitGroup
wg *sync.WaitGroup
validator dynamic.Dynamic
}

func NewMonitor(log *logrus.Entry, restConfig *rest.Config, oc *api.OpenShiftCluster, m metrics.Emitter, hiveRestConfig *rest.Config, hourlyRun bool, wg *sync.WaitGroup) (*Monitor, error) {
func NewMonitor(log *logrus.Entry, restConfig *rest.Config, oc *api.OpenShiftCluster, m metrics.Emitter, hiveRestConfig *rest.Config, hourlyRun bool, wg *sync.WaitGroup, validator dynamic.Dynamic) (*Monitor, error) {
r, err := azure.ParseResourceID(oc.ID)
if err != nil {
return nil, err
Expand Down Expand Up @@ -136,6 +138,7 @@ func NewMonitor(log *logrus.Entry, restConfig *rest.Config, oc *api.OpenShiftClu
ocpclientset: ocpclientset,
hiveclientset: hiveclientset,
wg: wg,
validator: validator,
}, nil
}

Expand Down Expand Up @@ -210,6 +213,7 @@ func (mon *Monitor) Monitor(ctx context.Context) (errs []error) {
mon.emitHiveRegistrationStatus,
mon.emitOperatorFlagsAndSupportBanner,
mon.emitMaintenanceState,
mon.emitValidatePermissions,
mon.emitCertificateExpirationStatuses,
mon.emitEtcdCertificateExpiry,
mon.emitPrometheusAlerts, // at the end for now because it's the slowest/least reliable
Expand Down
46 changes: 46 additions & 0 deletions pkg/monitor/cluster/validatepermissions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package cluster

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

import (
"context"

"github.com/Azure/ARO-RP/pkg/validate/dynamic"
)

/***************************************************************
Monitor the Cluster Service Prinicpal required permissions:
- Network Contributor role on vnet
****************************************************************/

func (mon *Monitor) emitValidatePermissions(ctx context.Context) error {
subnets := []dynamic.Subnet{{
ID: mon.oc.Properties.MasterProfile.SubnetID,
Path: "properties.masterProfile.subnetId",
}}

err := mon.validator.ValidateVnet(ctx, mon.oc.Location, subnets, mon.oc.Properties.NetworkProfile.PodCIDR,
mon.oc.Properties.NetworkProfile.ServiceCIDR)

if err != nil {
mon.emitGauge("cluster.validateVnet.permissions", 1, map[string]string{
"vnetError": err.Error(),
})
}

err = mon.validator.ValidateSubnets(ctx, mon.oc, subnets)
if err != nil {
mon.emitGauge("cluster.validateSubnets.permissions", 1, map[string]string{
"subnetError": err.Error(),
})
}

err = mon.validator.ValidateDiskEncryptionSets(ctx, mon.oc)
if err != nil {
mon.emitGauge("cluster.validateDiskEncryptionSets.permissions", 1, map[string]string{
"diskEncryptionSetError": err.Error(),
})
}
return nil
}
94 changes: 94 additions & 0 deletions pkg/monitor/cluster/validatepermissions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package cluster

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

import (
"context"
"testing"

"github.com/golang/mock/gomock"

"github.com/Azure/ARO-RP/pkg/api"
mock_authorization "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/authorization"
mock_dynamic "github.com/Azure/ARO-RP/pkg/util/mocks/dynamic"
mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics"
)

var (
resourceGroupName = "testGroup"
subscriptionID = "0000000-0000-0000-0000-000000000000"
resourceGroupID = "/subscriptions/" + subscriptionID + "/resourceGroups/" + resourceGroupName
vnetName = "testVnet"
vnetID = resourceGroupID + "/providers/Microsoft.Network/virtualNetworks/" + vnetName
)

func TestEmitValidatePermissions(t *testing.T) {

Check failure on line 26 in pkg/monitor/cluster/validatepermissions_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unnecessary leading newline (whitespace)

for _, tt := range []struct {
name string
mockVnetError error // Mock error for ValidateVnet
mockSubnetError error // Mock error for ValidateVnet
mockValidateDiskEncryptionSetsError error // Mock error for ValidateVnet
expectedValidateVnet string
expectedSubnet string
expectedValidateDiskEncryptionSets string
mocks func(*mock_authorization.MockPermissionsClient, context.CancelFunc)
}{
{
name: "fail: missing permissions",
mockVnetError: nil,
expectedValidateVnet: "400: InvalidServicePrincipalPermissions: : The cluster service principal (Application ID: fff51942-b1f9-4119-9453-aaa922259eb7) does not have Network Contributor role on vnet '" + vnetID + "'.",
},
} {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()

controller := gomock.NewController(t)
defer controller.Finish()

m := mock_metrics.NewMockEmitter(controller)
validator := mock_dynamic.NewMockDynamic(controller)
oc := &api.OpenShiftCluster{}
mon := &Monitor{
m: m,
oc: oc,
validator: validator, // Set the mock validator
}

validator.EXPECT().
ValidateVnet(ctx, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(tt.mockVnetError)

validator.EXPECT().
ValidateSubnets(ctx, gomock.Any(), gomock.Any()).
Return(tt.mockSubnetError)

validator.EXPECT().
ValidateDiskEncryptionSets(ctx, gomock.Any()).
Return(tt.mockValidateDiskEncryptionSetsError)

if tt.mockVnetError == nil {
m.EXPECT().EmitGauge("cluster.validateVnet.permissions", int64(1), map[string]string{
"vnetError": tt.expectedValidateVnet,
})
}

if tt.mockSubnetError != nil {
m.EXPECT().EmitGauge("cluster.validateSubnets.permissions", int64(1), map[string]string{
"subnetError": tt.expectedSubnet,
})
}

if tt.mockValidateDiskEncryptionSetsError != nil {
m.EXPECT().EmitGauge("cluster.validateDiskEncryptionSets.permissions", int64(1), map[string]string{
"diskEncryptionSetError": tt.expectedValidateDiskEncryptionSets,
})
}
err := mon.emitValidatePermissions(ctx)
if err != nil {
t.Fatal(err)
}
})
}
}
48 changes: 47 additions & 1 deletion pkg/monitor/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,25 @@ import (

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2"
"github.com/Azure/go-autorest/autorest/azure"
"github.com/jongio/azidext/go/azidext"
"github.com/sirupsen/logrus"
"k8s.io/client-go/rest"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/env"
"github.com/Azure/ARO-RP/pkg/metrics"
"github.com/Azure/ARO-RP/pkg/monitor/azure/nsg"
"github.com/Azure/ARO-RP/pkg/monitor/cluster"
"github.com/Azure/ARO-RP/pkg/monitor/dimension"
"github.com/Azure/ARO-RP/pkg/monitor/monitoring"
"github.com/Azure/ARO-RP/pkg/util/azureclient/authz/remotepdp"
utillog "github.com/Azure/ARO-RP/pkg/util/log"
"github.com/Azure/ARO-RP/pkg/util/recover"
"github.com/Azure/ARO-RP/pkg/util/restconfig"
"github.com/Azure/ARO-RP/pkg/validate/dynamic"
)

// This function will continue to run until such time as it has a config to add to the global Hive shard map
Expand Down Expand Up @@ -289,7 +294,48 @@ func (mon *monitor) workOne(ctx context.Context, log *logrus.Entry, doc *api.Ope
monitors = append(monitors, nsgMon)
}

c, err := cluster.NewMonitor(log, restConfig, doc.OpenShiftCluster, mon.clusterm, hiveRestConfig, hourlyRun, &wg)
var spClientCred azcore.TokenCredential
var pdpClient remotepdp.RemotePDPClient
spp := doc.OpenShiftCluster.Properties.ServicePrincipalProfile
_env, err := env.NewEnv(ctx, log)

Check failure on line 300 in pkg/monitor/worker.go

View workflow job for this annotation

GitHub Actions / golangci-lint

not enough arguments in call to env.NewEnv
if err != nil {
log.Error(err)
return
}

r, err := azure.ParseResourceID(doc.OpenShiftCluster.ID)
if err != nil {
log.Error(err)
return
}

sub_ := mon.subs[r.SubscriptionID]
tenantID := sub_.Subscription.Properties.TenantID
options := _env.Environment().ClientSecretCredentialOptions()
spTokenCredential, err := azidentity.NewClientSecretCredential(
tenantID, spp.ClientID, string(spp.ClientSecret), options)

if err != nil {
log.Error(err)
return
}

scopes := []string{_env.Environment().ResourceManagerScope}
spAuthorizer := azidext.NewTokenCredentialAdapter(spTokenCredential, scopes)

spDynamic := dynamic.NewValidator(
log,
_env,
_env.Environment(),
sub_.ID,
spAuthorizer,
spp.ClientID,
dynamic.AuthorizerClusterServicePrincipal,
spClientCred,
pdpClient,
)

c, err := cluster.NewMonitor(log, restConfig, doc.OpenShiftCluster, mon.clusterm, hiveRestConfig, hourlyRun, &wg, spDynamic)
if err != nil {
log.Error(err)
mon.m.EmitGauge("monitor.cluster.failedworker", 1, map[string]string{
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var _ = Describe("Monitor", func() {
wg.Add(1)
mon, err := cluster.NewMonitor(log, clients.RestConfig, &api.OpenShiftCluster{
ID: resourceIDFromEnv(),
}, &noop.Noop{}, nil, true, &wg)
}, &noop.Noop{}, nil, true, &wg, nil)
Expect(err).NotTo(HaveOccurred())

By("running the monitor once")
Expand Down

0 comments on commit d719881

Please sign in to comment.