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

Add Action check against minimum versions #575

Merged
merged 1 commit into from
Nov 4, 2019

Conversation

dpiddockcmp
Copy link
Contributor

@dpiddockcmp dpiddockcmp commented Oct 29, 2019

PR o'clock

Description

Add a Github Action check to try to validation the minimum versions listed. This won't catch everything but it does catch the two recent cases:

Checklist

@max-rocket-internet
Copy link
Contributor

Good idea @dpiddockcmp

Maybe we should just set all hashicorp/terraform:latest to hashicorp/terraform:0.12.2?

@dpiddockcmp
Copy link
Contributor Author

Hi Max.

Do you mean instead of this check? As this check is also validating the providers' minimum versions.

@max-rocket-internet
Copy link
Contributor

I meant we are already running the fmt job with hashicorp/terraform:latest: https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/.github/workflows/lint.yml#L17

And validate with hashicorp/terraform:latest: https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/.github/workflows/lint.yml#L36

So I was thinking we just switch all of these to 0.12.2 but now I think about it, you're saying we should validate for current latest AND our current minimum. Is that correct?

@dpiddockcmp
Copy link
Contributor Author

Personally, I hate the use of :latest as it can easily add unpredictable breakages to CI runs, like every time the terraform minor version increases. No doubt 1.0 or 0.13 will cause issues again.

I think it would be a good idea to set static version numbers for the other tests. This may not necessarily be the minimum of the module:

  • terraform fmt from 0.12.2 incorrectly complains about aws_auth.tf and data.tf but 0.12.6 and later do not.
  • examples include other modules which may have their own terraform restrictions. Currently only VPC is used which does not specify versions.

@max-rocket-internet
Copy link
Contributor

Personally, I hate the use of :latest as it can easily add unpredictable breakages

I agree. It's especially bad in docker 😅

I think it would be a good idea to set static version numbers for the other tests

I get the concept but the problem is that people will just brew install terraform, get the latest version and then use this module, and perhaps have an issue. But we need to know if the latest version has a problem with this module.

So I think we merge this and see how it goes unless someone has a better idea.

@max-rocket-internet max-rocket-internet merged commit c0ae644 into terraform-aws-modules:master Nov 4, 2019
@dpiddockcmp dpiddockcmp deleted the minimum branch December 11, 2019 12:51
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants