improvement: Removed dependency on external template provider

[dpiddockcmp]

PR o'clock

Description

Removed dependency on the external template provider and instead used the Terraform native templatefile() function. As recommended by the provider itself.

My primary goal was to stop unnecessary recreation of worker launch templates and configurations when certain other resources are modified, for example the aws_eks_cluster. For example, changing the IP whitelist on the cluster public endpoint should not impact worker nodes. Previously:

• the resource would be marked as "unknown" in the Refresh stage
• the data_template_file userdatas could not be refreshed and so were in an unknown state during Plan stage
• thus generating unnecessary pending recreation of launch config/templates

Thought I'd go the full mile and also convert kubeconfig. I moved some of the logic into the template itself to make the terraform easier to read.

Questions for maintainers

• The userdata lists are now locals and not data sources. Should they be moved to locals.tf or workers.tf/workers_launch_template.tf? I don't think they should remain in data.tf even if it makes history a little harder to follow.
• kubeconfig is a YAML file. Should we use yamlencode, like in aws-auth, instead of a template? It will make the locals messy again but the output YAML should always be valid.

Checklist

• CI tests are passing
• README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

[barryib]

Awesome ❤️! This will increase readability and consistency.

The userdata lists are now locals and not data sources. Should they be moved to locals.tf or workers.tf/workers_launch_template.tf? I don't think they should remain in data.tf even if it makes history a little harder to follow.

I'll prefer to have all locals into the locals.tf file.

kubeconfig is a YAML file. Should we use yamlencode, like in aws-auth, instead of a template? It will make the locals messy again but the output YAML should always be valid.

I think we can continue to use templatefile, it make the kubeconfig easily reusable between LT and LC.

[barryib]

@max-rocket-internet any thought ?

[max-rocket-internet]

Looks amazing 💗 I'll test it

[max-rocket-internet]

@dpiddockcmp would you like to be a maintainer of this module? Your contributions have been outstanding and I am spending less time on this project these days. And I see you're already a member?

I already asked you this some months back on a PR but perhaps you didn't see it 🙂

[max-rocket-internet]

I did a quick plan on 2 clusters and LGTM 🎉

[barryib]

I just tested twice and it's LGTM ❤️❤️❤️

@dpiddockcmp can you please remove template provider in example ? There are not needed anymore. Like https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/examples/basic/main.tf#L22-L24

[barryib]

Merging. I'll open another PR to remove template in examples.

[dpiddockcmp]

Merging. I'll open another PR to remove template in examples.

Oops, I had a couple busy days and was going to do the requested changes today. Thanks for sorting it out.

I'll prefer to have all locals into the locals.tf file.

This change wasn't done either before merging

kubeconfig is a YAML file. Should we use yamlencode, like in aws-auth, instead of a template? It will make the locals messy again but the output YAML should always be valid.

I think we can continue to use templatefile, it make the kubeconfig easily reusable between LT and LC.

I think you're thinking of the userdata 🙂 . Generated kubeconfig is only useful for end users now.

@dpiddockcmp would you like to be a maintainer of this module? Your contributions have been outstanding and I am spending less time on this project these days. And I see you're already a member?

I already asked you this some months back on a PR but perhaps you didn't see it 🙂

@max-rocket-internet I must have missed that. Sure, I'll be a maintainer 😄

[barryib]

PR to update examples #863

[barryib]

I'll prefer to have all locals into the locals.tf file.

This change wasn't done either before merging

Oops. I'll move them in #865

[max-rocket-internet]

Sure, I'll be a maintainer 😄

Done 🚀

[github-actions]

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.