Skip to content

Commit

Permalink
OCM-7552 | feat: output IAM policies that are being attached to IAM r…
Browse files Browse the repository at this point in the history
…oles
  • Loading branch information
marcolan018 committed Aug 7, 2024
1 parent 72a80f9 commit cab3016
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 37 deletions.
3 changes: 1 addition & 2 deletions cmd/attach/policy/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ func AttachPolicyRunner(userOptions *RosaAttachPolicyOptions) rosa.CommandRunner
}
switch mode {
case interactive.ModeAuto:
output, err := policySvc.AutoAttachArbitraryPolicy(options.roleName, policyArns,
err := policySvc.AutoAttachArbitraryPolicy(r.Reporter, options.roleName, policyArns,
r.Creator.AccountID, orgID)
r.Reporter.Infof(output)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/attach/policy/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ var _ = Describe("rosa attach policy", func() {
mockClient.EXPECT().GetAttachedPolicy(aws.String(roleName)).Return([]mock.PolicyDetail{}, nil)
mockClient.EXPECT().IsPolicyExists(policyArn1).Return(nil, nil)
mockClient.EXPECT().IsPolicyExists(policyArn2).Return(nil, nil)
mockClient.EXPECT().AttachRolePolicy(roleName, policyArn1).Return(nil)
mockClient.EXPECT().AttachRolePolicy(roleName, policyArn2).Return(nil)
mockClient.EXPECT().AttachRolePolicy(t.RosaRuntime.Reporter, roleName, policyArn1).Return(nil)
mockClient.EXPECT().AttachRolePolicy(t.RosaRuntime.Reporter, roleName, policyArn2).Return(nil)
runner := AttachPolicyRunner(options)
err := runner(context.Background(), t.RosaRuntime, c, nil)
Expect(err).NotTo(HaveOccurred())
Expand Down
7 changes: 4 additions & 3 deletions cmd/create/accountroles/creators.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ func (mp *managedPoliciesCreator) createRoles(r *rosa.Runtime, input *accountRol

r.Reporter.Debugf("Creating role '%s'", accRoleName)
tagsList := mp.getRoleTags(file, input)
r.Reporter.Debugf("start to EnsureRole")
roleARN, err := r.AWSClient.EnsureRole(accRoleName, assumeRolePolicy, input.permissionsBoundary,
input.defaultPolicyVersion, tagsList, input.path, true)
if err != nil {
Expand Down Expand Up @@ -114,7 +115,7 @@ func attachManagedPolicies(r *rosa.Runtime, input *accountRolesCreationInput, ro
}

r.Reporter.Debugf("Attaching permission policy to role '%s'", policyKey)
err = r.AWSClient.AttachRolePolicy(accRoleName, policyARN)
err = r.AWSClient.AttachRolePolicy(r.Reporter, accRoleName, policyARN)
if err != nil {
return err
}
Expand Down Expand Up @@ -291,7 +292,7 @@ func createRoleUnmanagedPolicy(r *rosa.Runtime, input *accountRolesCreationInput
}

r.Reporter.Debugf("Attaching permission policy to role '%s'", filename)
return r.AWSClient.AttachRolePolicy(accRoleName, policyARN)
return r.AWSClient.AttachRolePolicy(r.Reporter, accRoleName, policyARN)
}

func getAssumeRolePolicy(partition string, file string, input *accountRolesCreationInput) string {
Expand Down Expand Up @@ -328,7 +329,7 @@ func (hcp *hcpManagedPoliciesCreator) createRoles(r *rosa.Runtime, input *accoun
}

r.Reporter.Debugf("Attaching permission policy to role '%s'", policyKey)
err = r.AWSClient.AttachRolePolicy(accRoleName, policyARN)
err = r.AWSClient.AttachRolePolicy(r.Reporter, accRoleName, policyARN)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/create/ocmrole/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ func createPermissionPolicy(r *rosa.Runtime, policyARN string,
}

r.Reporter.Debugf("Attaching permission policy to role '%s'", roleName)
err := r.AWSClient.AttachRolePolicy(roleName, policyARN)
err := r.AWSClient.AttachRolePolicy(r.Reporter, roleName, policyARN)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/create/operatorroles/by_clusterkey.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func createRoles(
}

r.Reporter.Debugf("Attaching permission policy '%s' to role '%s'", policyArn, roleName)
err = r.AWSClient.AttachRolePolicy(roleName, policyArn)
err = r.AWSClient.AttachRolePolicy(r.Reporter, roleName, policyArn)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/create/operatorroles/by_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ func createRolesByPrefix(r *rosa.Runtime, prefix string, permissionsBoundary str
}

r.Reporter.Debugf("Attaching permission policy '%s' to role '%s'", policyArn, roleName)
err = r.AWSClient.AttachRolePolicy(roleName, policyArn)
err = r.AWSClient.AttachRolePolicy(r.Reporter, roleName, policyArn)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/upgrade/accountroles/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func upgradeAccountRolePolicies(reporter *rprtr.Object, awsClient aws.Client, pr
return err
}

err = awsClient.AttachRolePolicy(roleName, policyARN)
err = awsClient.AttachRolePolicy(reporter, roleName, policyARN)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/upgrade/roles/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ func upgradeAccountRolePoliciesFromCluster(
return err
}

err = awsClient.AttachRolePolicy(roleName, policyARN)
err = awsClient.AttachRolePolicy(reporter, roleName, policyARN)
if err != nil {
return err
}
Expand Down Expand Up @@ -866,7 +866,7 @@ func upgradeOperatorRolePoliciesFromCluster(
}

if operatorRoleName != "" {
err = awsClient.AttachRolePolicy(operatorRoleName, policyARN)
err = awsClient.AttachRolePolicy(reporter, operatorRoleName, policyARN)
if err != nil {
return err
}
Expand Down Expand Up @@ -1101,7 +1101,7 @@ func upgradeMissingOperatorRole(
}
r.Reporter.Infof("Created role '%s' with ARN '%s'", roleName, roleARN)
r.Reporter.Debugf("Attaching permission policy '%s' to role '%s'", policyARN, roleName)
err = r.AWSClient.AttachRolePolicy(roleName, policyARN)
err = r.AWSClient.AttachRolePolicy(r.Reporter, roleName, policyARN)
if err != nil {
return weberr.Errorf("Failed to attach role policy. Check your prefix or run "+
"'rosa create operator-roles' to create the necessary policies: %s", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/aws/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ type Client interface {
path string) (string, error)
EnsurePolicy(policyArn string, document string, version string, tagList map[string]string,
path string) (string, error)
AttachRolePolicy(roleName string, policyARN string) error
AttachRolePolicy(reporter *reporter.Object, roleName string, policyARN string) error
CreateOpenIDConnectProvider(issuerURL string, thumbprint string, clusterID string) (string, error)
DeleteOpenIDConnectProvider(providerURL string) error
HasOpenIDConnectProvider(issuerURL string, partition string, accountID string) (bool, error)
Expand Down
10 changes: 5 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.

23 changes: 21 additions & 2 deletions pkg/aws/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,18 @@ import (
client "github.com/openshift/rosa/pkg/aws/api_interface"
"github.com/openshift/rosa/pkg/aws/tags"
"github.com/openshift/rosa/pkg/helper"
"github.com/openshift/rosa/pkg/reporter"
)

var DefaultPrefix = "ManagedOpenShift"
const (
awsManagedPolicyRegexPattern = `^arn:aws:iam::aws:policy/.*$`
awsManagedPolicyUrlPrefix = "https://docs.aws.amazon.com/aws-managed-policy/latest/reference/"
)

var (
awsManagedPolicyRegex = regexp.MustCompile(awsManagedPolicyRegexPattern)
DefaultPrefix = "ManagedOpenShift"
)

type Operator struct {
Name string
Expand Down Expand Up @@ -445,14 +454,20 @@ func (c *awsClient) hasCompatibleMajorMinorVersionTags(iamTags []iamtypes.Tag, v
return false, nil
}

func (c *awsClient) AttachRolePolicy(roleName string, policyARN string) error {
func (c *awsClient) AttachRolePolicy(reporter *reporter.Object, roleName string, policyARN string) error {
_, err := c.iamClient.AttachRolePolicy(context.Background(), &iam.AttachRolePolicyInput{
RoleName: aws.String(roleName),
PolicyArn: aws.String(policyARN),
})
if err != nil {
return err
}
if isAwsManagedPolicy(policyARN) {
policyARNArr := strings.Split(policyARN, "/")
policyName := policyARNArr[len(policyARNArr)-1]
policyARN = fmt.Sprintf("%s(%s)", policyName, awsManagedPolicyUrlPrefix+policyName)
}
reporter.Infof("Attached policy '%s' to role '%s'\n", policyARN, roleName)
return nil
}

Expand Down Expand Up @@ -2139,3 +2154,7 @@ func getOperatorRolePolicyTags(c client.IamApiClient, roleName string) (map[stri
}
return tagmap, nil
}

func isAwsManagedPolicy(policyArn string) bool {
return awsManagedPolicyRegex.MatchString(policyArn)
}
15 changes: 15 additions & 0 deletions pkg/aws/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -604,3 +604,18 @@ 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))
})
})
2 changes: 1 addition & 1 deletion pkg/helper/roles/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func upgradeMissingOperatorRole(missingRoles map[string]*cmv1.STSOperator, clust
}
r.Reporter.Infof("Created role '%s' with ARN '%s'", roleName, roleARN)
r.Reporter.Debugf("Attaching permission policy '%s' to role '%s'", policyARN, roleName)
err = r.AWSClient.AttachRolePolicy(roleName, policyARN)
err = r.AWSClient.AttachRolePolicy(r.Reporter, roleName, policyARN)
if err != nil {
return fmt.Errorf("Failed to attach role policy. Check your prefix or run "+
"'rosa create account-roles' to create the necessary policies: %s", err)
Expand Down
16 changes: 8 additions & 8 deletions pkg/policy/policy_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/openshift/rosa/pkg/aws"
"github.com/openshift/rosa/pkg/aws/tags"
"github.com/openshift/rosa/pkg/ocm"
"github.com/openshift/rosa/pkg/reporter"
)

const (
Expand All @@ -35,7 +36,8 @@ const (

type PolicyService interface {
ValidateAttachOptions(roleName string, policyArns []string) error
AutoAttachArbitraryPolicy(roleName string, policyArns []string, accountID, orgID string) (string, error)
AutoAttachArbitraryPolicy(reporter *reporter.Object, roleName string,
policyArns []string, accountID, orgID string) error
ManualAttachArbitraryPolicy(roleName string, policyArns []string, accountID, orgID string) string
ValidateDetachOptions(roleName string, policyArns []string) error
AutoDetachArbitraryPolicy(roleName string, policyArns []string, accountID, orgID string) (string, error)
Expand Down Expand Up @@ -74,24 +76,22 @@ func (p *policyService) ValidateDetachOptions(roleName string, policyArns []stri
return validateRoleAndPolicies(p.AWSClient, roleName, policyArns)
}

func (p *policyService) AutoAttachArbitraryPolicy(roleName string, policyArns []string,
accountID, orgID string) (string, error) {
output := ""
func (p *policyService) AutoAttachArbitraryPolicy(reporter *reporter.Object, roleName string, policyArns []string,
accountID, orgID string) error {
for _, policyArn := range policyArns {
err := p.AWSClient.AttachRolePolicy(roleName, policyArn)
err := p.AWSClient.AttachRolePolicy(reporter, roleName, policyArn)
if err != nil {
return output, fmt.Errorf("Failed to attach policy %s to role %s: %s",
return fmt.Errorf("Failed to attach policy %s to role %s: %s",
policyArn, roleName, err)
}
output = output + fmt.Sprintf("Attached policy '%s' to role '%s'\n", policyArn, roleName)
p.OCMClient.LogEvent("ROSAAttachPolicyAuto", map[string]string{
ocm.Account: accountID,
ocm.Organization: orgID,
ocm.RoleName: roleName,
ocm.PolicyArn: policyArn,
})
}
return output[:len(output)-1], nil
return nil
}

func (p *policyService) ManualAttachArbitraryPolicy(roleName string, policyArns []string,
Expand Down
12 changes: 6 additions & 6 deletions pkg/policy/policy_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ import (
mock "github.com/openshift/rosa/pkg/aws"
"github.com/openshift/rosa/pkg/aws/tags"
"github.com/openshift/rosa/pkg/ocm"
"github.com/openshift/rosa/pkg/rosa"
)

var (
r *rosa.Runtime
awsClient *mock.MockClient
ocmClient *ocm.Client
policySvc PolicyService
Expand All @@ -56,6 +58,7 @@ func TestDescribeUpgrade(t *testing.T) {
var _ = Describe("Policy Service", func() {
Context("Attach Policy", Ordered, func() {
BeforeAll(func() {
r = rosa.NewRuntime()
roleName = "sample-role"
policyArn1 = "arn:aws:iam::111111111111:policy/Sample-Policy-1"
policyArn2 = "arn:aws:iam::111111111111:policy/Sample-Policy-2"
Expand Down Expand Up @@ -116,14 +119,11 @@ var _ = Describe("Policy Service", func() {
Expect(err).ShouldNot(HaveOccurred())
})
It("Test AutoAttachArbitraryPolicy", func() {
awsClient.EXPECT().AttachRolePolicy(roleName, policyArn1).Return(nil)
awsClient.EXPECT().AttachRolePolicy(roleName, policyArn2).Return(nil)
output, err := policySvc.AutoAttachArbitraryPolicy(roleName, policyArns,
awsClient.EXPECT().AttachRolePolicy(r.Reporter, roleName, policyArn1).Return(nil)
awsClient.EXPECT().AttachRolePolicy(r.Reporter, roleName, policyArn2).Return(nil)
err := policySvc.AutoAttachArbitraryPolicy(r.Reporter, roleName, policyArns,
"sample-account-id", "sample-org-id")
Expect(err).ShouldNot(HaveOccurred())
Expect(output).To(Equal(fmt.Sprintf("Attached policy '%s' to role '%s'\n"+
"Attached policy '%s' to role '%s'",
policyArn1, roleName, policyArn2, roleName)))
})
It("Test ManualAttachArbitraryPolicy", func() {
output := policySvc.ManualAttachArbitraryPolicy(roleName, policyArns,
Expand Down

0 comments on commit cab3016

Please sign in to comment.