Skip to content

Commit

Permalink
OCM-10511 | fix: missing operator roles when filtering due to extende…
Browse files Browse the repository at this point in the history
…d naming cutting the postfix
  • Loading branch information
ciaranRoche committed Aug 20, 2024
1 parent 6816d0d commit f181e00
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 9 deletions.
47 changes: 38 additions & 9 deletions pkg/aws/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ func isAccountRoleVersionCompatible(tagsList []iamtypes.Tag, roleType string,
}

func (c *awsClient) ListRoles() ([]iamtypes.Role, error) {
roles := []iamtypes.Role{}
var roles []iamtypes.Role
paginator := iam.NewListRolesPaginator(c.iamClient, &iam.ListRolesInput{})
for paginator.HasMorePages() {
output, err := paginator.NextPage(context.TODO())
Expand Down Expand Up @@ -994,6 +994,18 @@ func (c *awsClient) ListOperatorRoles(targetVersion string,
return operatorMap, nil
}

func (c *awsClient) ValidateIfRosaOperatorRole(role iamtypes.Role,
credRequest map[string]*cmv1.STSOperator) (bool, error) {
listRoleTags, err := c.iamClient.ListRoleTags(context.Background(), &iam.ListRoleTagsInput{
RoleName: role.RoleName,
})
if err != nil {
return false, err
}
role.Tags = listRoleTags.Tags
return checkIfROSAOperatorRole(role, credRequest), nil
}

// Check if it is one of the ROSA account roles
func checkIfAccountRole(roleName *string) bool {
for _, prefix := range AccountRoles {
Expand All @@ -1005,9 +1017,16 @@ func checkIfAccountRole(roleName *string) bool {
}

// Check if it is one of the ROSA account roles
func checkIfROSAOperatorRole(roleName *string, credRequest map[string]*cmv1.STSOperator) bool {
func checkIfROSAOperatorRole(role iamtypes.Role, credRequest map[string]*cmv1.STSOperator) bool {
for _, operatorRole := range credRequest {
if strings.Contains(aws.ToString(roleName), operatorRole.Namespace()) {
for _, tag := range role.Tags {
if aws.ToString(tag.Key) == "operator_namespace" {
if strings.Contains(aws.ToString(tag.Value), operatorRole.Namespace()) {
return true
}
}
}
if strings.Contains(aws.ToString(role.RoleName), operatorRole.Namespace()) {
return true
}
}
Expand Down Expand Up @@ -1341,15 +1360,20 @@ func (c *awsClient) detachOperatorRolePolicies(role *string) error {

func (c *awsClient) GetOperatorRolesFromAccountByClusterID(clusterID string,
credRequest map[string]*cmv1.STSOperator) ([]string, error) {
roleList := []string{}
var roleList []string
roles, err := c.ListRoles()
if err != nil {
return roleList, err
}
for _, role := range roles {
if !checkIfROSAOperatorRole(role.RoleName, credRequest) {
isValidOperatorRole, err := c.ValidateIfRosaOperatorRole(role, credRequest)
if err != nil {
return roleList, err
}
if !isValidOperatorRole {
continue
}

listRoleTagsOutput, err := c.iamClient.ListRoleTags(context.Background(),
&iam.ListRoleTagsInput{
RoleName: role.RoleName,
Expand All @@ -1376,19 +1400,24 @@ func (c *awsClient) GetOperatorRolesFromAccountByClusterID(clusterID string,

func (c *awsClient) GetOperatorRolesFromAccountByPrefix(prefix string,
credRequest map[string]*cmv1.STSOperator) ([]string, error) {
roleList := []string{}
var roleList []string
roles, err := c.ListRoles()
if err != nil {
return roleList, err
}
prefixOperatorRoleRE := regexp.MustCompile(("(?i)" + fmt.Sprintf("(%s)-(openshift|kube-system)", prefix)))
for _, role := range roles {
if !checkIfROSAOperatorRole(role.RoleName, credRequest) {
if !prefixOperatorRoleRE.MatchString(*role.RoleName) {
continue
}
if prefixOperatorRoleRE.MatchString(*role.RoleName) {
roleList = append(roleList, aws.ToString(role.RoleName))
isValidOperatorRole, err := c.ValidateIfRosaOperatorRole(role, credRequest)
if err != nil {
return roleList, err
}
if !isValidOperatorRole {
continue
}
roleList = append(roleList, aws.ToString(role.RoleName))
}
return roleList, nil
}
Expand Down
87 changes: 87 additions & 0 deletions pkg/aws/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
common "github.com/openshift-online/ocm-common/pkg/aws/validations"
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
"github.com/sirupsen/logrus"

"github.com/openshift/rosa/pkg/aws/mocks"
Expand Down Expand Up @@ -604,3 +605,89 @@ var _ = Describe("Cluster Roles/Policies", func() {
Expect(err).NotTo(HaveOccurred())
})
})

var _ = Describe("Validates isAwsManagedPolicy function", func() {
var (
awsManagedPolicyArn = "arn:aws:iam::aws:policy/service-role/ROSAInstallerPolicy"
customPolicyArn = "arn:aws:iam::765374464689:policy/test-policy"
)
It("check aws managed policy", func() {
result := isAwsManagedPolicy(awsManagedPolicyArn)
Expect(result).To(Equal(true))
})
It("check custom policy", func() {
result := isAwsManagedPolicy(customPolicyArn)
Expect(result).To(Equal(false))
})
})

var _ = Describe("CheckIfROSAOperatorRole", func() {

var (
credRequest map[string]*cmv1.STSOperator
role iamtypes.Role
result bool
)

BeforeEach(func() {
stsOperator1, err := cmv1.NewSTSOperator().Namespace("namespace-1").Build()
Expect(err).NotTo(HaveOccurred())
stsOperator2, err := cmv1.NewSTSOperator().Namespace("namespace-2").Build()
Expect(err).NotTo(HaveOccurred())
credRequest = map[string]*cmv1.STSOperator{
"operator-1": stsOperator1,
"operator-2": stsOperator2,
}
})

When("the role has matching tags", func() {
It("should return true", func() {
role = iamtypes.Role{
RoleName: aws.String("test-role-name"),
Tags: []iamtypes.Tag{
{
Key: aws.String("operator_namespace"),
Value: aws.String("namespace-1"),
},
},
}
result = checkIfROSAOperatorRole(role, credRequest)
Expect(result).To(BeTrue())
})
})

Context("the role name contains the namespace", func() {
BeforeEach(func() {
role = iamtypes.Role{
RoleName: aws.String("test-role-namespace-2"),
Tags: []iamtypes.Tag{},
}
result = checkIfROSAOperatorRole(role, credRequest)
})

It("should return true", func() {
role = iamtypes.Role{
RoleName: aws.String("test-role-namespace-2"),
Tags: []iamtypes.Tag{},
}
result = checkIfROSAOperatorRole(role, credRequest)
Expect(result).To(BeTrue())
})
})

When("the role has no matching tags or name", func() {
It("should return false", func() {
role = iamtypes.Role{
RoleName: aws.String("test-role-name"),
Tags: []iamtypes.Tag{
{
Key: aws.String("operator_namespace"),
Value: aws.String("non-matching-namespace"),
},
},
}
result = checkIfROSAOperatorRole(role, credRequest)
Expect(result).To(BeFalse())
})
})
})

0 comments on commit f181e00

Please sign in to comment.