Skip to content

Commit

Permalink
OCM-12806 | feat: Delete operator roles auto mode (and changes to acc…
Browse files Browse the repository at this point in the history
…ount roles)
  • Loading branch information
hunterkepley committed Dec 2, 2024
1 parent 37f81bb commit 4bb370e
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 27 deletions.
7 changes: 6 additions & 1 deletion cmd/dlt/accountroles/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/openshift/rosa/pkg/interactive"
"github.com/openshift/rosa/pkg/interactive/confirm"
"github.com/openshift/rosa/pkg/ocm"
"github.com/openshift/rosa/pkg/roles"
"github.com/openshift/rosa/pkg/rosa"
)

Expand Down Expand Up @@ -204,12 +205,16 @@ func deleteAccountRoles(r *rosa.Runtime, env string, prefix string, clusters []*
r.Reporter.Infof(fmt.Sprintf("Deleting %saccount roles", roleTypeString))

r.OCMClient.LogEvent("ROSADeleteAccountRoleModeAuto", nil)
deleteHcpSharedVpcPolicies := false
if roles.CheckIfRolesAreHcpSharedVpc(r, finalRoleList) {
deleteHcpSharedVpcPolicies = confirm.Prompt(true, "Attempt to delete Hosted CP shared VPC policies?")
}
for _, role := range finalRoleList {
if !confirm.Prompt(true, "Delete the account role '%s'?", role) {
continue
}
r.Reporter.Infof("Deleting account role '%s'", role)
err := r.AWSClient.DeleteAccountRole(role, prefix, managedPolicies)
err := r.AWSClient.DeleteAccountRole(role, prefix, managedPolicies, deleteHcpSharedVpcPolicies)
if err != nil {
r.Reporter.Warnf("There was an error deleting the account roles or policies: %s", err)
continue
Expand Down
20 changes: 19 additions & 1 deletion cmd/dlt/operatorrole/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/openshift/rosa/pkg/interactive"
"github.com/openshift/rosa/pkg/interactive/confirm"
"github.com/openshift/rosa/pkg/ocm"
"github.com/openshift/rosa/pkg/roles"
"github.com/openshift/rosa/pkg/rosa"
)

Expand Down Expand Up @@ -220,6 +221,13 @@ func run(cmd *cobra.Command, _ []string) {
switch mode {
case interactive.ModeAuto:
r.OCMClient.LogEvent("ROSADeleteOperatorroleModeAuto", nil)

// Only ask user if they want to delete policies if they are deleting HcpSharedVpc roles
deleteHcpSharedVpcPolicies := false
if roles.CheckIfRolesAreHcpSharedVpc(r, foundOperatorRoles) {
deleteHcpSharedVpcPolicies = confirm.Prompt(true, "Attempt to delete Hosted CP shared VPC policies?")
}
allSharedVpcPoliciesNotDeleted := make(map[string]bool)
for _, role := range foundOperatorRoles {
if !confirm.Prompt(true, "Delete the operator role '%s'?", role) {
continue
Expand All @@ -228,7 +236,11 @@ func run(cmd *cobra.Command, _ []string) {
if spin != nil {
spin.Start()
}
err := r.AWSClient.DeleteOperatorRole(role, managedPolicies)
sharedVpcPoliciesNotDeleted, err := r.AWSClient.DeleteOperatorRole(role, managedPolicies,
deleteHcpSharedVpcPolicies)
for key, value := range sharedVpcPoliciesNotDeleted {
allSharedVpcPoliciesNotDeleted[key] = value
}

if err != nil {
if spin != nil {
Expand All @@ -242,6 +254,12 @@ func run(cmd *cobra.Command, _ []string) {
spin.Stop()
}
}
for policyOutput, notDeleted := range allSharedVpcPoliciesNotDeleted {
if notDeleted {
r.Logger.Warnf("Unable to delete policy %s: Policy still attached to other resources",
policyOutput)
}
}
if !errOccured {
r.Reporter.Infof("Successfully deleted the operator roles")
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/aws/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ type Client interface {
ListOidcProviders(targetClusterId string, config *cmv1.OidcConfig) ([]OidcProviderOutput, error)
GetRoleByARN(roleARN string) (iamtypes.Role, error)
GetRoleByName(roleName string) (iamtypes.Role, error)
DeleteOperatorRole(roles string, managedPolicies bool) error
DeleteOperatorRole(roles string, managedPolicies bool, deleteHcpSharedVpcPolicies bool) (map[string]bool, error)
GetOperatorRolesFromAccountByClusterID(
clusterID string,
credRequests map[string]*cmv1.STSOperator,
Expand All @@ -156,11 +156,12 @@ type Client interface {
GetAccountRoleForCurrentEnv(env string, roleName string) (Role, error)
GetAccountRoleForCurrentEnvWithPrefix(env string, rolePrefix string,
accountRolesMap map[string]AccountRole) ([]Role, error)
DeleteAccountRole(roleName string, prefix string, managedPolicies bool) error
DeleteAccountRole(roleName string, prefix string, managedPolicies bool, deleteHcpSharedVpcPolicies bool) error
DeleteOCMRole(roleARN string, managedPolicies bool) error
DeleteUserRole(roleName string) error
GetAccountRolePolicies(roles []string, prefix string) (map[string][]PolicyDetail, map[string][]PolicyDetail, error)
GetAttachedPolicy(role *string) ([]PolicyDetail, error)
GetPolicyDetailsFromRole(role *string) ([]*iam.GetPolicyOutput, error)
HasPermissionsBoundary(roleName string) (bool, error)
GetOpenIDConnectProviderByClusterIdTag(clusterID string) (string, error)
GetOpenIDConnectProviderByOidcEndpointUrl(oidcEndpointUrl string) (string, error)
Expand Down
36 changes: 26 additions & 10 deletions pkg/aws/client_mock.go

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

76 changes: 65 additions & 11 deletions pkg/aws/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -1065,15 +1065,37 @@ func checkIfROSAOperatorRole(role iamtypes.Role, credRequest map[string]*cmv1.ST
return false
}

func (c *awsClient) DeleteOperatorRole(roleName string, managedPolicies bool) error {
func (c *awsClient) GetPolicyDetailsFromRole(role *string) ([]*iam.GetPolicyOutput, error) {
policies, err := c.iamClient.ListAttachedRolePolicies(context.Background(), &iam.ListAttachedRolePoliciesInput{
RoleName: role,
})
if err != nil {
return nil, err
}
finalOutput := make([]*iam.GetPolicyOutput, 0)
for _, attachedPolicy := range policies.AttachedPolicies {
policy, err := c.iamClient.GetPolicy(context.Background(), &iam.GetPolicyInput{
PolicyArn: attachedPolicy.PolicyArn,
})
if err != nil {
return nil, err
}
finalOutput = append(finalOutput, policy)
}
return finalOutput, nil
}

func (c *awsClient) DeleteOperatorRole(roleName string, managedPolicies bool,
deleteHcpSharedVpcPolicies bool) (map[string]bool, error) {
role := aws.String(roleName)
sharedVpcPoliciesNotDeleted := make(map[string]bool)
tagFilter, err := getOperatorRolePolicyTags(c.iamClient, roleName)
if err != nil {
return err
return sharedVpcPoliciesNotDeleted, err
}
policies, excludedPolicies, err := getAttachedPolicies(c.iamClient, roleName, tagFilter)
if err != nil {
return err
return sharedVpcPoliciesNotDeleted, err
}
err = c.detachOperatorRolePolicies(role)
if err != nil {
Expand All @@ -1086,25 +1108,55 @@ func (c *awsClient) DeleteOperatorRole(roleName string, managedPolicies bool) er
err = nil
}
if err != nil {
return err
return sharedVpcPoliciesNotDeleted, err
}
}
err = c.DeleteRole(*role)
if err != nil {
return err
return sharedVpcPoliciesNotDeleted, err
}
if !managedPolicies {
_, err = c.deletePolicies(policies)
} else {
} else if deleteHcpSharedVpcPolicies {
var sharedVpcHcpPolicies []string
for _, policy := range excludedPolicies {
if strings.Contains(policy, SharedVpcAssumeRolePrefix) {
sharedVpcHcpPolicies = append(sharedVpcHcpPolicies, policy)
policyOutput, err := c.iamClient.GetPolicy(context.Background(), &iam.GetPolicyInput{
PolicyArn: aws.String(policy),
})
if err != nil || policyOutput == nil {
return sharedVpcPoliciesNotDeleted, err
}

containsManagedTag := false
containsHcpSharedVpcTag := false
for _, policyTag := range policyOutput.Policy.Tags {
switch strings.Trim(*policyTag.Key, " ") {
case tags.RedHatManaged:
containsManagedTag = true
case tags.HcpSharedVpc:
containsHcpSharedVpcTag = true
}
}
// Delete if no attachments and correct tags showing it's a hcp sharedvpc policy
if containsManagedTag && containsHcpSharedVpcTag {
if *policyOutput.Policy.AttachmentCount == 0 {
// If the policy is deleted, remove from the warning outputs
sharedVpcPoliciesNotDeleted[*policyOutput.Policy.Arn] = false

// Add to list of sharedVpc policies to be actually deleted
c.logger.Infof("Deleting policy '%s'", policy)
sharedVpcHcpPolicies = append(sharedVpcHcpPolicies, policy)
} else {
// Print warning message after all roles are checked (will result in duplicated warnings without
// adding to this map and displaying at the end of role deletion due to multiple roles having
// one of the policies)
sharedVpcPoliciesNotDeleted[*policyOutput.Policy.Arn] = true
}
}
}
_, err = c.deletePolicies(sharedVpcHcpPolicies)
}
return err
return sharedVpcPoliciesNotDeleted, err
}

func (c *awsClient) DeleteRole(role string) error {
Expand Down Expand Up @@ -1140,7 +1192,8 @@ func (c *awsClient) GetInstanceProfilesForRole(r string) ([]string, error) {
return instanceProfiles, nil
}

func (c *awsClient) DeleteAccountRole(roleName string, prefix string, managedPolicies bool) error {
func (c *awsClient) DeleteAccountRole(roleName string, prefix string, managedPolicies bool,
deleteHcpSharedVpcPolicies bool) error {
role := aws.String(roleName)
err := c.DeleteInlineRolePolicies(aws.ToString(role))
if err != nil {
Expand Down Expand Up @@ -1173,7 +1226,7 @@ func (c *awsClient) DeleteAccountRole(roleName string, prefix string, managedPol
}
if !managedPolicies {
_, err = c.deletePolicies(policyMap)
} else {
} else if deleteHcpSharedVpcPolicies {
var sharedVpcHcpPolicies []string
for _, policy := range excludedPolicyMap {
policyOutput, err := c.iamClient.GetPolicy(context.Background(), &iam.GetPolicyInput{
Expand All @@ -1195,6 +1248,7 @@ func (c *awsClient) DeleteAccountRole(roleName string, prefix string, managedPol
}
if containsManagedTag && containsHcpSharedVpcTag {
if *policyOutput.Policy.AttachmentCount == 0 {
c.logger.Infof("Deleting policy '%s'", policy)
sharedVpcHcpPolicies = append(sharedVpcHcpPolicies, policy)
} else {
c.logger.Warnf("Unable to delete policy %s: Policy still attached to %v other resource(s)",
Expand Down
13 changes: 11 additions & 2 deletions pkg/aws/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,15 @@ var _ = Describe("Cluster Roles/Policies", func() {
Expect(policies).To(HaveLen(1))
Expect(policies[0]).To(Equal(operatorRolePolicyArn))
})
It("Test GetPolicyDetailsFromRole", func() {
mockIamAPI.EXPECT().ListAttachedRolePolicies(gomock.Any(), &iam.ListAttachedRolePoliciesInput{
RoleName: aws.String(accountRole),
}).Return(accountRoleAttachedPolicies, nil).Times(1)
mockIamAPI.EXPECT().GetPolicy(gomock.Any(), gomock.Any()).Times(2)
output, err := client.GetPolicyDetailsFromRole(&accountRole)
Expect(err).NotTo(HaveOccurred())
Expect(output).To(HaveLen(2))
})
It("Test DeleteAccountRole", func() {
mockIamAPI.EXPECT().ListRolePolicies(gomock.Any(), gomock.Any()).Return(&iam.ListRolePoliciesOutput{}, nil)
mockIamAPI.EXPECT().ListAttachedRolePolicies(gomock.Any(), &iam.ListAttachedRolePoliciesInput{
Expand Down Expand Up @@ -567,7 +576,7 @@ var _ = Describe("Cluster Roles/Policies", func() {
mockIamAPI.EXPECT().DeletePolicy(gomock.Any(), &iam.DeletePolicyInput{
PolicyArn: aws.String(accountRolePolicyArn),
}).Return(&iam.DeletePolicyOutput{}, nil)
err := client.DeleteAccountRole(accountRole, rolePrefix, false)
err := client.DeleteAccountRole(accountRole, rolePrefix, false, false)
Expect(err).NotTo(HaveOccurred())
})
It("Test DeleteOperatorRole", func() {
Expand Down Expand Up @@ -602,7 +611,7 @@ var _ = Describe("Cluster Roles/Policies", func() {
AttachmentCount: &attachCount,
},
}, nil)
err := client.DeleteOperatorRole(operatorRole, false)
_, err := client.DeleteOperatorRole(operatorRole, false, false)
Expect(err).NotTo(HaveOccurred())
})
})
Expand Down
26 changes: 26 additions & 0 deletions pkg/roles/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,29 @@ func GetHcpSharedVpcPolicyDetails(r *rosa.Runtime, roleArn string) (bool, string

return existsQuery != nil, createPolicy, policyName, nil
}

func CheckIfRolesAreHcpSharedVpc(r *rosa.Runtime, roles []string) bool {
isHcpSharedVpc := false
for _, roleName := range roles {
ptrRoleName := roleName
attachedPolicies, err := r.AWSClient.GetPolicyDetailsFromRole(&ptrRoleName)
if err != nil {
r.Reporter.Errorf("Failed to get policy details for role '%s': %v", roleName, err)
}
for _, attachedPolicy := range attachedPolicies {
rhManaged := false
hcpSharedVpc := false
for _, tag := range attachedPolicy.Policy.Tags {
if *tag.Key == tags.RedHatManaged {
rhManaged = true
} else if *tag.Key == tags.HcpSharedVpc {
hcpSharedVpc = true
}
}
if rhManaged && hcpSharedVpc {
isHcpSharedVpc = true
}
}
}
return isHcpSharedVpc
}

0 comments on commit 4bb370e

Please sign in to comment.