cisagov / terraformer-packer

Create an AMI that can be used to deploy AWS resources via Terraform.
Creative Commons Zero v1.0 Universal
7 stars 4 forks source link

Move S3 Bucket Name to Variable #35

Open coffeegist opened 1 year ago

coffeegist commented 1 year ago

💡 Summary

I wanted to propose the idea of moving variables, such as the S3 bucket name used by the packer projects, to a variable that can be easily changed to allow easier re-use by the open-source community.

Motivation and context

As this is the base project, it makes most sense to start with this change here, and propagate to subprojects as necessary.

This would be useful because, in the project's current state, none of the code is useable outside of the current team using it due to the fact that (at a minimum) S3 bucket names must be globally unique. Since this code has the S3 bucket name hardcoded, it would require Find and Replace Copy/Paste code changes to reuse the project. It's a great project, and I think the open-source community would benefit from this change greatly :)

Implementation notes

This could be implemented through Terraform variables (with defaults pointing to the current value). However, other options might be preferred by the team. I'm no TF+AWS expert :)

Acceptance criteria

This work is done when:

jsf9k commented 1 year ago

Thanks for the issue, @coffeegist!

jsf9k commented 1 year ago

@coffeegist, which bucket names are you referring to?

jsf9k commented 1 year ago

FWIW, we already do this with bucket names; see, e.g., this code in cisagov/kali-packer, which is called here.

coffeegist commented 1 year ago

I suppose a bucket name would be helpful 🙃 Here is the specific bucket I was referring to

https://github.com/cisagov/terraformer-packer/blob/develop/terraform-build-user/backend.tf#L4

jsf9k commented 1 year ago

Ah, I get it now.

jsf9k commented 1 year ago

For the record, the backend configuration is one place where variables cannot be used in Terraform; but, we may be able to work around that using Terragrunt.

coffeegist commented 1 year ago

Thanks for the clarification, I was not aware of that. Makes more sense now :)