improve tfsec version providing, behavior, add more known versions

pantsbuild / pants

The Pants Build System

https://www.pantsbuild.org

Apache License 2.0

3.19k stars 613 forks source link

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

Closed purajit closed 5 days ago

purajit commented 1 week ago

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 commented 1 week ago

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 commented 5 days ago

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 commented 5 days ago

Test added in https://github.com/pantsbuild/pants/pull/21116 . Turns out it was easier than I thought; bootstrap_args already does all the plumbing necessary.

sureshjoshi commented 5 days ago

@lilatomic @purajit

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

lilatomic commented 4 days ago

Yes it is. It's called it out in the release notes, is there somewhere else we should add it?

sureshjoshi commented 4 days ago

No, that's the place - but I thought we had a 3-version deprecation rule.

lilatomic commented 4 days ago

ah, I thought that didn't apply to experimental. It'll be easy to just strip the leading "v" and keep backwards compat, lemme do that

sureshjoshi commented 4 days ago

Moved the conversation here: https://pantsbuild.slack.com/archives/C0D7TNJHL/p1719780913456259