From a50f8b1ea3b98d3576a1a01142c1327b943d55a0 Mon Sep 17 00:00:00 2001 From: Amit Date: Sun, 29 Oct 2023 16:39:41 +0530 Subject: [PATCH] Validate required permissions --- pkg/monitor/cluster/cluster.go | 8 +- pkg/monitor/cluster/validatepermissions.go | 46 ++++++++++ .../cluster/validatepermissions_test.go | 92 +++++++++++++++++++ pkg/monitor/worker.go | 48 +++++++++- test/e2e/monitor.go | 4 +- 5 files changed, 194 insertions(+), 4 deletions(-) create mode 100644 pkg/monitor/cluster/validatepermissions.go create mode 100644 pkg/monitor/cluster/validatepermissions_test.go diff --git a/pkg/monitor/cluster/cluster.go b/pkg/monitor/cluster/cluster.go index 22ee2eef06e..3b1684b3370 100644 --- a/pkg/monitor/cluster/cluster.go +++ b/pkg/monitor/cluster/cluster.go @@ -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) @@ -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 @@ -136,6 +138,7 @@ func NewMonitor(log *logrus.Entry, restConfig *rest.Config, oc *api.OpenShiftClu ocpclientset: ocpclientset, hiveclientset: hiveclientset, wg: wg, + validator: validator, }, nil } @@ -210,6 +213,7 @@ func (mon *Monitor) Monitor(ctx context.Context) (errs []error) { mon.emitHiveRegistrationStatus, mon.emitOperatorFlagsAndSupportBanner, mon.emitPucmState, + mon.emitValidatePermissions, mon.emitCertificateExpirationStatuses, mon.emitEtcdCertificateExpiry, mon.emitPrometheusAlerts, // at the end for now because it's the slowest/least reliable diff --git a/pkg/monitor/cluster/validatepermissions.go b/pkg/monitor/cluster/validatepermissions.go new file mode 100644 index 00000000000..b41665b08f9 --- /dev/null +++ b/pkg/monitor/cluster/validatepermissions.go @@ -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 +} diff --git a/pkg/monitor/cluster/validatepermissions_test.go b/pkg/monitor/cluster/validatepermissions_test.go new file mode 100644 index 00000000000..affc114a63f --- /dev/null +++ b/pkg/monitor/cluster/validatepermissions_test.go @@ -0,0 +1,92 @@ +package cluster + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + "errors" + "testing" + + "github.com/golang/mock/gomock" + + "github.com/Azure/ARO-RP/pkg/api" + mock_dynamic "github.com/Azure/ARO-RP/pkg/util/mocks/dynamic" + mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics" +) + +func TestEmitValidatePermissions(t *testing.T) { + 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 + }{ + { + name: "VnetError", + mockVnetError: errors.New("test"), // Mock an error + mockSubnetError: errors.New("test"), // Mock an error + mockValidateDiskEncryptionSetsError: errors.New("test"), // Mock an error + expectedValidateVnet: "test", + expectedSubnet: "test", + expectedValidateDiskEncryptionSets: "test", + }, + { + name: "NoError", + mockVnetError: nil, // No error + }, + } { + 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) + } + }) + } +} diff --git a/pkg/monitor/worker.go b/pkg/monitor/worker.go index 8f0ce09ca33..1befa1c0e57 100644 --- a/pkg/monitor/worker.go +++ b/pkg/monitor/worker.go @@ -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 @@ -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) + 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{ diff --git a/test/e2e/monitor.go b/test/e2e/monitor.go index cb979ca217f..47f51b65aa1 100644 --- a/test/e2e/monitor.go +++ b/test/e2e/monitor.go @@ -13,6 +13,7 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/metrics/noop" "github.com/Azure/ARO-RP/pkg/monitor/cluster" + "github.com/Azure/ARO-RP/pkg/validate/dynamic" ) var _ = Describe("Monitor", func() { @@ -21,9 +22,10 @@ var _ = Describe("Monitor", func() { By("creating a new monitor instance for the test cluster") var wg sync.WaitGroup wg.Add(1) + var validator dynamic.Dynamic mon, err := cluster.NewMonitor(log, clients.RestConfig, &api.OpenShiftCluster{ ID: resourceIDFromEnv(), - }, &noop.Noop{}, nil, true, &wg) + }, &noop.Noop{}, nil, true, &wg, validator) Expect(err).NotTo(HaveOccurred()) By("running the monitor once")