diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f1897ca438..b5771cf182 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,7 +14,7 @@ Added working on StackStorm, improve our security posture, and improve CI reliability thanks in part to pants' use of PEX lockfiles. This is not a user-facing addition. #5778 #5789 #5817 #5795 #5830 #5833 #5834 #5841 #5840 #5838 #5842 #5837 #5849 #5850 - #5846 #5853 #5848 #5847 #5858 + #5846 #5853 #5848 #5847 #5858 #5857 Contributed by @cognifloyd * Added a joint index to solve the problem of slow mongo queries for scheduled executions. #5805 diff --git a/Makefile b/Makefile index 6120a6f992..7cdf9c60fc 100644 --- a/Makefile +++ b/Makefile @@ -460,11 +460,7 @@ generate-api-spec: requirements .generate-api-spec @echo @echo "================== Generate openapi.yaml file ====================" @echo - echo "# NOTE: This file is auto-generated - DO NOT EDIT MANUALLY" > st2common/st2common/openapi.yaml - echo "# Edit st2common/st2common/openapi.yaml.j2 and then run" >> st2common/st2common/openapi.yaml - echo "# make .generate-api-spec" >> st2common/st2common/openapi.yaml - echo "# to generate the final spec file" >> st2common/st2common/openapi.yaml - . $(VIRTUALENV_DIR)/bin/activate; python st2common/bin/st2-generate-api-spec --config-file conf/st2.dev.conf >> st2common/st2common/openapi.yaml + . $(VIRTUALENV_DIR)/bin/activate; python st2common/bin/st2-generate-api-spec --config-file conf/st2.dev.conf > st2common/st2common/openapi.yaml .PHONY: circle-lint-api-spec circle-lint-api-spec: diff --git a/pants-plugins/README.md b/pants-plugins/README.md index a20d0b0fa4..29289c32ae 100644 --- a/pants-plugins/README.md +++ b/pants-plugins/README.md @@ -9,8 +9,22 @@ The plugins here add custom goals or other logic into pants. To see available goals, do "./pants help goals" and "./pants help $goal". These StackStorm-specific plugins are probably only useful for the st2 repo. +- `api_spec` - `schemas` +### `api_spec` plugin + +This plugin wires up pants to make sure `st2common/st2common/openapi.yaml` +gets regenerated if needed. Now, whenever someone runs the `fmt` goal +(eg `./pants fmt st2common/st2common/openapi.yaml`), the api spec will +be regenerated if any of the files used to generate it has changed. +Also, running the `lint` goal will fail if the schemas need to be +regenerated. + +This plugin also wires up pants so that the `lint` goal runs additional +api spec validation on `st2common/st2common/openapi.yaml` with something +like `./pants lint st2common/st2common/openapi.yaml`. + ### `schemas` plugin This plugin wires up pants to make sure `contrib/schemas/*.json` gets diff --git a/pants-plugins/api_spec/BUILD b/pants-plugins/api_spec/BUILD new file mode 100644 index 0000000000..0eea8b1cf1 --- /dev/null +++ b/pants-plugins/api_spec/BUILD @@ -0,0 +1,5 @@ +python_sources() + +python_tests( + name="tests", +) diff --git a/pants-plugins/api_spec/__init__.py b/pants-plugins/api_spec/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pants-plugins/api_spec/register.py b/pants-plugins/api_spec/register.py new file mode 100644 index 0000000000..bf2a9ad1e5 --- /dev/null +++ b/pants-plugins/api_spec/register.py @@ -0,0 +1,24 @@ +# Copyright 2023 The StackStorm Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from api_spec.rules import rules as api_spec_rules +from api_spec.target_types import APISpec + + +def rules(): + return [*api_spec_rules()] + + +def target_types(): + return [APISpec] diff --git a/pants-plugins/api_spec/rules.py b/pants-plugins/api_spec/rules.py new file mode 100644 index 0000000000..7d941475df --- /dev/null +++ b/pants-plugins/api_spec/rules.py @@ -0,0 +1,259 @@ +# Copyright 2023 The StackStorm Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from dataclasses import dataclass + +from pants.backend.python.target_types import EntryPoint +from pants.backend.python.util_rules import pex, pex_from_targets +from pants.backend.python.util_rules.pex import ( + VenvPex, + VenvPexProcess, +) +from pants.backend.python.util_rules.pex_from_targets import PexFromTargetsRequest +from pants.core.goals.fmt import FmtResult, FmtTargetsRequest +from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest +from pants.core.target_types import FileSourceField, ResourceSourceField +from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest +from pants.engine.addresses import Address +from pants.engine.fs import ( + CreateDigest, + Digest, + FileContent, + MergeDigests, + Snapshot, +) +from pants.engine.process import FallibleProcessResult, ProcessResult +from pants.engine.rules import Get, MultiGet, collect_rules, rule +from pants.engine.target import ( + FieldSet, + SourcesField, + TransitiveTargets, + TransitiveTargetsRequest, +) +from pants.engine.unions import UnionRule +from pants.util.logging import LogLevel + +from api_spec.target_types import APISpecSourceField + + +# these constants are also used in the tests +CMD_SOURCE_ROOT = "st2common" +CMD_DIR = "st2common/st2common/cmd" +CMD_MODULE = "st2common.cmd" +GENERATE_CMD = "generate_api_spec" +VALIDATE_CMD = "validate_api_spec" + + +@dataclass(frozen=True) +class APISpecFieldSet(FieldSet): + required_fields = (APISpecSourceField,) + + source: APISpecSourceField + + +class GenerateAPISpecViaFmtTargetsRequest(FmtTargetsRequest): + field_set_type = APISpecFieldSet + name = GENERATE_CMD + + +class ValidateAPISpecRequest(LintTargetsRequest): + field_set_type = APISpecFieldSet + name = VALIDATE_CMD + + +@rule( + desc="Update openapi.yaml with st2-generate-api-spec", + level=LogLevel.DEBUG, +) +async def generate_api_spec_via_fmt( + request: GenerateAPISpecViaFmtTargetsRequest, +) -> FmtResult: + # There will only be one target+field_set, but we iterate + # to satisfy how fmt expects that there could be more than one. + # If there is more than one, they will all get the same contents. + + # Find all the dependencies of our target + transitive_targets = await Get( + TransitiveTargets, + TransitiveTargetsRequest( + [field_set.address for field_set in request.field_sets] + ), + ) + + dependency_files_get = Get( + SourceFiles, + SourceFilesRequest( + sources_fields=[ + tgt.get(SourcesField) for tgt in transitive_targets.dependencies + ], + for_sources_types=(FileSourceField, ResourceSourceField), + ), + ) + + source_files_get = Get( + SourceFiles, + SourceFilesRequest(field_set.source for field_set in request.field_sets), + ) + + # actually generate it with an external script. + # Generation cannot be inlined here because it needs to import the st2 code. + pex_get = Get( + VenvPex, + PexFromTargetsRequest( + [ + Address( + CMD_DIR, + target_name="cmd", + relative_file_path=f"{GENERATE_CMD}.py", + ), + ], + output_filename=f"{GENERATE_CMD}.pex", + internal_only=True, + main=EntryPoint.parse(f"{CMD_MODULE}.{GENERATE_CMD}:main"), + ), + ) + + pex, dependency_files, source_files = await MultiGet( + pex_get, dependency_files_get, source_files_get + ) + + # If we were given an input digest from a previous formatter for the source files, then we + # should use that input digest instead of the one we read from the filesystem. + source_files_snapshot = ( + source_files.snapshot if request.snapshot is None else request.snapshot + ) + + input_digest = await Get( + Digest, + MergeDigests((dependency_files.snapshot.digest, source_files_snapshot.digest)), + ) + + result = await Get( + ProcessResult, + VenvPexProcess( + pex, + argv=( + "--config-file", + "conf/st2.dev.conf", + ), + input_digest=input_digest, + description="Regenerating openapi.yaml api spec", + level=LogLevel.DEBUG, + ), + ) + + contents = [ + FileContent( + f"{field_set.address.spec_path}/{field_set.source.value}", + result.stdout, + ) + for field_set in request.field_sets + ] + + output_digest = await Get(Digest, CreateDigest(contents)) + output_snapshot = await Get(Snapshot, Digest, output_digest) + # TODO: Drop result.stdout since we already wrote it to a file? + return FmtResult.create(request, result, output_snapshot, strip_chroot_path=True) + + +@rule( + desc="Validate openapi.yaml with st2-validate-api-spec", + level=LogLevel.DEBUG, +) +async def validate_api_spec( + request: ValidateAPISpecRequest, +) -> LintResults: + # There will only be one target+field_set, but we iterate + # to satisfy how lint expects that there could be more than one. + # If there is more than one, they will all get the same contents. + + # Find all the dependencies of our target + transitive_targets = await Get( + TransitiveTargets, + TransitiveTargetsRequest( + [field_set.address for field_set in request.field_sets] + ), + ) + + dependency_files_get = Get( + SourceFiles, + SourceFilesRequest( + sources_fields=[ + tgt.get(SourcesField) for tgt in transitive_targets.dependencies + ], + for_sources_types=(FileSourceField, ResourceSourceField), + ), + ) + + source_files_get = Get( + SourceFiles, + SourceFilesRequest(field_set.source for field_set in request.field_sets), + ) + + # actually validate it with an external script. + # Validation cannot be inlined here because it needs to import the st2 code. + pex_get = Get( + VenvPex, + PexFromTargetsRequest( + [ + Address( + CMD_DIR, + target_name="cmd", + relative_file_path=f"{VALIDATE_CMD}.py", + ), + ], + output_filename=f"{VALIDATE_CMD}.pex", + internal_only=True, + main=EntryPoint.parse(f"{CMD_MODULE}.{VALIDATE_CMD}:main"), + ), + ) + + pex, dependency_files, source_files = await MultiGet( + pex_get, dependency_files_get, source_files_get + ) + + input_digest = await Get( + Digest, + MergeDigests((dependency_files.snapshot.digest, source_files.snapshot.digest)), + ) + + process_result = await Get( + FallibleProcessResult, + VenvPexProcess( + pex, + argv=( + "--config-file", + "conf/st2.dev.conf", + # TODO: Uncomment these as part of a project to fix the (many) issues it identifies. + # We can uncomment --validate-defs (and possibly --verbose) once the spec defs are valid. + # "--validate-defs", # check for x-api-model in definitions + # "--verbose", # show model definitions on failure (only applies to --validate-defs) + ), + input_digest=input_digest, + description="Validating openapi.yaml api spec", + level=LogLevel.DEBUG, + ), + ) + + result = LintResult.from_fallible_process_result(process_result) + return LintResults([result], linter_name=request.name) + + +def rules(): + return [ + *collect_rules(), + UnionRule(FmtTargetsRequest, GenerateAPISpecViaFmtTargetsRequest), + UnionRule(LintTargetsRequest, ValidateAPISpecRequest), + *pex.rules(), + *pex_from_targets.rules(), + ] diff --git a/pants-plugins/api_spec/rules_test.py b/pants-plugins/api_spec/rules_test.py new file mode 100644 index 0000000000..0492a5d5dc --- /dev/null +++ b/pants-plugins/api_spec/rules_test.py @@ -0,0 +1,267 @@ +# Copyright 2023 The StackStorm Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from __future__ import annotations + +import os + +from typing import Sequence + +import pytest + +from pants.backend.python import target_types_rules +from pants.backend.python.target_types import PythonSourcesGeneratorTarget + +from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest +from pants.engine.addresses import Address +from pants.engine.fs import CreateDigest, Digest, EMPTY_DIGEST, FileContent, Snapshot +from pants.engine.target import Target +from pants.core.goals.fmt import FmtResult +from pants.core.goals.lint import LintResult, LintResults +from pants.testutil.rule_runner import QueryRule, RuleRunner + +from .rules import ( + CMD_DIR, + CMD_SOURCE_ROOT, + GENERATE_CMD, + VALIDATE_CMD, + APISpecFieldSet, + GenerateAPISpecViaFmtTargetsRequest, + ValidateAPISpecRequest, + rules as api_spec_rules, +) +from .target_types import APISpec + + +@pytest.fixture +def rule_runner() -> RuleRunner: + return RuleRunner( + rules=[ + *api_spec_rules(), + *target_types_rules.rules(), + QueryRule(FmtResult, (GenerateAPISpecViaFmtTargetsRequest,)), + QueryRule(LintResults, (ValidateAPISpecRequest,)), + QueryRule(SourceFiles, (SourceFilesRequest,)), + ], + target_types=[APISpec, PythonSourcesGeneratorTarget], + ) + + +def run_st2_generate_api_spec( + rule_runner: RuleRunner, + targets: list[Target], + *, + extra_args: list[str] | None = None, +) -> FmtResult: + rule_runner.set_options( + [ + "--backend-packages=api_spec", + f"--source-root-patterns=/{CMD_SOURCE_ROOT}", + *(extra_args or ()), + ], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, + ) + field_sets = [APISpecFieldSet.create(tgt) for tgt in targets] + input_sources = rule_runner.request( + SourceFiles, + [ + SourceFilesRequest(field_set.source for field_set in field_sets), + ], + ) + fmt_result = rule_runner.request( + FmtResult, + [ + GenerateAPISpecViaFmtTargetsRequest( + field_sets, snapshot=input_sources.snapshot + ), + ], + ) + return fmt_result + + +def run_st2_validate_api_spec( + rule_runner: RuleRunner, + targets: list[Target], + *, + extra_args: list[str] | None = None, +) -> Sequence[LintResult]: + rule_runner.set_options( + [ + "--backend-packages=api_spec", + f"--source-root-patterns=/{CMD_SOURCE_ROOT}", + *(extra_args or ()), + ], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, + ) + field_sets = [APISpecFieldSet.create(tgt) for tgt in targets] + lint_results = rule_runner.request( + LintResults, + [ + ValidateAPISpecRequest(field_sets), + ], + ) + return lint_results.results + + +# copied from pantsbuild/pants.git/src/python/pants/backend/python/lint/black/rules_integration_test.py +def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: + files = [ + FileContent(path, content.encode()) for path, content in source_files.items() + ] + digest = rule_runner.request(Digest, [CreateDigest(files)]) + return rule_runner.request(Snapshot, [digest]) + + +# add dummy script at st2common/st2common/cmd/generate_api_spec.py that the test can load. +GENERATE_API_SPEC_PY = """ +import os + + +def main(): + api_spec_text = "{api_spec_text}" + print(api_spec_text) +""" + + +def write_generate_files( + api_spec_dir: str, + api_spec_file: str, + before: str, + after: str, + rule_runner: RuleRunner, +) -> None: + files = { + f"{api_spec_dir}/{api_spec_file}": before, + f"{api_spec_dir}/BUILD": f"api_spec(name='t', source='{api_spec_file}')", + # add in the target that's hard-coded in the generate_api_spec_via_fmt rule + f"{CMD_DIR}/{GENERATE_CMD}.py": GENERATE_API_SPEC_PY.format( + api_spec_dir=api_spec_dir, api_spec_text=after + ), + f"{CMD_DIR}/BUILD": "python_sources()", + } + + module = CMD_DIR + while module != CMD_SOURCE_ROOT: + files[f"{module}/__init__.py"] = "" + module = os.path.dirname(module) + + rule_runner.write_files(files) + + +def test_generate_changed(rule_runner: RuleRunner) -> None: + write_generate_files( + api_spec_dir="my_dir", + api_spec_file="dummy.yaml", + before="BEFORE", + after="AFTER", + rule_runner=rule_runner, + ) + + tgt = rule_runner.get_target( + Address("my_dir", target_name="t", relative_file_path="dummy.yaml") + ) + fmt_result = run_st2_generate_api_spec(rule_runner, [tgt]) + assert fmt_result.output == get_snapshot( + rule_runner, {"my_dir/dummy.yaml": "AFTER\n"} + ) + assert fmt_result.did_change is True + + +def test_generate_unchanged(rule_runner: RuleRunner) -> None: + write_generate_files( + api_spec_dir="my_dir", + api_spec_file="dummy.yaml", + before="AFTER\n", + after="AFTER", # print() adds a newline + rule_runner=rule_runner, + ) + + tgt = rule_runner.get_target( + Address("my_dir", target_name="t", relative_file_path="dummy.yaml") + ) + fmt_result = run_st2_generate_api_spec(rule_runner, [tgt]) + assert fmt_result.output == get_snapshot( + rule_runner, {"my_dir/dummy.yaml": "AFTER\n"} + ) + assert fmt_result.did_change is False + + +# add dummy script at st2common/st2common/cmd/validate_api_spec.py that the test can load. +VALIDATE_API_SPEC_PY = """ +import sys + + +def main(): + sys.exit({rc}) +""" + + +def write_validate_files( + api_spec_dir: str, + api_spec_file: str, + contents: str, + rc: int, + rule_runner: RuleRunner, +) -> None: + files = { + f"{api_spec_dir}/{api_spec_file}": contents, + f"{api_spec_dir}/BUILD": f"api_spec(name='t', source='{api_spec_file}')", + # add in the target that's hard-coded in the generate_api_spec_via_fmt rule + f"{CMD_DIR}/{VALIDATE_CMD}.py": VALIDATE_API_SPEC_PY.format( + api_spec_dir=api_spec_dir, rc=rc + ), + f"{CMD_DIR}/BUILD": "python_sources()", + } + + module = CMD_DIR + while module != CMD_SOURCE_ROOT: + files[f"{module}/__init__.py"] = "" + module = os.path.dirname(module) + + rule_runner.write_files(files) + + +def test_validate_passed(rule_runner: RuleRunner) -> None: + write_validate_files( + api_spec_dir="my_dir", + api_spec_file="dummy.yaml", + contents="PASS", + rc=0, + rule_runner=rule_runner, + ) + + tgt = rule_runner.get_target( + Address("my_dir", target_name="t", relative_file_path="dummy.yaml") + ) + lint_result = run_st2_validate_api_spec(rule_runner, [tgt]) + assert len(lint_result) == 1 + assert lint_result[0].exit_code == 0 + assert lint_result[0].report == EMPTY_DIGEST + + +def test_validate_failed(rule_runner: RuleRunner) -> None: + write_validate_files( + api_spec_dir="my_dir", + api_spec_file="dummy.yaml", + contents="FAIL", + rc=1, + rule_runner=rule_runner, + ) + + tgt = rule_runner.get_target( + Address("my_dir", target_name="t", relative_file_path="dummy.yaml") + ) + lint_result = run_st2_validate_api_spec(rule_runner, [tgt]) + assert len(lint_result) == 1 + assert lint_result[0].exit_code == 1 + assert lint_result[0].report == EMPTY_DIGEST diff --git a/pants-plugins/api_spec/target_types.py b/pants-plugins/api_spec/target_types.py new file mode 100644 index 0000000000..c87bcc6e41 --- /dev/null +++ b/pants-plugins/api_spec/target_types.py @@ -0,0 +1,40 @@ +# Copyright 2023 The StackStorm Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from pants.backend.python.target_types import PythonResolveField +from pants.engine.target import ( + COMMON_TARGET_FIELDS, + Dependencies, + Target, +) +from pants.core.target_types import ( + ResourceSourceField, +) + + +class APISpecSourceField(ResourceSourceField): + default = "openapi.yaml" + + +class APISpec(Target): + alias = "api_spec" + core_fields = ( + *COMMON_TARGET_FIELDS, + Dependencies, + APISpecSourceField, + # hack: work around an issue in the pylint backend that tries to + # use this field on the api_spec target, possibly because + # it depends on python files. + PythonResolveField, + ) + help = "Generate openapi.yaml file from Jinja2 template and python sources." diff --git a/pants.toml b/pants.toml index 3de5e4904f..922e7984d9 100644 --- a/pants.toml +++ b/pants.toml @@ -24,6 +24,7 @@ backend_packages = [ # internal plugins in pants-plugins/ "pants.backend.plugin_development", + "api_spec", "schemas", ] # pants ignores files in .gitignore, .*/ directories, /dist/ directory, and __pycache__. diff --git a/st2common/st2common/BUILD b/st2common/st2common/BUILD index 38259c4c32..c40efc6ef3 100644 --- a/st2common/st2common/BUILD +++ b/st2common/st2common/BUILD @@ -5,7 +5,17 @@ python_sources( ) # These may be loaded with st2common.util.spec_loader -resources( +resource( + name="openapi_spec_template", + source="openapi.yaml.j2", +) +api_spec( name="openapi_spec", - sources=["openapi.yaml", "openapi.yaml.j2"], + 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 + ], ) diff --git a/st2common/st2common/cmd/generate_api_spec.py b/st2common/st2common/cmd/generate_api_spec.py index 7ff7757b71..c1a0c6ccc2 100644 --- a/st2common/st2common/cmd/generate_api_spec.py +++ b/st2common/st2common/cmd/generate_api_spec.py @@ -30,12 +30,23 @@ LOG = logging.getLogger(__name__) +# TODO: replace makefile reference with pants equivalent +# ./pants fmt st2common/st2common/openapi.yaml +SPEC_HEADER = """\ +# NOTE: This file is auto-generated - DO NOT EDIT MANUALLY +# Edit st2common/st2common/openapi.yaml.j2 and then run +# make .generate-api-spec +# to generate the final spec file +""" + + def setup(): common_setup(config=config, setup_db=False, register_mq_exchanges=False) def generate_spec(): spec_string = spec_loader.generate_spec("st2common", "openapi.yaml.j2") + print(SPEC_HEADER.rstrip()) print(spec_string)