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(terragrunt-version): implement tgenv version file updates #8816

Merged
merged 7 commits into from Feb 24, 2021
Merged

feat(terragrunt-version): implement tgenv version file updates #8816

merged 7 commits into from Feb 24, 2021

Conversation

sajidk1
Copy link
Contributor

@sajidk1 sajidk1 commented Feb 22, 2021

Changes:

Closes: #8815

Context:

See example renovate PR: sajidk1/renovate-test#1

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
  • No new tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@CLAassistant
Copy link

CLAassistant commented Feb 22, 2021

CLA assistant check
All committers have signed the CLA.

@sajidk1 sajidk1 mentioned this pull request Feb 22, 2021
lib/manager/terragrunt-version/extract.spec.ts Outdated Show resolved Hide resolved
lib/manager/terragrunt-version/index.ts Outdated Show resolved Hide resolved
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@sajidk1 sajidk1 requested a review from viceice February 23, 2021 09:19
…e.md

Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com>
HonkingGoose
HonkingGoose previously approved these changes Feb 23, 2021
Copy link
Collaborator

@HonkingGoose HonkingGoose left a comment

Choose a reason for hiding this comment

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

The docs look good to me.

lib/manager/terragrunt-version/extract.spec.ts Outdated Show resolved Hide resolved
const dep: PackageDependency = {
depName: 'gruntwork-io/terragrunt',
currentValue: content.trim(),
datasource: datasourceGitHubRelease.id,
Copy link
Member

Choose a reason for hiding this comment

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

I guess terragrunt uses terraform versioning?, so should be added here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I understand.

terragrunt uses terraform versioning in what way?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like we need a custom versioning here

https://github.com/cunymatthieu/tgenv#terragrunt-version

can be a version, latest or latest:<regex> so we should at least ignore the latest values with this pr, otherwise renovate would try to update them to real versions.

like here:

if (!isVersion(currentValue)) {
dep.skipReason = SkipReason.UnsupportedVersion;
} else {

Copy link
Contributor Author

@sajidk1 sajidk1 Feb 23, 2021

Choose a reason for hiding this comment

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

Firstly, thanks for all the feedback!

As you've likely figured out by now I'm not familiar with this codebase or typescript 😅

So this PR may be open for a while as a figure it out 🙃

Can I make the case for not including this right now?

  • tfenv has the same versioning: https://github.com/tfutils/tfenv#terraform-version-file
    • Yet that's not supported fully in the terraform-version manager in renovate
  • I'd argue the main use case of these version managers tools when combined with renovate are precise version bumps

Copy link
Member

Choose a reason for hiding this comment

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

so terraform-version manager is wrong too 🙈

Copy link
Collaborator

Choose a reason for hiding this comment

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

The lookup process will already do a filter based on isValid(). We prefer not to set unsupported version in manager extracts so that users can configure a custom versioning using package rules that gets applied post-extract and pre-validation

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.

needs snapshot updates

@sajidk1
Copy link
Contributor Author

sajidk1 commented Feb 23, 2021

needs snapshot updates

How do I just run my test?

I'm using the dev container

@rarkins
Copy link
Collaborator

rarkins commented Feb 23, 2021

yarn jest terragrunt -u

@sajidk1 sajidk1 requested a review from viceice February 23, 2021 18:29
@rarkins rarkins merged commit 0d5d358 into renovatebot:master Feb 24, 2021
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 24.64.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Support tgenv
6 participants