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

feat(terraform): update terraform lock files #8429

Merged
merged 78 commits into from
Jun 16, 2021

Conversation

secustor
Copy link
Collaborator

@secustor secustor commented Jan 26, 2021

Changes:

This PR adds a post update hook, similar to NPM to update Terraform lock files.

A Terraform binary ( >= 0.14.0 ) has to be in PATH for this to work.

Context:

Prerequisites: renovatebot/docker-renovate-full#256
Closes #7895

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added unit tests, or
  • Unit tests + ran on a real repository

https://github.com/secustor/renovate_terraform_lockfile

@secustor secustor marked this pull request as draft January 26, 2021 15:32
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

Please do not follow npm implementation, use artifact handling like in helmv3 manager.

export async function updateArtifacts({

@secustor secustor requested a review from viceice January 27, 2021 15:15
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

This needs some unit tests

lib/workers/branch/index.ts Outdated Show resolved Hide resolved
lib/manager/terraform/post-update/index.ts Outdated Show resolved Hide resolved
@secustor
Copy link
Collaborator Author

I have copied the helmv3 unit tests and adapted them to Terraform, but I`m not sure if they are correct like they are right now.

@secustor secustor requested a review from viceice January 28, 2021 15:02
@Djiit
Copy link
Contributor

Djiit commented Feb 22, 2021

Hey guys, can we help with this in any way? Would be awesome to see this on master. Cheers

lib/manager/terraform/artifacts.ts Outdated Show resolved Hide resolved
lib/manager/terraform/artifacts.ts Outdated Show resolved Hide resolved
@secustor secustor force-pushed the implement-terraform-lock-files branch from 20d27a4 to d565e44 Compare April 9, 2021 23:52
@secustor
Copy link
Collaborator Author

Because of hashicorp/terraform#28333 and as there is no movement regarding the Hashicorp Registry improvements, I will investigate more in the direction to handle the lock file directly without using the terraform binary

My initial thoughts are:

Any thoughts from you guys?

@viceice
Copy link
Member

viceice commented Apr 10, 2021

Sounds good. Hashes should be safe to store in global cache for long time as they shouldn't ever change. They are also not privacy relevant.

@secustor
Copy link
Collaborator Author

secustor commented Apr 16, 2021

changed the approach and dropped the hash generation at the datasource as the intial sync would take about 1-2h per provider.
I have implemented it now at the artifact layer as we here already know which exact version is needed.

@secustor secustor requested a review from rarkins June 5, 2021 21:25
docs/usage/self-hosted-experimental.md Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.spec.ts Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.spec.ts Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.spec.ts Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.spec.ts Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.spec.ts Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.spec.ts Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.spec.ts Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.spec.ts Outdated Show resolved Hide resolved
lib/manager/terraform/lockfile/index.ts Outdated Show resolved Hide resolved
@secustor
Copy link
Collaborator Author

@viceice Any chance you find some time this week to take a look?

@JamieMagee
Copy link
Contributor

Merged latest main into this branch, just to be sure that there were no conflicts with #10362.

@secustor
Copy link
Collaborator Author

There have been conflicts, but they have been resolved in 051506c

@JamieMagee
Copy link
Contributor

@secustor Ah, I'm sorry! When I initially looked I was only concerned with git conflicts, and didn't look for implementation conflicts. I hope it wasn't too much difficulty.

@rarkins
Copy link
Collaborator

rarkins commented Jun 16, 2021

So the first use case for this will be self-hosted only, right? Because it's a env variable then we app users can't opt-in individually. Could be good to have @secustor battle test it a little first anyway :)

@rarkins rarkins changed the title feat(Terraform): update Terraform lock files feat(terraform): update terraform lock files Jun 16, 2021
@rarkins rarkins enabled auto-merge (squash) June 16, 2021 07:52
@rarkins rarkins merged commit dab27f2 into renovatebot:main Jun 16, 2021
@secustor
Copy link
Collaborator Author

@JamieMagee No problem! I have found this out by accident as I have been merging the main.

@rarkins Yes that would work only on self hosted instances. Regarding battle testing, I'm not using lock files as we pin the deps to exact versions. But I'm sure there will be people using it as it has been the most upvoted issue.

@rarkins
Copy link
Collaborator

rarkins commented Jun 16, 2021

I think the tests may fail on Windows:

image

(we only run Windows tests on main, not every PR)

We'll need to fix ASAP or revert.

rarkins added a commit that referenced this pull request Jun 16, 2021
viceice pushed a commit that referenced this pull request Jun 16, 2021
* fix: Revert "fix(manager): optimize lockfile cache handling (#10463)"

This reverts commit 713e35e.

* fix: Revert "fix(terraform): use path joins instead of slashes (#10461)"

This reverts commit 2776db6.

* fix: Revert "feat(terraform): update terraform lock files (#8429)"

This reverts commit dab27f2.
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 25.43.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2021
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.

Terraform lock files
7 participants