Add pants-plugins/api_spec to streamline regenerating openapi.yaml

[cognifloyd]

Background

This is another part of introducing pants, as discussed in various TSC meetings.

Related PRs can be found in:

• pantsbuild milestone (which includes PRs since Initialize pantsbuild #5713)
• pantsbuild label (which also includes preparatory PRs that came before adding pantsbuild)

Overview of this PR

This PR improves the DX (developer experience) by wiring up:

• the fmt goal to generate st2common/st2common/openapi.yaml if needed (effectively runs st2-generate-api-spec), and
• the lint goal to validate st2common/st2common/openapi.yaml to make sure the generated schema is valid (effectively runs st2-validate-api-spec)

This includes creating a api_spec plugin for pants that adds a api_spec target.

Developer Experience

Today, if someone changes one of the files/scripts used to generate our openapi spec, then CI might complain as part of the ci-checks Makefile target, with instructions to Please run "make generate-api-spec". This also gets regenerated any time someone runs the lint or tests Makefile targets, which might be surprising (tests generally shouldn't handle codegen).

In any case, this wires up pants to run st2-generate-api-spec and st2-validate-api-spec at times that are analogous to when the Makefile runs them.

With this PR, we add api spec generation into the fmt goal and validation to the lint goal. Anything that runs under fmt also runs under the lint goal, so we will get two signals about the api spec: (1) whether or not it needs to be regenerated, and (2) whether or not there are validation errors in the spec.

Our api spec is generated from a jinja template, which only does minimal interpolation (populating the values for some of the enums). There are actually quite a few inconsistencies between our models and the api spec, so we will have to refactor the spec, the spec generation, and possibly the models in the future. That refactoring work, however, is out-of-scope for this PR. After we get pants in, it would be excellent to revisit these issues.

In working on this plugin, I discovered that our api spec validation has been broken for awhile, which I fixed in #5709. Part of our custom validation is still disabled because there are so many things in the spec that are out of date. This PR also does not fix that, but it does leave a comment in the pants plugin on how to enable that later.

Fine grained results cache

After someone runs ./pants fmt :: once, they will benefit from the pants' results cache. For the api_spec plugin, here's what that means:

First, the api_spec target in st2common/st2common depends on the generate/validate binaries in st2common/bin:

st2/st2common/st2common/BUILD

Lines 12 to 21 in 1856f61

	api_spec(
	name="openapi_spec",
	source="openapi.yaml",
	dependencies=[
	":openapi_spec_template",
	"st2common/st2common/cmd/generate_api_spec.py", # st2-generate-api-spec
	"st2common/st2common/cmd/validate_api_spec.py", # st2-validate-api-spec
	"//conf:st2_dev_conf", # used for both generate and validate
	],
	)

If either script (st2-generate-api-spec or st2-validate-api-spec) changes, then pants now knows that openapi.yaml need to be regenerated. Or, in other words, the st2-generate-api-spec file is part of the cache key for st2common/st2common/openapi.yaml. If that file changes, then pants will re-run the generator.

st2-generate-api-spec is a python script, and we use pants' python dependency inference. So pants (effectively) also adds the python files imported by st2-generate-api-spec to the cache key. Stepping through the code, we can see what pants would infer:

st2/st2common/bin/st2-generate-api-spec

Line 18 in 1856f61

from st2common.cmd.generate_api_spec import main

st2/st2common/st2common/cmd/generate_api_spec.py

Lines 22 to 26 in 1856f61

	from st2common import config
	from st2common import log as logging
	from st2common.util import spec_loader
	from st2common.script_setup import setup as common_setup
	from st2common.script_setup import teardown as common_teardown

st2/st2common/st2common/util/spec_loader.py

Lines 21 to 26 in 1856f61

	import st2common.constants.pack
	import st2common.constants.action
	from st2common.rbac.types import PermissionType
	from st2common.util import isotime
	from st2common.util import yml as yaml_utils

Here we reach the few things that get interpolated into the Jinja Template:

st2/st2common/st2common/util/spec_loader.py

Lines 30 to 35 in 1856f61

	ARGUMENTS = {
	"DEFAULT_PACK_NAME": st2common.constants.pack.DEFAULT_PACK_NAME,
	"LIVEACTION_STATUSES": st2common.constants.action.LIVEACTION_STATUSES,
	"PERMISSION_TYPE": PermissionType,
	"ISO8601_UTC_REGEX": isotime.ISO8601_UTC_REGEX,
	}

Now, if anyone changes any of these files, or any of the python bits that get imported to help define things in them, then the next time someone runs the lint goal (or in CI) they will find out that they need to run the fmt goal.

Developing pants plugins

Pants has extensive documentation on the plugin API, including how to create targets, how to write rules, and there are even mini tutorials about how to add a linter or a formatter (adding formatters/linters is a very common thing plugins to do).

• Plugins overview
• The Target API
• The Rules API
  • Processes (running subprocesses in pants plugins)
• Add a linter
• Add a formatter
• Testing Plugins

[cognifloyd]

@amanda11 Thanks for the feedback. I think I've addressed your comments.

[amanda11]

LGTM

[cognifloyd]

We are using st2common.cmd.generate_api_spec:main directly instead of using the st2-generate-api-spec script because pants can't run python with a - in the name (because pants relies on importing from the target script, and python cannot import from files with a - in the name).

Eventually I'll get around to plumbing a change in pants to make that possible (it already works with the underlying pex, but pants hasn't learned about that arg yet). In the meantime, we just use the entrypoint itself instead of the script.

[rush-skills]

Are we in the wrong here for having a python script named st2-generate-api-spec in the first place?

[cognifloyd]

Having a command with - is fairly common, so I don't think that's a problem.

Pex (used by pants) already had everything it needed to read files without importing them, but that didn't have a user facing API. So, when I asked about it, John (pex maintainer) quickly added an --executable option, so now pex supports it. But, pants hasn't learned about that pex option yet, so it doesn't use it.

I started adding the pants bits here, but haven't had time to dive farther than this: pantsbuild/pants@main...cognifloyd:pants:pex-exe-arg

[cognifloyd]

The same thing applies here. We are using st2common.cmd.validate_api_spec:main directly instead of using the st2-validate-api-spec script directly.

[rush-skills]

LGTM