Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCM-12806 | feat: Delete operator roles auto mode (and changes to account roles) #2656

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
den-rgb marked this conversation as resolved.
Show resolved Hide resolved
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
}