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.

Sign up for GitHub

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

Jump to bottom

Support AWS v4 provider #71

Merged
merged 2 commits into from
Mar 4, 2022
Merged

Support AWS v4 provider #71

merged 2 commits into from
Mar 4, 2022

Conversation

Nuru
Copy link
Contributor

@Nuru Nuru commented Feb 26, 2022

what

  • Migrate to AWS v4 Terraform provider
  • Add features
    • Allow full S3 storage lifecycle configuration
    • Allow multiple bucket policy documents
    • Allow specifying the bucket name directly, rather than requiring it to be generated by null-label
    • Allow specifying S3 object ownership
    • Allow enabling S3 bucket keys for encryption
  • Deprecate variable by variable specification of a single storage lifecycle rule
  • Add extra safety measure force_destroy_enabled

why

  • AWS v4 broke this module
  • Feature parity
  • Replaced with more power and more flexible input
  • Reduce the chance that automated upgrades will cause data loss

references

@Nuru Nuru added the enhancement New feature or request label Feb 26, 2022
@Nuru Nuru requested review from a team as code owners February 26, 2022 06:36
@Nuru Nuru self-assigned this Feb 26, 2022
@Nuru
Copy link
Contributor Author

Nuru commented Feb 26, 2022

/test all

aknysh
aknysh reviewed Mar 4, 2022
View reviewed changes
examples/complete/variables.tf Outdated

variable "lifecycle_configuration_rules" {
type = any
Copy link
Member

Choose a reason for hiding this comment

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

should it be list(any) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really it should match the module's variable definition, but I didn't want to have to keep them in sync.

aknysh
aknysh reviewed Mar 4, 2022
View reviewed changes
main.tf Show resolved Hide resolved
aknysh
aknysh reviewed Mar 4, 2022
View reviewed changes
variables-deprecated.tf Outdated
default = ""
}


Copy link
Member

Choose a reason for hiding this comment

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

remove the extra empty line

aknysh
aknysh reviewed Mar 4, 2022
View reviewed changes
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

please see comments

bridgecrew[bot]
bridgecrew bot reviewed Mar 4, 2022
View reviewed changes
Copy link

@bridgecrew bridgecrew bot left a comment

Choose a reason for hiding this comment

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

⚠️   Due to 71d9c12 - Respond to reviewer comments - 1 new error was added and 1 error was fixed.

Change details

Error ID Change Path Resource
BC_VUL_2 Added /test/src/go.sum undefined
BC_AWS_GENERAL_31 Fixed /launch-template.tf aws_launch_template.default

@Nuru
Copy link
Contributor Author

Nuru commented Mar 4, 2022

/test all

@Nuru Nuru merged commit 112c75d into master Mar 4, 2022
@Nuru Nuru deleted the aws-v4 branch March 4, 2022 22:20
This was referenced Apr 3, 2022
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bridgecrew bridgecrew[bot] bridgecrew[bot] left review comments

@aknysh aknysh aknysh approved these changes

@osterman osterman Awaiting requested review from osterman

@mcalhoun mcalhoun Awaiting requested review from mcalhoun

@max-lobur max-lobur Awaiting requested review from max-lobur

@nitrocode nitrocode Awaiting requested review from nitrocode

@korenyoni korenyoni Awaiting requested review from korenyoni

Assignees

@Nuru Nuru

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Nuru @aknysh