From 27db4dc4faf3a44c62b59c5bbc7a3762c31fcbfc Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 17 Jun 2022 12:32:57 -0500 Subject: [PATCH 1/4] actually validate the openapi spec --- st2common/st2common/cmd/validate_api_spec.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/st2common/st2common/cmd/validate_api_spec.py b/st2common/st2common/cmd/validate_api_spec.py index 01659be732..ee623ea454 100644 --- a/st2common/st2common/cmd/validate_api_spec.py +++ b/st2common/st2common/cmd/validate_api_spec.py @@ -22,8 +22,8 @@ from __future__ import absolute_import import os +import prance from oslo_config import cfg -from prance import ResolvingParser from st2common import config from st2common import log as logging @@ -100,7 +100,7 @@ def validate_spec(): f.write(spec_string) f.flush() - parser = ResolvingParser(spec_file) + parser = prance.ResolvingParser(spec_file) spec = parser.specification return _validate_definitions(spec) @@ -118,7 +118,8 @@ def main(): # The spec loader do not allow duplicate keys. spec_loader.load_spec("st2common", "openapi.yaml.j2") - ret = 0 + # run the schema through prance to validate openapi spec. + ret = validate_spec() except Exception: LOG.error("Failed to validate openapi.yaml file", exc_info=True) ret = 1 From 2922499b9b7f154df0e7581b975badfc275d480e Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 17 Jun 2022 17:32:13 -0500 Subject: [PATCH 2/4] Make schema validation optional There are a lot of issues, so we're only partially validating the spec now. We still validate with prance, but skip checking x-api-model because there are so many legacy issues. I looked at adding the x-api-model ... but wow, we haven't been adding that for a long time. And there are open issues about it: https://github.com/StackStorm/st2/issues/3575 https://github.com/StackStorm/st2/issues/3788 So, we just make it possible, but optional, to run the schema validation. --- st2common/st2common/cmd/validate_api_spec.py | 21 +++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/st2common/st2common/cmd/validate_api_spec.py b/st2common/st2common/cmd/validate_api_spec.py index ee623ea454..0a9facd9c1 100644 --- a/st2common/st2common/cmd/validate_api_spec.py +++ b/st2common/st2common/cmd/validate_api_spec.py @@ -45,6 +45,14 @@ ) ) +# When disabled, only load the spec in prance to validate. Otherwise check for x-api-model as well. +# validate-defs is disabled by default until these are resolved: +# https://github.com/StackStorm/st2/issues/3575 +# https://github.com/StackStorm/st2/issues/3788 +cfg.CONF.register_cli_opt( + cfg.BoolOpt("validate-defs", short="-d", required=False, default=False) +) + cfg.CONF.register_cli_opt( cfg.BoolOpt("generate", short="-c", required=False, default=False) ) @@ -71,6 +79,7 @@ def _validate_definitions(spec): if verbose: LOG.info("Supplied definition for model %s: \n\n%s.", model, definition) + msg += "\n" error = True LOG.error(msg) @@ -81,6 +90,7 @@ def _validate_definitions(spec): def validate_spec(): spec_file = cfg.CONF.spec_file generate_spec = cfg.CONF.generate + validate_defs = cfg.CONF.validate_defs if not os.path.exists(spec_file) and not generate_spec: msg = ( @@ -103,10 +113,13 @@ def validate_spec(): parser = prance.ResolvingParser(spec_file) spec = parser.specification + if not validate_defs: + return True + return _validate_definitions(spec) -def teartown(): +def teardown(): common_teardown() @@ -119,11 +132,13 @@ def main(): spec_loader.load_spec("st2common", "openapi.yaml.j2") # run the schema through prance to validate openapi spec. - ret = validate_spec() + passed = validate_spec() + + ret = 0 if passed else 1 except Exception: LOG.error("Failed to validate openapi.yaml file", exc_info=True) ret = 1 finally: - teartown() + teardown() return ret From bea304c9672a78e7a8ecf389e9d0baf0b7234423 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 12 Aug 2022 15:03:57 -0500 Subject: [PATCH 3/4] add changelog entry --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1e8d09b0da..d8231e0197 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -29,6 +29,9 @@ Fixed * Fixed eventlet monkey patching so more of the unit tests work under pytest. #5689 +* Fix and reenable prance-based openapi spec validation, but make our custom ``x-api-model`` validation optional as the spec is out-of-date. #5709 + Contributed by @cognifloyd + Added ~~~~~ From 710137f6ed39ae9ab3768284d2b11e5d2fc4f8a8 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 17 Jun 2022 17:32:13 -0500 Subject: [PATCH 4/4] Fix issues identified by now-enabled openapi spec validation --- st2common/st2common/models/api/pack.py | 7 ++++++- st2common/st2common/openapi.yaml | 15 ++++++++++----- st2common/st2common/openapi.yaml.j2 | 15 ++++++++++----- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/st2common/st2common/models/api/pack.py b/st2common/st2common/models/api/pack.py index 6de2893427..8c31718b0e 100644 --- a/st2common/st2common/models/api/pack.py +++ b/st2common/st2common/models/api/pack.py @@ -383,12 +383,17 @@ class PackInstallRequestAPI(BaseAPI): schema = { "type": "object", "properties": { - "packs": {"type": "array"}, + "packs": {"type": "array"}, # TODO: add enum "force": { "type": "boolean", "description": "Force pack installation", "default": False, }, + "skip_dependencies": { + "type": "boolean", + "description": "Set to True to skip pack dependency installations.", + "default": False, + }, }, } diff --git a/st2common/st2common/openapi.yaml b/st2common/st2common/openapi.yaml index 107b97a1c2..e86e42727d 100644 --- a/st2common/st2common/openapi.yaml +++ b/st2common/st2common/openapi.yaml @@ -1998,7 +1998,7 @@ paths: schema: type: array items: - $ref: '#/definitions/PackView' + $ref: '#/definitions/DataFilesSubSchema' examples: application/json: ref: 'core.local' @@ -2391,7 +2391,7 @@ paths: description: User performing the operation. responses: '200': - description: Policy created successfully. + description: Policy list schema: type: array items: @@ -3074,7 +3074,7 @@ paths: description: User performing the operation. responses: '200': - description: List of rules + description: List of runner types schema: type: array items: @@ -4883,7 +4883,7 @@ definitions: type: object description: Execution result properties: - $ref: '#/definitions/Execution' + $ref: '#/definitions/Execution/properties' message: type: string AliasMatchAndExecuteInputAPI: @@ -5172,8 +5172,9 @@ definitions: skip_dependencies: type: boolean description: Set to True to skip pack dependency installations. - required: false default: false + required: + - packs PacksUninstall: type: object properties: @@ -5198,6 +5199,10 @@ definitions: type: array items: $ref: '#/definitions/PacksContentRegisterType' + fail_on_failure: + type: boolean + description: True to fail on failure + default: true PacksContentRegisterType: type: string enum: ['all', diff --git a/st2common/st2common/openapi.yaml.j2 b/st2common/st2common/openapi.yaml.j2 index a54bb423f1..f053f0f3d0 100644 --- a/st2common/st2common/openapi.yaml.j2 +++ b/st2common/st2common/openapi.yaml.j2 @@ -1994,7 +1994,7 @@ paths: schema: type: array items: - $ref: '#/definitions/PackView' + $ref: '#/definitions/DataFilesSubSchema' examples: application/json: ref: 'core.local' @@ -2387,7 +2387,7 @@ paths: description: User performing the operation. responses: '200': - description: Policy created successfully. + description: Policy list schema: type: array items: @@ -3070,7 +3070,7 @@ paths: description: User performing the operation. responses: '200': - description: List of rules + description: List of runner types schema: type: array items: @@ -4879,7 +4879,7 @@ definitions: type: object description: Execution result properties: - $ref: '#/definitions/Execution' + $ref: '#/definitions/Execution/properties' message: type: string AliasMatchAndExecuteInputAPI: @@ -5168,8 +5168,9 @@ definitions: skip_dependencies: type: boolean description: Set to True to skip pack dependency installations. - required: false default: false + required: + - packs PacksUninstall: type: object properties: @@ -5194,6 +5195,10 @@ definitions: type: array items: $ref: '#/definitions/PacksContentRegisterType' + fail_on_failure: + type: boolean + description: True to fail on failure + default: true PacksContentRegisterType: type: string enum: ['all',