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

A role to create S3 bucket with Dynamodb table for Terraform Backend #1

Merged
merged 16 commits into from
Jan 22, 2024

Conversation

abikouo
Copy link
Contributor

@abikouo abikouo commented Dec 13, 2023

  • A new role to create S3 bucket with versioning enabled and optionally and create a Dynamodb table with a partition key named 'LockID' with type of String
  • Add integration tests for the new role

@abikouo abikouo changed the title A role to create S3 bucket with Dynamodb table for Terraform Backend DNM - A role to create S3 bucket with Dynamodb table for Terraform Backend Dec 18, 2023
@abikouo abikouo closed this Jan 8, 2024
@abikouo abikouo reopened this Jan 8, 2024
@abikouo abikouo changed the title DNM - A role to create S3 bucket with Dynamodb table for Terraform Backend A role to create S3 bucket with Dynamodb table for Terraform Backend Jan 8, 2024
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
roles/aws_s3backend/README.md Outdated Show resolved Hide resolved
@abikouo abikouo requested a review from hakbailey January 9, 2024 10:58
@hakbailey
Copy link
Contributor

@abikouo I also noticed that the Jira ticket specified "The role should be able to either accept an existing IAM role to be granted the above permissions or create a new one", which this doesn't currently do. But I'm not sure if that's actually necessary. @gravesm do we think optionally creating a new IAM role for managing terraform state should be part of this role?

@abikouo abikouo mentioned this pull request Jan 11, 2024
@abikouo abikouo requested a review from hakbailey January 11, 2024 15:48
@gravesm
Copy link
Member

gravesm commented Jan 11, 2024

do we think optionally creating a new IAM role for managing terraform state should be part of this role?

Are there reasons we wouldn't want to? IMO an IAM role is going to be the most common way that people will use to handle access to the backend. There's a whole config section in the s3 backend dedicated to assume role functionality. It seems like it should be easy enough for us to create the role, and would make this much more useful.

tests/integration/constraints.txt Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
tox.ini Show resolved Hide resolved
@abikouo abikouo requested a review from alinabuzachis January 19, 2024 10:24
Copy link
Member

@gravesm gravesm left a comment

Choose a reason for hiding this comment

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

Small change, otherwise LGTM.

README.md Outdated Show resolved Hide resolved
@abikouo abikouo merged commit b9accb9 into redhat-cop:main Jan 22, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants