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

fix: Make minimum AWS provider version common #1454

Closed
wants to merge 1 commit into from
Closed

fix: Make minimum AWS provider version common #1454

wants to merge 1 commit into from

Conversation

shoekstra
Copy link

PR o'clock

Description

The node_group module requires a more recent version of the AWS provider and as this is use by the EKS module, it should also have its version updated, and since the fargate module is called from the EKS module, it should also have it's version updated.

Checklist

☝️ should happen via terraform-docs.

Verified

This commit was signed with the committer’s verified signature.
tzarc Nick Brassel
The node_group module requires a more recent version of the AWs provider and as this is use by the eks module, it should also have it's version updated, and since the fargate module is called from the EKS module, it should also have it's version updated.

Signed-off-by: Stephen Hoekstra <shoekstra@schubergphilis.com>
@shoekstra shoekstra changed the title Make minimum AWS provider version common fix: Make minimum AWS provider version common Jun 21, 2021
@junaid-ali
Copy link
Contributor

@shoekstra this PR should fix this as well: #1445
haven't got any reviews from maintainers yet.

@daroga0002
Copy link
Contributor

@shoekstra could you update this in all versions.tf files:

$ find . -name versions.tf
./versions.tf
./examples/create_false/versions.tf
./examples/fargate/versions.tf
./examples/managed_node_groups/versions.tf
./examples/basic/versions.tf
./examples/spot_instances/versions.tf
./examples/instance_refresh/versions.tf
./examples/launch_templates_with_managed_node_groups/versions.tf
./examples/irsa/versions.tf
./examples/launch_templates/versions.tf
./examples/secrets_encryption/versions.tf
./modules/node_groups/versions.tf
./modules/fargate/versions.tf

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

I don't think this PR is necessary because modules/fargate can be used as a standalone module and thus it has another version requirements.

There is no need to make versions of providers to match.

The only requirement is that the minimum version of providers contain features used by the module (see CHANGELOG.md in Terraform AWS Provider repo).

@daroga0002 Let's close this PR unless version 3.43.0 is the minimum required version for certain features in the master branch?

@shoekstra
Copy link
Author

There is no need to make versions of providers to match.

I agree with this, but IMO if a module is calling a module that has a higher requirement, then the caller module should match the minimum requirement of the called module.

I can no longer recall why I opened this, I guess there was some issue or I wouldn't have created the PR. If there is no functional difference with current master and this, or if no one else is affected, please feel free to close it. We are specifying our AWS provider version in our versions.tf so no longer affected.

@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 13, 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.

None yet

4 participants