Use bandit (via pantsbuild)

[cognifloyd]

Background

This is another part of introducing pants, as discussed in the TSC Meetings on 12 July 2022, 02 Aug 2022, 06 Sept 2022, and 04 Oct 2022. Pants has fine-grained per-file caching of results for lint, fmt (like black), test, etc. It also has lockfiles that work well for monorepos that have multiple python packages. With these lockfiles CI should not break when any of our dependencies or our transitive dependencies release new versions, because CI will continue to use the locked version until we explicitly relock with updates.

To keep PRs as manageable/reviewable as possible, introducing pants will take a series of PRs. I do not know yet how many PRs; I will break this up into logical steps with these goals:

• introduce pants to the st2 repo, and
• teach some (but not all) TSC members about pants step-by-step.

Other pants PRs include:

• Initialize pantsbuild #5713
• Configure pantsbuild source roots #5724
• Enable pantsbuild language backends #5725
• Adjust .gitignore to work better with pantsbuild #5726
• Add GHA workflow to validate pants BUILD files #5732
• Configure pants_ignore and bump pants to v2.14.0rc1 #5733
• Explicitly disable pantsbuild opt-in anonymous telemetry #5737
• Create pantsbuild BUILD files with ./pants tailor :: #5738
• Add GHA workflow to lint with pantsbuild #5758
• Use shellcheck (via pantsbuild) #5751
• Use black (via pantsbuild) #5774
• Use flake8 (via pantsbuild) #5776
• Use regex-lint to provide circular dependency checking in pants #5778

Overview of this PR

This configures pants to use bandit when running the lint goal.

By default, pants uses bandit>=1.7.0,<1.8, but I told it to use the version we have it pinned to: bandit==1.7.0:

st2/test-requirements.txt

Line 10 in 83904f6

bandit==1.7.0

Pants uses a lockfile for each tool it uses to ensure we get consistent results. Since I changed [bandit].version in pants.toml, pants errors, saying that the lockfile (in this case the <default> lockfile distributed with pants) is out-of-date. Note how the message is very helpful in explaining exactly what has to happen to fix this:

$ ./pants lint ::
18:05:57.53 [INFO] Initializing scheduler...
18:05:57.84 [INFO] Scheduler initialized.
18:05:59.81 [INFO] Completed: Lint with Shellcheck - shellcheck succeeded.
18:06:02.59 [ERROR] 1 Exception encountered:

  InvalidLockfileError: You are using the `<default>` lockfile provided by Pants to install the tool `bandit`, but it is not compatible with your configuration:

- You have set different requirements than those used to generate the lockfile. You can fix this by updating `[bandit].version` and/or `[bandit].extra_requirements`, or by using a new custom lockfile.
In the input requirements, but not in the lockfile: ['bandit==1.7.0']
In the lockfile, but not in the input requirements: ['bandit<1.8,>=1.7.0']

To generate a custom lockfile based on your current configuration, set `[bandit].lockfile` to where you want to create the lockfile, then run `./pants generate-lockfiles --resolve=bandit`.

As described in #5774, I'm putting lockfiles under the lockfiles/ directory. I'm also not using an extension on the file. Pants does not care about extensions, so we can adopt our own convention on naming the lockfiles. For now, I skipped the extension, but we could easily use .lock or something similar.

NOTE: it is not safe to manually change the lockfiles. If we need to change any dependencies (including transitive deps), we need to use pants to regenerate the applicable lockfiles. Therefore, 481 lines of this change are auto-generated - you can review them for sanity, but we should not change them manually.

Unlike black (#5774) and flake8 (#5776), we do not need to add skip_bandit=True in any of the BUILD files.

I used [bandit].args to reuse the bandit settings we already have in the Makefile:

st2/Makefile

Line 556 in 94d0798

. $(VIRTUALENV_DIR)/bin/activate; bandit -r $(COMPONENTS_WITH_RUNNERS) -lll -x build,dist

st2/pants.toml

Lines 88 to 93 in 837fb1c

	args = [
	"-lll", # only HIGH severity level
	"--exclude",
	"build,dist",
	"--quiet", # only show output in the case of an error
	]

I had to look up what each of the args meant, so I documented the -lll arg in a comment, and used --exclude instead of the shorter -x so that it's easier to tell what it's doing. Also, I added the --quiet arg because bandit has a very verbose success message--this was recommended in the pants docs.

Relevant Pants documentation

• ./pants generate-lockfiles goal
• ./pants lint goal
• Python Overview
  • Python Linters and Formatters
  • bandit backend
• source targets:
  • python_sources
  • python_source
• Video about how pants facilitates "Incrementally Adopting New Tools"

Things you can do with pantsbuild

I needed to generate the lockfiles/bandit lockfile, which I did with this (the "scheduler" info messages occur whenever pantsd is starting up in the background, so we can ignore those.):

$./pants generate-lockfiles --resolve=bandit
20:48:36.94 [INFO] Initializing scheduler...
20:48:37.23 [INFO] Scheduler initialized.
20:49:00.55 [INFO] Completed: Generate lockfile for bandit
20:49:00.55 [INFO] Wrote lockfile for the resolve `bandit` to lockfiles/bandit

You can run ./pants lint :: to see if bandit finds any issues (the GHA Lint workflow runs this as well). You can run only one of the linters with the helpful --only arg like this:

$ ./pants lint --only=bandit ::
20:49:08.16 [INFO] Completed: Building bandit.pex from lockfiles/bandit
20:49:09.61 [INFO] Completed: Lint with Bandit - bandit succeeded.
20:49:10.36 [INFO] Completed: Lint with Bandit - bandit succeeded.
20:49:10.99 [INFO] Completed: Lint with Bandit - bandit succeeded.
20:49:11.51 [INFO] Completed: Lint with Bandit - bandit succeeded.
20:49:18.92 [INFO] Completed: Lint with Bandit - bandit succeeded.
20:49:19.91 [INFO] Completed: Lint with Bandit - bandit succeeded.
20:49:22.35 [INFO] Completed: Lint with Bandit - bandit succeeded.
20:49:24.10 [INFO] Completed: Lint with Bandit - bandit succeeded.
20:49:27.22 [INFO] Completed: Lint with Bandit - bandit succeeded.

✓ bandit succeeded.

The --only flag is "scoped" to the lint goal. So, these are equivalent if you wanted to specify that flag in a different order:
./pants lint --only=bandit ::
./pants --lint-only=bandit lint ::
./pants lint :: --lint-only=bandit

note: that I had to add lint in the arg name to use a different order

[amanda11]

LGTM - although the vote seems to be going for .lock files...

[cognifloyd]

This was requested in #5774 (comment)
I missed adding it before merging, so I'm adding it here.

[Eric-Arellano]

Bandit is one of my favorite tools. Nice :)

[rush-skills]

LGTM