Skip to content

Commit

Permalink
OCM-8867 | fix: include prefix check for clusters running registered …
Browse files Browse the repository at this point in the history
…oidc configs
  • Loading branch information
gdbranco committed Jun 17, 2024
1 parent 717d528 commit ddca85c
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 15 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ docs/
cover.out
/temp
dist/
# for VS Code debug sessions
**/__debug_bin*
**/ginkgo.report
13 changes: 8 additions & 5 deletions cmd/list/operatorroles/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ func run(cmd *cobra.Command, _ []string) {
os.Exit(1)
}
clusterId = cluster.ID()
args.prefix = cluster.AWS().STS().OperatorRolePrefix()
}

operatorsMap, err := r.AWSClient.ListOperatorRoles(args.version, clusterId)
operatorsMap, err := r.AWSClient.ListOperatorRoles(args.version, clusterId, args.prefix)
prefixes := helper.MapKeys(operatorsMap)
helper.SortStringRespectLength(prefixes)

Expand All @@ -132,16 +133,18 @@ func run(cmd *cobra.Command, _ []string) {
if args.version != "" {
noOperatorRolesOutput = fmt.Sprintf("%s in version '%s'", noOperatorRolesOutput, args.version)
}
if args.prefix != "" {
if _, ok := operatorsMap[args.prefix]; !ok {
r.Reporter.Infof("No operator roles available for prefix '%s'", args.prefix)
os.Exit(0)
}
}
r.Reporter.Infof(noOperatorRolesOutput)
os.Exit(0)
}
if output.HasFlag() {
var resource interface{} = operatorsMap
if args.prefix != "" {
if _, ok := operatorsMap[args.prefix]; !ok {
r.Reporter.Infof("No operator roles available for prefix '%s'", args.prefix)
os.Exit(0)
}
resource = operatorsMap[args.prefix]
}
err = output.Print(resource)
Expand Down
2 changes: 1 addition & 1 deletion pkg/aws/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ type Client interface {
ListUserRoles() ([]Role, error)
ListOCMRoles() ([]Role, error)
ListAccountRoles(version string) ([]Role, error)
ListOperatorRoles(version string, clusterID string) (map[string][]OperatorRoleDetail, error)
ListOperatorRoles(version string, clusterID string, prefix string) (map[string][]OperatorRoleDetail, error)
ListAttachedRolePolicies(roleName string) ([]string, error)
ListOidcProviders(targetClusterId string, config *cmv1.OidcConfig) ([]OidcProviderOutput, error)
GetRoleByARN(roleARN string) (iamtypes.Role, error)
Expand Down
9 changes: 4 additions & 5 deletions pkg/aws/client_mock.go

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

15 changes: 11 additions & 4 deletions pkg/aws/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -870,8 +870,8 @@ func (c *awsClient) ListAccountRoles(version string) ([]Role, error) {
return c.mapToAccountRoles(version, roles)
}

func (c *awsClient) ListOperatorRoles(version string,
targetClusterId string) (map[string][]OperatorRoleDetail, error) {
func (c *awsClient) ListOperatorRoles(targetVersion string,
targetClusterId string, targetPrefix string) (map[string][]OperatorRoleDetail, error) {

operatorMap := map[string][]OperatorRoleDetail{}
roles, err := c.ListRoles()
Expand Down Expand Up @@ -958,7 +958,7 @@ func (c *awsClient) ListOperatorRoles(version string,
switch aws.ToString(tag.Key) {
case common.OpenShiftVersion:
tagValue := aws.ToString(tag.Value)
if version != "" && tagValue != version {
if targetVersion != "" && tagValue != targetVersion {
skip = true
break
}
Expand All @@ -977,8 +977,15 @@ func (c *awsClient) ListOperatorRoles(version string,
for key, list := range operatorMap {
if len(list) == 0 {
emptyListKeys = append(emptyListKeys, key)
} else if targetClusterId != "" && list[0].ClusterID != targetClusterId {
continue
}
if targetClusterId != "" && list[0].ClusterID != "" && list[0].ClusterID != targetClusterId {
emptyListKeys = append(emptyListKeys, key)
continue
}
if targetPrefix != "" && targetPrefix != key {
emptyListKeys = append(emptyListKeys, key)
continue
}
}
for _, key := range emptyListKeys {
Expand Down
131 changes: 131 additions & 0 deletions pkg/aws/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,137 @@ import (
"github.com/openshift/rosa/pkg/aws/tags"
)

var _ = Describe("ListOperatorRoles", func() {
var (
client awsClient
mockIamAPI *mocks.MockIamApiClient
mockCtrl *gomock.Controller
)

BeforeEach(func() {
mockCtrl = gomock.NewController(GinkgoT())
mockIamAPI = mocks.NewMockIamApiClient(mockCtrl)
client = awsClient{
iamClient: mockIamAPI,
}
})

It("Retrieves by target version", func() {
mockIamAPI.EXPECT().ListRoles(gomock.Any(), gomock.Any()).Return(
&iam.ListRolesOutput{
IsTruncated: false,
Roles: []iamtypes.Role{
{
RoleName: aws.String("some-role-name-openshift"),
},
},
}, nil)
mockIamAPI.EXPECT().ListRoleTags(gomock.Any(), gomock.Any()).Return(
&iam.ListRoleTagsOutput{
IsTruncated: false,
}, nil)
mockIamAPI.EXPECT().ListAttachedRolePolicies(gomock.Any(), gomock.Any()).Return(
&iam.ListAttachedRolePoliciesOutput{
IsTruncated: false,
AttachedPolicies: []iamtypes.AttachedPolicy{
{
PolicyName: aws.String("some-policy-name"),
},
},
}, nil)
mockIamAPI.EXPECT().ListPolicyTags(gomock.Any(), gomock.Any()).Return(
&iam.ListPolicyTagsOutput{
IsTruncated: false,
Tags: []iamtypes.Tag{
{
Key: aws.String(common.OpenShiftVersion),
Value: aws.String("4.13"),
},
},
}, nil)
roles, err := client.ListOperatorRoles("4.13", "", "")
Expect(err).ToNot(HaveOccurred())
Expect(roles).To(HaveLen(1))
})

It("Retrieves by target cluster ID", func() {
mockIamAPI.EXPECT().ListRoles(gomock.Any(), gomock.Any()).Return(
&iam.ListRolesOutput{
IsTruncated: false,
Roles: []iamtypes.Role{
{
RoleName: aws.String("some-role-name-openshift"),
},
},
}, nil)
mockIamAPI.EXPECT().ListRoleTags(gomock.Any(), gomock.Any()).Return(
&iam.ListRoleTagsOutput{
IsTruncated: false,
Tags: []iamtypes.Tag{
{
Key: aws.String(tags.ClusterID),
Value: aws.String("123"),
},
},
}, nil)
mockIamAPI.EXPECT().ListAttachedRolePolicies(gomock.Any(), gomock.Any()).Return(
&iam.ListAttachedRolePoliciesOutput{
IsTruncated: false,
AttachedPolicies: []iamtypes.AttachedPolicy{
{
PolicyName: aws.String("some-policy-name"),
},
},
}, nil)
mockIamAPI.EXPECT().ListPolicyTags(gomock.Any(), gomock.Any()).Return(
&iam.ListPolicyTagsOutput{
IsTruncated: false,
Tags: []iamtypes.Tag{
{
Key: aws.String(common.OpenShiftVersion),
Value: aws.String("4.13"),
},
},
}, nil)
roles, err := client.ListOperatorRoles("", "123", "")
Expect(err).ToNot(HaveOccurred())
Expect(roles).To(HaveLen(1))
})

It("Retrieves by target prefix", func() {
mockIamAPI.EXPECT().ListRoles(gomock.Any(), gomock.Any()).Return(
&iam.ListRolesOutput{
IsTruncated: false,
Roles: []iamtypes.Role{
{
RoleName: aws.String("some-role-name-openshift"),
},
},
}, nil)
mockIamAPI.EXPECT().ListRoleTags(gomock.Any(), gomock.Any()).Return(
&iam.ListRoleTagsOutput{
IsTruncated: false,
Tags: []iamtypes.Tag{
{
Key: aws.String(common.ManagedPolicies),
Value: aws.String("true"),
}},
}, nil)
mockIamAPI.EXPECT().ListAttachedRolePolicies(gomock.Any(), gomock.Any()).Return(
&iam.ListAttachedRolePoliciesOutput{
IsTruncated: false,
AttachedPolicies: []iamtypes.AttachedPolicy{
{
PolicyName: aws.String("some-policy-name"),
},
},
}, nil)
roles, err := client.ListOperatorRoles("", "", "some-role-name")
Expect(err).ToNot(HaveOccurred())
Expect(roles).To(HaveLen(1))
})
})

var _ = Describe("mapToAccountRoles", func() {
var (
client awsClient
Expand Down

0 comments on commit ddca85c

Please sign in to comment.