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

Change the order of cloud-nuke to delete IAM policy first before IAM groups #393

Merged
merged 6 commits into from
Jan 13, 2023

Conversation

hongil0316
Copy link
Contributor

@hongil0316 hongil0316 commented Jan 10, 2023

Change the order of cloud-nuke to delete IAM policy first before IAM groups

Description

Fixes https://gruntwork.atlassian.net/browse/CORE-330.

Testing

  • Created a test IAM policy and IAM group via AWS UI
  • Inspected that both element exist by running the command:
aws-vault exec demo-logs -- ./cloud-nuke inspect-aws --resource-type iam-policy --resource-type iam-group
  • Ran the cloud-nuke command to delete both resources at the same time:
aws-vault exec demo-logs -- ./cloud-nuke aws --resource-type iam-group --resource-type iam-policy 

Final confirmation from the cloud-nuke CLI:

image

  • Checked the AWS UI and verified both resources have been deleted successfully.
    Also, tested inline group policy deletion as well by manually creating an inline group policy AWS UI:

image

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
  • Include release notes. If this PR is backward incompatible, include a migration guide.
  • Attention Grunts - if this PR adds support for a new resource, ensure the nuke_sandbox and nuke_phxdevops jobs in .circleci/config.yml have been updated with appropriate exclusions (either directly in the job or via the .circleci/nuke_config.yml file) to prevent nuking IAM roles, groups, resources, etc that are important for the test accounts.

Release Notes (draft)

Added / Removed / Updated [X].
Updated [Updated the iam-group cloud-nuke operation to delete inline group policies]

Migration Guide

No migration needed

Copy link
Contributor

@ellisonc ellisonc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an issue with inline polices on the IAM group, can you add a test for that?

@arsci
Copy link
Contributor

arsci commented Jan 10, 2023

Can you update the description - todos and release notes section

@hongil0316
Copy link
Contributor Author

This was an issue with inline polices on the IAM group, can you add a test for that?

Hmm I tried to look through the API documentation and existing set of tests in the code base, but it doesn't seem like there's a way to create IAM group with inline policy. For instance, if we look at the input struct, it doesn't provide option to create inline policy:

type AttachGroupPolicyInput struct {
	_ struct{} `type:"structure"`

	// The name (friendly name, not ARN) of the group to attach the policy to.
	//
	// This parameter allows (through its regex pattern (http://wikipedia.org/wiki/regex))
	// a string of characters consisting of upper and lowercase alphanumeric characters
	// with no spaces. You can also include any of the following characters: _+=,.@-
	//
	// GroupName is a required field
	GroupName *string `min:"1" type:"string" required:"true"`

	// The Amazon Resource Name (ARN) of the IAM policy you want to attach.
	//
	// For more information about ARNs, see Amazon Resource Names (ARNs) (https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html)
	// in the Amazon Web Services General Reference.
	//
	// PolicyArn is a required field
	PolicyArn *string `min:"20" type:"string" required:"true"`
}

Also the CreateGroup request struct:

type CreateGroupInput struct {
	_ struct{} `type:"structure"`

	// The name of the group to create. Do not include the path in this value.
	//
	// IAM user, group, role, and policy names must be unique within the account.
	// Names are not distinguished by case. For example, you cannot create resources
	// named both "MyResource" and "myresource".
	//
	// GroupName is a required field
	GroupName *string `min:"1" type:"string" required:"true"`

	// The path to the group. For more information about paths, see IAM identifiers
	// (https://docs.aws.amazon.com/IAM/latest/UserGuide/Using_Identifiers.html)
	// in the IAM User Guide.
	//
	// This parameter is optional. If it is not included, it defaults to a slash
	// (/).
	//
	// This parameter allows (through its regex pattern (http://wikipedia.org/wiki/regex))
	// a string of characters consisting of either a forward slash (/) by itself
	// or a string that must begin and end with forward slashes. In addition, it
	// can contain any ASCII character from the ! (\u0021) through the DEL character
	// (\u007F), including most punctuation characters, digits, and upper and lowercased
	// letters.
	Path *string `min:"1" type:"string"`
}

I can test this manually via the AWS UI and update the description above.

@hongil0316
Copy link
Contributor Author

Can you update the description - todos and release notes section

Updated. Let me know if there's anything I missed!

@arsci
Copy link
Contributor

arsci commented Jan 12, 2023

Hmm I tried to look through the API documentation and existing set of tests in the code base, but it doesn't seem like there's a way to create IAM group with inline policy.

I think you can use PutGroupPolicy for creating inline policies

https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/iam#Client.PutGroupPolicy

Copy link
Contributor

@arsci arsci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hongil0316 hongil0316 dismissed ellisonc’s stale review January 13, 2023 17:14

his comment has been addressed

@hongil0316 hongil0316 merged commit 313bf7c into master Jan 13, 2023
@hongil0316 hongil0316 deleted the CORE-181/cloud-nuke-iam-policy branch January 13, 2023 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants