Skip to content

Commit

Permalink
Prevent duplicate azure custom role update (#533)
Browse files Browse the repository at this point in the history
  • Loading branch information
otterobert authored Dec 15, 2024
1 parent 8e21903 commit b367996
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 1 deletion.
15 changes: 14 additions & 1 deletion src/shared/azureagent/customroles.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/otterize/intents-operator/src/shared/agentutils"
"github.com/otterize/intents-operator/src/shared/errors"
"github.com/samber/lo"
"github.com/sirupsen/logrus"
"strings"
)

Expand Down Expand Up @@ -60,7 +61,7 @@ func (a *Agent) CreateCustomRole(ctx context.Context, scope string, uai armmsi.I
},
}

// create the custom role
logrus.WithField("name", *uai.Name).Debug("Creating custom role for uai")
resp, err := a.roleDefinitionsClient.CreateOrUpdate(ctx, roleScope, id, roleDefinition, nil)
if err != nil {
return nil, errors.Wrap(err)
Expand All @@ -70,6 +71,10 @@ func (a *Agent) CreateCustomRole(ctx context.Context, scope string, uai armmsi.I
}

func (a *Agent) UpdateCustomRole(ctx context.Context, scope string, role *armauthorization.RoleDefinition, actions []v2alpha1.AzureAction, dataActions []v2alpha1.AzureDataAction) error {
if role == nil || role.Properties == nil || role.Properties.Permissions == nil || len(role.Properties.Permissions) == 0 {
return errors.Errorf("role definition is nil or does not have any permissions")
}

roleScope := a.getSubscriptionScope(scope)

formattedActions := lo.Map(actions, func(action v2alpha1.AzureAction, _ int) *string {
Expand All @@ -79,13 +84,20 @@ func (a *Agent) UpdateCustomRole(ctx context.Context, scope string, role *armaut
return to.Ptr(string(action))
})

// Compare the actions and dataActions to the existing role definition
if IsEqualAzureActions(role.Properties.Permissions[0].Actions, formattedActions) && IsEqualAzureActions(role.Properties.Permissions[0].DataActions, formattedDataActions) {
logrus.Debugf("Role %s already has the correct permissions", *role.Name)
return nil
}

role.Properties.Permissions = []*armauthorization.Permission{
{
Actions: formattedActions,
DataActions: formattedDataActions,
},
}

logrus.WithField("name", *role.Name).Debug("Updating custom role")
_, err := a.roleDefinitionsClient.CreateOrUpdate(ctx, roleScope, *role.Name, *role, nil)
if err != nil {
return errors.Wrap(err)
Expand Down Expand Up @@ -115,6 +127,7 @@ func (a *Agent) FindCustomRoleByName(ctx context.Context, scope string, name str
func (a *Agent) DeleteCustomRole(ctx context.Context, scope string, roleDefinitionID string) error {
roleScope := a.getSubscriptionScope(scope)

logrus.WithField("id", roleDefinitionID).Debug("Deleting custom role")
_, err := a.roleDefinitionsClient.Delete(ctx, roleScope, roleDefinitionID, nil)
if err != nil {
if azureerrors.IsNotFoundErr(err) {
Expand Down
26 changes: 26 additions & 0 deletions src/shared/azureagent/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package azureagent

import (
"github.com/google/go-cmp/cmp"
"slices"
)

func IsEqualAzureActions(a, b []*string) bool {
slices.SortFunc(a, func(a, b *string) int {
return compareStrings(a, b)
})
slices.SortFunc(b, func(a, b *string) int {
return compareStrings(a, b)
})

return cmp.Equal(a, b)
}

func compareStrings(a, b *string) int {
if *a < *b {
return -1
} else if *a > *b {
return 1
}
return 0
}
51 changes: 51 additions & 0 deletions src/shared/azureagent/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package azureagent

import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/stretchr/testify/suite"
"testing"
)

type AzureAgentUtilsTestSuite struct {
suite.Suite
}

type IsEqualTestCase struct {
a []*string
b []*string
expected bool
}

var testCases = []IsEqualTestCase{
{
a: []*string{to.Ptr("a"), to.Ptr("b"), to.Ptr("c")},
b: []*string{to.Ptr("a"), to.Ptr("b"), to.Ptr("c")},
expected: true,
},
{
a: []*string{to.Ptr("a"), to.Ptr("b"), to.Ptr("c")},
b: []*string{to.Ptr("c"), to.Ptr("b"), to.Ptr("a")},
expected: true,
},
{
a: []*string{to.Ptr("a"), to.Ptr("b"), to.Ptr("c")},
b: []*string{to.Ptr("a"), to.Ptr("b"), to.Ptr("d")},
expected: false,
},
{
a: []*string{to.Ptr("a"), to.Ptr("b"), to.Ptr("c")},
b: []*string{to.Ptr("a"), to.Ptr("b")},
expected: false,
},
}

func (s *AzureAgentUtilsTestSuite) TestIsEqualAzureActions() {
for _, testCase := range testCases {
result := IsEqualAzureActions(testCase.a, testCase.b)
s.Equal(testCase.expected, result)
}
}

func TestAzureAgentUtilsSuite(t *testing.T) {
suite.Run(t, new(AzureAgentUtilsTestSuite))
}

0 comments on commit b367996

Please sign in to comment.