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

Breaking changes ahead? #14

Closed
wcamarao opened this issue Mar 10, 2019 · 14 comments
Closed

Breaking changes ahead? #14

wcamarao opened this issue Mar 10, 2019 · 14 comments

Comments

@wcamarao
Copy link

wcamarao commented Mar 10, 2019

With new AWS provider major version 2.0, should we expect breaking changes soon when using source = "git::https://github.com/cloudposse/terraform-aws-tfstate-backend.git?ref=master"?

@osterman
Copy link
Member

We never recommend pinning to master. You will notice anywhere we actually embed modules we never pin to master and in our terraform-root-modules (blueprints) the same.

Our examples pin to master because we have 130+ terraform modules, so we just use master to make it easy on us (CloudPosse) otherwise we forget to update it.

@wcamarao
Copy link
Author

wcamarao commented Mar 11, 2019

Thanks. Perhaps it could be helpful to mention this on the README then, and an example using a tag. BTW, it would be ref=0.3.1, for example?

Update: for others in the future, format would be ref=tags/x.y.z

@osterman
Copy link
Member

I agree we should update our readme template to include that recommendation.

@wcamarao
Copy link
Author

I'll do it, can you confirm if this would be the right format? E.g. ref=0.3.1?

@osterman
Copy link
Member

@wcamarao thanks! Would love your help, but it might be more work than you had expected.

Unfortunately, the only scalable way to incorporate this change in a meaningful way so it percolates out to all of our modules is to add it to our upstream README.md template found here: https://raw.githubusercontent.com/cloudposse/build-harness/master/templates/README.md

e.g. We should add this to the ## Usage section.

{{ if (file.Exists "main.tf") }}
**IMPORTANT:** Do not pin to `master` because there may be breaking changes between releases. Instead pin to the release tag (e.g. `?ref=tags/x.y.z`) of one of our [latest releases]({{ printf "https://github.com/%s/releases" (ds "config").github_repo}}).
{{end}}

Since we have 130+ modules, we can't easily incorporate a change like this without adding it to our template. I'll open up an issue and have someone on the team get to it (but no ETA).

@wcamarao
Copy link
Author

Cheers for that Erik!

@rtomlinson-latacora
Copy link

I'm sorry, but where do I find the release list? I copied ref=tags/0.5.3 from the embedded modules but would like to know what I should be doing going forward.
I noticed that the backend module used to to preserve hyphens in the name attribute when constructing the dynamodb table and s3 bucket but no longer does. Thank you

@osterman
Copy link
Member

@rtomlinson-latacora
Copy link

Sweet. Thanks

@osterman
Copy link
Member

I noticed that the backend module used to to preserve hyphens in the name attribute when constructing the dynamodb table and s3 bucket but no longer does. Thank you

This should still be the case. If it doesn't, maybe it's related to some changes in the terraform-null-label provider. We should by default preserve the -, but maybe a bug was introduced?

https://github.com/cloudposse/terraform-null-label/pulls?q=is%3Apr+is%3Aclosed

@aknysh
Copy link
Member

aknysh commented Mar 13, 2019

@rtomlinson-latacora
please see this comment on terraform-null-label
cloudposse/terraform-null-label#49 (comment)

The label version needs to be updated, and this regex should be used to preserve the hyphens in the names

regex_replace_chars = "/[^a-zA-Z0-9-]/"

@osterman
Copy link
Member

@aknysh I think we should permit hyphens in names by default. If this is not the case, then that was an oversight on our part.

@rtomlinson-latacora
Copy link

Thanks. We've opted to not use hyphens in the various attributes to avoid this issue going forward.

@aknysh
Copy link
Member

aknysh commented Mar 14, 2019

@osterman I'll update terraform-null-label module to support hyphens by default, and then update terraform-aws-tfstate-backend with the new terraform-null-label version

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

No branches or pull requests

4 participants