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

improve tfsec version providing, behavior, add more known versions #21111

Merged
merged 2 commits into from
Jun 29, 2024

Conversation

purajit
Copy link
Contributor

@purajit purajit commented Jun 26, 2024

  • Most importantly, make the check_existence check search for all configs. Currently, it completely ignores _tfchecks.{yaml,json}, the standard way to define custom checks.
  • Add v1.28.6 and make it the default
  • I find it less hacky to provide version numbers as semver-like strings, and add "v"s and other prefixes where needed. (Versus substringing). I can undo if there are strong opinions here.

@purajit purajit marked this pull request as ready for review June 26, 2024 21:17
Copy link
Contributor

@lilatomic lilatomic left a comment

Choose a reason for hiding this comment

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

Thanks for this!

For context, the config file originally just pulled in the config file itself. But this is probably the easiest way to pull the custom tests in for execution.

Using plain semver strings makes sense to me, IDK why I did it like this originally.

I was going to ask you to add a custom check to the integration test. But it looks like it's not possible to change the ignored paths for the test runner. So, assuming you've tested this manually, nevermind.

@lilatomic lilatomic added category:new feature backend: Terraform Terraform backend-related issues labels Jun 27, 2024
@purajit
Copy link
Contributor Author

purajit commented Jun 27, 2024

Thanks for this!

For context, the config file originally just pulled in the config file itself. But this is probably the easiest way to pull the custom tests in for execution.

Using plain semver strings makes sense to me, IDK why I did it like this originally.

I was going to ask you to add a custom check to the integration test. But it looks like it's not possible to change the ignored paths for the test runner. So, assuming you've tested this manually, nevermind.

Yeah, I was debating whether to explicitly just add .tfsec/_tfchecks.{yaml,json} instead of using a glob, but I figured a glob would be better so that we don't have to constantly keep this up-to-date with any additional configs/name changes tfsec might do. And yes, tested manually!

@lilatomic
Copy link
Contributor

I tested manually myself, looks good!
I've got an idea for modifying the rule runner to allow setting the ignore paths. I'm going to merge this and then see if I can get that to work and get a test in.

@lilatomic lilatomic merged commit b1592ca into pantsbuild:main Jun 29, 2024
25 checks passed
@lilatomic
Copy link
Contributor

Test added in #21116 . Turns out it was easier than I thought; bootstrap_args already does all the plumbing necessary.

@sureshjoshi
Copy link
Member

@lilatomic @purajit

Is this a breaking change for people who specified the tool version in their pants.toml?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Terraform Terraform backend-related issues category:new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants