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: Add an option --tf-version to terraform_docs, terraform_fmt, and terraform_validate #194

Closed
wants to merge 1 commit into from

Conversation

svvitale
Copy link

Enables the use of a version parameter which is in turn used by tfenv to dynamically switch the version of terraform being used at runtime.

This feature is intended for users who may have more than one version of Terraform in use. Running terraform fmt can produce wildly different results depending on the version of Terraform being used, and by allowing the version to be set in the hook configuration, pre-commit can switch to the proper version, run the hook, and then switch back to the previously configured version to restore the original system state.

Please let me know if you have any questions at all. I did modify terraform_fmt.sh pretty extensively to look like the other shell scripts since it now needs to accept a parameter.

@svvitale svvitale changed the title Add an option --tf-version to terraform_docs, terraform_fmt, and terraform_validate feat: Add an option --tf-version to terraform_docs, terraform_fmt, and terraform_validate Apr 22, 2021
…and terraform_validate

Enables the use of a version parameter which is in turn used by `tfenv` to dynamically switch the version of terraform being used at runtime.
@svvitale svvitale force-pushed the optionally-use-tfenv branch from bc8cc2d to 8910cd3 Compare April 22, 2021 05:22
@antonbabenko
Copy link
Owner

antonbabenko commented Apr 22, 2021

Hi @svvitale !

Thanks for the proposal. I am not sure we need to parameterize each hook separately but instead...

I think we can have another hook named something like use_terraform_version which will configure the Terraform version based on:

  1. tfenv from .terraform-version or via args.
  2. tfswitch (https://tfswitch.warrensbox.com/ - I use this one myself and like it more). We don't need to support it right away but good to plan in advance.

Sample config:

hooks:
  - id: use_terraform_version
    args: ['--tfenv --version=min-required']
    # args: ['--tfswitch --version=latest']
    # args: ['--tfenv --version=0.12.18']
    # args: ['--tfswitch --version="^0.12"']

The final PR will be much smaller.

What do you think?

@svvitale
Copy link
Author

Hi @antonbabenko! Thanks so much for the quick reply, I'm glad to hear that you would get some value out of this feature as well.

tfswitch looks really slick, I may actually move our usage to that instead.

I didn't realize that hooks were executed in sequential order, although that makes sense now that you highlighted it. I think this is a MUCH better approach that isolates it from current and future hook code. My only concern is that I'd like to ensure that the previously configured terraform version is restored after the hooks complete. I suppose that can be an additional (and optional) hook that users can place at the end of their hooks list.

This approach would also not be compatible with the fail_fast option, but that could be called out in the docs.

Let me know if you agree with the "restore" option and I'll get to work on the rest. Thanks again for taking the time to look through this.

@antonbabenko
Copy link
Owner

Perfect!

There are some cases like the one you described (fail_fast) but even more often I run pre-commit run -a terraform_fmt which will have to be prefixed with use_terraform_version.

I agree it will be fine as long as we have use_terraform_version and restore_terraform_version at the end.

@MaxymVlasov MaxymVlasov added estimate/2days Need 2 work days to be done hook/terraform_fmt Bash hook feature New feature or request labels Sep 9, 2021
@MaxymVlasov
Copy link
Collaborator

Relates #188

@github-actions
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 10, 2021
@MaxymVlasov MaxymVlasov removed the stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 10, 2021
@MaxymVlasov MaxymVlasov mentioned this pull request Dec 23, 2021
4 tasks
@MaxymVlasov
Copy link
Collaborator

Close as too outdated. Simpler to create a new branch with specified changes than try to resolve conflicts

@BartusZak
Copy link

Is there any possibility to stick with terraform 1.5.7 for terraform_validate?
@MaxymVlasov @antonbabenko

@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Mar 29, 2024

Sure. Install terraform 1.5.7 locally and use it for validation proposes.

Also, what you asking for is part of #570 (comment) which would be implemented by someone in the future (it can be you)

@BartusZak
Copy link

BartusZak commented Mar 29, 2024

@MaxymVlasov does docker image use latest terraform version?

Im using GitLab CI/CD Pipeline and wondering if I should use a decent version or simply install required terraform version before triggering pre-commit run.
e.g:

precommit:
  image: ghcr.io/antonbabenko/pre-commit-terraform:latest
  script:
    - <----------- INSTALL TF here?
    - pre-commit run --all-files

Or is there other possibility to use version 1.5.7 in pipeline?

@MaxymVlasov
Copy link
Collaborator

You can build your own docker image with any tool version, including terraform

Check the Docker section at https://github.com/antonbabenko/pre-commit-terraform?tab=readme-ov-file#1-install-dependencies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
estimate/2days Need 2 work days to be done feature New feature or request hook/terraform_fmt Bash hook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants