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: Revert "feat: add __GIT_WORKING_DIR__ to tfsec" #259

Closed
wants to merge 1 commit into from

Conversation

MaxymVlasov
Copy link
Collaborator

Reverts #255

Remove useless crutch. It works fine without it

@MaxymVlasov MaxymVlasov changed the title Revert "feat: add __GIT_WORKING_DIR__ to tfsec" fix: Revert "feat: add __GIT_WORKING_DIR__ to tfsec" Oct 25, 2021
Copy link
Collaborator

@yermulnik yermulnik left a comment

Choose a reason for hiding this comment

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

If the "crutch" doesn't break any existing functionality and doesn't remove an option to specify relative paths like (./dir/file) along with a placeholder for absolute repo paths (like __GIT_WORKING_DIR__/dir/file) I would vote for keeping this quirk in place (and moreover adding it to other alike hooks) at least for the sake of unifying stuff and to support interchangeability of approaches used in hooks for different tools. Just IMHO though.

@MaxymVlasov
Copy link
Collaborator Author

I have plans to remove __GIT_WORKING_DIR__ from tflint, but it will be breaking change.

Well, I will create an issue for that

@MaxymVlasov
Copy link
Collaborator Author

MaxymVlasov commented Oct 25, 2021

I think that each thing should be done only in one standard way. The most common way to specify a relative path is using ./path or path not __GIT_WORKING_DIR__/path.

The latest crutch exist only in tflint hook and can be replaced via https://github.com/antonbabenko/pre-commit-terraform/pull/260/files#diff-39de6fea7de87803bfb4fa9736c7e69179d795f93e94212987b273e2a6313eb1R4-R9

@yermulnik
Copy link
Collaborator

I see. Great. Going to approve #260

@MaxymVlasov
Copy link
Collaborator Author

#264 (comment)

@MaxymVlasov MaxymVlasov deleted the revert-255-patch-1 branch October 26, 2021 10:31
@gravitybacklight
Copy link
Contributor

gravitybacklight commented Oct 26, 2021

If .tfsec.json is at the top level of the git repo and you have differing structures of terraform, for example:

AA/Terraform-here
BB/CC/Terraform-here

a relative tfsec.json file path fails with:
failed to access config file '.tfsec.json': stat .tfsec.json: no such file or directory
in my example as it can't both be ../../.tfsec.json & ../.tfsec.json
unless you start building out multiple tfsec runs with excludes - if that is possible.

So to have a team wide pre-commit configuration, the __GIT_WORKING_DIR__ like tflint works well.

Am i missing a feature that would permit this for the whole team without paths specific to my machine?

@MaxymVlasov
Copy link
Collaborator Author

@gravitybacklight nope, you're right.
Also, prepared little docs clarification - #266

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants