Skip to content

Commit

Permalink
Validate scope before creating Azure custom roles (#523)
Browse files Browse the repository at this point in the history
  • Loading branch information
otterobert authored Dec 3, 2024
1 parent fd624a2 commit 44393ee
Show file tree
Hide file tree
Showing 10 changed files with 180 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,12 @@ func (a *Agent) ensureCustomRoleForIntent(ctx context.Context, userAssignedIdent
actions := intent.Azure.Actions
dataActions := intent.Azure.DataActions

// Validate that the scope exists before creating the custom role
err := a.ValidateScope(ctx, scope)
if err != nil {
return errors.Wrap(err)
}

customRoleName := a.GenerateCustomRoleName(userAssignedIdentity, scope)
role, found := a.FindCustomRoleByName(ctx, customRoleName)
if found {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/keyvault/armkeyvault"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/msi/armmsi"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
"github.com/google/uuid"
otterizev2alpha1 "github.com/otterize/intents-operator/src/operator/api/v2alpha1"
"github.com/otterize/intents-operator/src/shared/azureagent"
Expand All @@ -30,6 +31,7 @@ type AzureCustomRoleTestCase struct {
type AzureAgentPoliciesCustomRolesSuite struct {
suite.Suite

mockResourcesClient *mock_azureagent.MockAzureARMResourcesClient
mockSubscriptionsClient *mock_azureagent.MockAzureARMSubscriptionsClient
mockResourceGroupsClient *mock_azureagent.MockAzureARMResourcesResourceGroupsClient
mockManagedClustersClient *mock_azureagent.MockAzureARMContainerServiceManagedClustersClient
Expand All @@ -42,6 +44,14 @@ type AzureAgentPoliciesCustomRolesSuite struct {
agent *Agent
}

func (s *AzureAgentPoliciesCustomRolesSuite) expectGetByIDReturnsResource(scope string) {
s.mockResourcesClient.EXPECT().GetByID(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(armresources.ClientGetByIDResponse{
GenericResource: armresources.GenericResource{
ID: &scope,
},
}, nil)
}

func (s *AzureAgentPoliciesCustomRolesSuite) expectListRoleDefinitionsReturnsPager(roles []*armauthorization.RoleDefinition) {
s.mockRoleDefinitionsClient.EXPECT().NewListPager(gomock.Any(), gomock.Any()).Return(azureagent.NewListPager[armauthorization.RoleDefinitionsClientListResponse](
armauthorization.RoleDefinitionsClientListResponse{
Expand Down Expand Up @@ -96,6 +106,8 @@ func (s *AzureAgentPoliciesCustomRolesSuite) expectListKeyVaultsReturnsEmpty() {

func (s *AzureAgentPoliciesCustomRolesSuite) SetupTest() {
controller := gomock.NewController(s.T())

s.mockResourcesClient = mock_azureagent.NewMockAzureARMResourcesClient(controller)
s.mockSubscriptionsClient = mock_azureagent.NewMockAzureARMSubscriptionsClient(controller)
s.mockResourceGroupsClient = mock_azureagent.NewMockAzureARMResourcesResourceGroupsClient(controller)
s.mockManagedClustersClient = mock_azureagent.NewMockAzureARMContainerServiceManagedClustersClient(controller)
Expand All @@ -116,6 +128,7 @@ func (s *AzureAgentPoliciesCustomRolesSuite) SetupTest() {
AKSClusterOIDCIssuerURL: testOIDCIssuerURL,
},
nil,
s.mockResourcesClient,
s.mockSubscriptionsClient,
s.mockResourceGroupsClient,
s.mockManagedClustersClient,
Expand Down Expand Up @@ -201,11 +214,13 @@ var azureCustomRoleTestCases = []AzureCustomRoleTestCase{

func (s *AzureAgentPoliciesCustomRolesSuite) TestAddRolePolicyFromIntents_CustomRoles() {
for _, testCase := range azureCustomRoleTestCases {
targetScope := "/providers/Microsoft.Storage/storageAccounts/test/blobServices/default/containers/container"

s.Run(testCase.Name, func() {
intents := []otterizev2alpha1.Target{
{
Azure: &otterizev2alpha1.AzureTarget{
Scope: "/providers/Microsoft.Storage/storageAccounts/test/blobServices/default/containers/container",
Scope: targetScope,
Roles: testCase.Roles,
Actions: testCase.Actions,
DataActions: testCase.DataActions,
Expand All @@ -231,6 +246,7 @@ func (s *AzureAgentPoliciesCustomRolesSuite) TestAddRolePolicyFromIntents_Custom
// Make sure the custom role is created
var customRoleDefinition armauthorization.RoleDefinition
if testCase.UpdateExpected {
s.expectGetByIDReturnsResource(targetScope)
s.expectCreateOrUpdateRoleDefinitionWriteRoleDefinition(&customRoleDefinition)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
type AzureAgentIdentitiesSuite struct {
suite.Suite

mockResourcesClient *mock_azureagent.MockAzureARMResourcesClient
mockSubscriptionsClient *mock_azureagent.MockAzureARMSubscriptionsClient
mockResourceGroupsClient *mock_azureagent.MockAzureARMResourcesResourceGroupsClient
mockManagedClustersClient *mock_azureagent.MockAzureARMContainerServiceManagedClustersClient
Expand All @@ -31,6 +32,8 @@ type AzureAgentIdentitiesSuite struct {

func (s *AzureAgentIdentitiesSuite) SetupTest() {
controller := gomock.NewController(s.T())

s.mockResourcesClient = mock_azureagent.NewMockAzureARMResourcesClient(controller)
s.mockSubscriptionsClient = mock_azureagent.NewMockAzureARMSubscriptionsClient(controller)
s.mockResourceGroupsClient = mock_azureagent.NewMockAzureARMResourcesResourceGroupsClient(controller)
s.mockManagedClustersClient = mock_azureagent.NewMockAzureARMContainerServiceManagedClustersClient(controller)
Expand All @@ -51,6 +54,7 @@ func (s *AzureAgentIdentitiesSuite) SetupTest() {
AKSClusterOIDCIssuerURL: testOIDCIssuerURL,
},
nil,
s.mockResourcesClient,
s.mockSubscriptionsClient,
s.mockResourceGroupsClient,
s.mockManagedClustersClient,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
type AzureAgentPoliciesKeyVaultSuite struct {
suite.Suite

mockResourcesClient *mock_azureagent.MockAzureARMResourcesClient
mockSubscriptionsClient *mock_azureagent.MockAzureARMSubscriptionsClient
mockResourceGroupsClient *mock_azureagent.MockAzureARMResourcesResourceGroupsClient
mockManagedClustersClient *mock_azureagent.MockAzureARMContainerServiceManagedClustersClient
Expand All @@ -36,6 +37,8 @@ type AzureAgentPoliciesKeyVaultSuite struct {

func (s *AzureAgentPoliciesKeyVaultSuite) SetupTest() {
controller := gomock.NewController(s.T())

s.mockResourcesClient = mock_azureagent.NewMockAzureARMResourcesClient(controller)
s.mockSubscriptionsClient = mock_azureagent.NewMockAzureARMSubscriptionsClient(controller)
s.mockResourceGroupsClient = mock_azureagent.NewMockAzureARMResourcesResourceGroupsClient(controller)
s.mockManagedClustersClient = mock_azureagent.NewMockAzureARMContainerServiceManagedClustersClient(controller)
Expand All @@ -56,6 +59,7 @@ func (s *AzureAgentPoliciesKeyVaultSuite) SetupTest() {
AKSClusterOIDCIssuerURL: testOIDCIssuerURL,
},
nil,
s.mockResourcesClient,
s.mockSubscriptionsClient,
s.mockResourceGroupsClient,
s.mockManagedClustersClient,
Expand Down
24 changes: 22 additions & 2 deletions src/shared/azureagent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Config struct {
type Agent struct {
Conf Config
credentials *azidentity.DefaultAzureCredential
resourceClient AzureARMResourcesClient
subscriptionClient AzureARMSubscriptionsClient
resourceGroupsClient AzureARMResourcesResourceGroupsClient
managedClustersClient AzureARMContainerServiceManagedClustersClient
Expand All @@ -49,6 +50,11 @@ func NewAzureAgent(ctx context.Context, conf Config) (*Agent, error) {
return nil, errors.Wrap(err)
}

armResourcesClientFactory, err := armresources.NewClientFactory(conf.SubscriptionID, credentials, nil)
if err != nil {
return nil, errors.Wrap(err)
}

armsubscriptionsClientFactory, err := armsubscriptions.NewClientFactory(credentials, nil)
if err != nil {
return nil, errors.Wrap(err)
Expand Down Expand Up @@ -79,6 +85,7 @@ func NewAzureAgent(ctx context.Context, conf Config) (*Agent, error) {
return nil, errors.Wrap(err)
}

resourceClient := armResourcesClientFactory.NewClient()
subscriptionClient := armsubscriptionsClientFactory.NewClient()
userAssignedIdentitiesClient := armmsiClientFactory.NewUserAssignedIdentitiesClient()
federatedIdentityCredentialsClient := armmsiClientFactory.NewFederatedIdentityCredentialsClient()
Expand All @@ -87,7 +94,19 @@ func NewAzureAgent(ctx context.Context, conf Config) (*Agent, error) {
managedClustersClient := armcontainerserviceClientFactory.NewManagedClustersClient()
vaultsClient := armkeyvaultClientFactory.NewVaultsClient()

agent := NewAzureAgentFromClients(conf, credentials, subscriptionClient, resourceGroupsClient, managedClustersClient, userAssignedIdentitiesClient, federatedIdentityCredentialsClient, roleDefinitionsClient, roleAssignmentsClient, vaultsClient)
agent := NewAzureAgentFromClients(
conf,
credentials,
resourceClient,
subscriptionClient,
resourceGroupsClient,
managedClustersClient,
userAssignedIdentitiesClient,
federatedIdentityCredentialsClient,
roleDefinitionsClient,
roleAssignmentsClient,
vaultsClient,
)

if err := agent.loadConfDefaults(ctx); err != nil {
return nil, errors.Wrap(err)
Expand All @@ -96,10 +115,11 @@ func NewAzureAgent(ctx context.Context, conf Config) (*Agent, error) {
return agent, nil
}

func NewAzureAgentFromClients(conf Config, credentials *azidentity.DefaultAzureCredential, subscriptionClient AzureARMSubscriptionsClient, resourceGroupsClient AzureARMResourcesResourceGroupsClient, managedClustersClient AzureARMContainerServiceManagedClustersClient, userAssignedIdentitiesClient AzureARMMSIUserAssignedIdentitiesClient, federatedIdentityCredentialsClient AzureARMMSIFederatedIdentityCredentialsClient, roleDefinitionsClient AzureARMAuthorizationRoleDefinitionsClient, roleAssignmentsClient AzureARMAuthorizationRoleAssignmentsClient, vaultsClient AzureARMKeyVaultVaultsClient) *Agent {
func NewAzureAgentFromClients(conf Config, credentials *azidentity.DefaultAzureCredential, resourceClient AzureARMResourcesClient, subscriptionClient AzureARMSubscriptionsClient, resourceGroupsClient AzureARMResourcesResourceGroupsClient, managedClustersClient AzureARMContainerServiceManagedClustersClient, userAssignedIdentitiesClient AzureARMMSIUserAssignedIdentitiesClient, federatedIdentityCredentialsClient AzureARMMSIFederatedIdentityCredentialsClient, roleDefinitionsClient AzureARMAuthorizationRoleDefinitionsClient, roleAssignmentsClient AzureARMAuthorizationRoleAssignmentsClient, vaultsClient AzureARMKeyVaultVaultsClient) *Agent {
return &Agent{
Conf: conf,
credentials: credentials,
resourceClient: resourceClient,
subscriptionClient: subscriptionClient,
resourceGroupsClient: resourceGroupsClient,
managedClustersClient: managedClustersClient,
Expand Down
4 changes: 4 additions & 0 deletions src/shared/azureagent/azureclients.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions"
)

type AzureARMResourcesClient interface {
GetByID(ctx context.Context, resourceID string, apiVersion string, options *armresources.ClientGetByIDOptions) (armresources.ClientGetByIDResponse, error)
}

type AzureARMSubscriptionsClient interface {
Get(ctx context.Context, subscriptionID string, options *armsubscriptions.ClientGetOptions) (armsubscriptions.ClientGetResponse, error)
}
Expand Down
11 changes: 11 additions & 0 deletions src/shared/azureagent/customroles.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ func (a *Agent) GenerateCustomRoleName(uai armmsi.Identity, scope string) string
return agentutils.TruncateHashName(fullName, maxRoleNameLength)
}

func (a *Agent) ValidateScope(ctx context.Context, scope string) error {
res, err := a.resourceClient.GetByID(ctx, scope, "2022-09-01", nil)
if err != nil {
return err
}
if res.GenericResource.ID == nil {
return errors.Errorf("scope %s not found", scope)
}
return nil
}

func (a *Agent) CreateCustomRole(ctx context.Context, scope string, uai armmsi.Identity, actions []v2alpha1.AzureAction, dataActions []v2alpha1.AzureDataAction) (*armauthorization.RoleDefinition, error) {
roleScope := a.getCustomRoleScope()

Expand Down
38 changes: 38 additions & 0 deletions src/shared/azureagent/mocks/azureclients.go

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

44 changes: 37 additions & 7 deletions src/shared/otterizecloud/graphqlclient/schema.graphql

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

Loading

0 comments on commit 44393ee

Please sign in to comment.