Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate required customer service principal permissions #3361

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -207,6 +210,7 @@ func (mon *Monitor) Monitor(ctx context.Context) (errs []error) {
mon.emitStatefulsetStatuses,
mon.emitJobConditions,
mon.emitSummary,
mon.emitValidatePermissions,
mon.emitHiveRegistrationStatus,
mon.emitOperatorFlagsAndSupportBanner,
mon.emitMaintenanceState,
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 {
AldoFusterTurpin marked this conversation as resolved.
Show resolved Hide resolved
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,
AldoFusterTurpin marked this conversation as resolved.
Show resolved Hide resolved
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
}
115 changes: 115 additions & 0 deletions pkg/monitor/cluster/validatepermissions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
package cluster

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

import (
"context"
"fmt"
"os"
"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"
"github.com/Azure/ARO-RP/pkg/validate/dynamic"
)

var (
resourceGroupName = "testGroup"

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `resourceGroupName` is unused (unused)
subscriptionID = "0000000-0000-0000-0000-000000000000"

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `subscriptionID` is unused (unused)
resourceGroupID = "/subscriptions/" + subscriptionID + "/resourceGroups/" + resourceGroupName

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `resourceGroupID` is unused (unused)
vnetName = "testVnet"

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `vnetName` is unused (unused)
vnetID = resourceGroupID + "/providers/Microsoft.Network/virtualNetworks/" + vnetName

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `vnetID` is unused (unused)
masterSubnet = vnetID + "/subnet/masterSubnet"

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `masterSubnet` is unused (unused)
workerSubnet = vnetID + "/subnet/workerSubnet"

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `workerSubnet` is unused (unused)
masterSubnetPath = "properties.masterProfile.subnetId"

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `masterSubnetPath` is unused (unused)
workerSubnetPath = "properties.workerProfile.subnetId"

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `workerSubnetPath` is unused (unused)
masterRtID = resourceGroupID + "/providers/Microsoft.Network/routeTables/masterRt"

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

View workflow job for this annotation

GitHub Actions / golangci-lint

var `masterRtID` is unused (unused)
workerRtID = resourceGroupID + "/providers/Microsoft.Network/routeTables/workerRt"
masterNgID = resourceGroupID + "/providers/Microsoft.Network/natGateways/masterNg"
workerNgID = resourceGroupID + "/providers/Microsoft.Network/natGateways/workerNg"
masterNSGv1 = resourceGroupID + "/providers/Microsoft.Network/networkSecurityGroups/aro-controlplane-nsg"
workerNSGv1 = resourceGroupID + "/providers/Microsoft.Network/networkSecurityGroups/aro-node-nsg"
)

func TestEmitValidatePermissions(t *testing.T) {
ctx := context.Background()
var (
subscriptionID = "af848f0a-dbe3-449f-9ccd-6f23ac6ef9f1"
)
for _, tt := range []struct {
name string
expectedState error
PodCIDR string
location string
ServiceCIDR string
subnet []dynamic.Subnet
SubnetId string
validator func(controller *gomock.Controller) dynamic.Dynamic
}{
{
name: "pass",
location: "eastus",
PodCIDR: "10.128.0.0/14",
ServiceCIDR: "0.0.0.0/0",
subnet: []dynamic.Subnet{},
SubnetId: fmt.Sprintf("/subscriptions/%s/resourceGroups/vnet/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/master", subscriptionID),
expectedState: nil,
validator: func(controller *gomock.Controller) dynamic.Dynamic {
validator := mock_dynamic.NewMockDynamic(controller)
validator.EXPECT().ValidateVnet(ctx, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
return validator
},
AldoFusterTurpin marked this conversation as resolved.
Show resolved Hide resolved
},
} {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()

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

var validatorMock dynamic.Dynamic
// fmt.Println(err)
if tt.validator != nil {
validatorMock = tt.validator(controller)
}

m := mock_metrics.NewMockEmitter(controller)
/*oc := &api.OpenShiftCluster{
Location: tt.location,
Properties: api.OpenShiftClusterProperties{
NetworkProfile: api.NetworkProfile{
PodCIDR: tt.PodCIDR,
ServiceCIDR: tt.ServiceCIDR,
},
MasterProfile: api.MasterProfile{
SubnetID: tt.SubnetId,
},
},
}*/
oc := &api.OpenShiftCluster{}

mon := &Monitor{
m: m,
oc: oc,
validator: validatorMock,
}

err := validatorMock.ValidateVnet(ctx, tt.location, tt.subnet, tt.ServiceCIDR, tt.PodCIDR)
fmt.Println(err.Error())
AldoFusterTurpin marked this conversation as resolved.
Show resolved Hide resolved
os.Exit(-1)

m.EXPECT().EmitGauge("cluster.validateVnet.permissions", int64(1), map[string]string{
"vnetError": "nil",
})
AldoFusterTurpin marked this conversation as resolved.
Show resolved Hide resolved

err1 := mon.emitValidatePermissions(ctx)
if err1 != nil {
t.Fatal(err1)
}
})
}
}
52 changes: 51 additions & 1 deletion pkg/monitor/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,24 @@ import (
"sync"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"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/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"
)

// nsgMonitoringFrequency is used for initializing NSG monitoring ticker
Expand Down Expand Up @@ -266,7 +272,51 @@ func (mon *monitor) workOne(ctx context.Context, log *logrus.Entry, doc *api.Ope

nsgMon := nsg.NewMonitor(log, doc.OpenShiftCluster, mon.env, sub.ID, sub.Subscription.Properties.TenantID, mon.clusterm, dims, &wg, nsgMonTicker.C)

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, env.COMPONENT_MONITOR)

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
Loading