From 19ee0b7cbfbdac8bdf3b5f9f314d903a1e80bb42 Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Tue, 29 Oct 2024 11:11:32 -0400 Subject: [PATCH] new: Add support for loading JSON OpenAPI spec files (#629) --- .github/workflows/docker-build.yml | 2 +- Dockerfile | 1 + Makefile | 3 +- linodecli/__init__.py | 9 +-- linodecli/arg_helpers.py | 27 -------- linodecli/baked/parsing.py | 2 +- linodecli/cli.py | 102 ++++++++++++++++++++++++++++- tests/fixtures/cli_test_load.json | 95 +++++++++++++++++++++++++++ tests/fixtures/cli_test_load.yaml | 64 ++++++++++++++++++ tests/unit/conftest.py | 26 +++++++- tests/unit/test_arg_helpers.py | 8 --- tests/unit/test_cli.py | 44 +++++++++++++ 12 files changed, 333 insertions(+), 50 deletions(-) create mode 100644 tests/fixtures/cli_test_load.json create mode 100644 tests/fixtures/cli_test_load.yaml diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 6d49887fd..c77f7a84b 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -17,4 +17,4 @@ jobs: - name: Build the Docker image run: docker build . --file Dockerfile --tag linode/cli:$(date +%s) --build-arg="github_token=$GITHUB_TOKEN" env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/Dockerfile b/Dockerfile index 499343bc9..0397591fe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,7 @@ FROM python:3.11-slim AS builder ARG linode_cli_version + ARG github_token WORKDIR /src diff --git a/Makefile b/Makefile index a8884fdf2..42cae3c4d 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,8 @@ # # Makefile for more convenient building of the Linode CLI and its baked content # + +# Test-related arguments MODULE := TEST_CASE_COMMAND := TEST_ARGS := @@ -9,7 +11,6 @@ ifdef TEST_CASE TEST_CASE_COMMAND = -k $(TEST_CASE) endif - SPEC_VERSION ?= latest ifndef SPEC override SPEC = $(shell ./resolve_spec_url ${SPEC_VERSION}) diff --git a/linodecli/__init__.py b/linodecli/__init__.py index 0a7463303..6f2d62c5e 100755 --- a/linodecli/__init__.py +++ b/linodecli/__init__.py @@ -15,12 +15,7 @@ from linodecli import plugins from linodecli.exit_codes import ExitCodes -from .arg_helpers import ( - bake_command, - register_args, - register_plugin, - remove_plugin, -) +from .arg_helpers import register_args, register_plugin, remove_plugin from .cli import CLI from .completion import get_completions from .configuration import ENV_TOKEN_NAME @@ -103,7 +98,7 @@ def main(): # pylint: disable=too-many-branches,too-many-statements if parsed.action is None: print("No spec provided, cannot bake", file=sys.stderr) sys.exit(ExitCodes.ARGUMENT_ERROR) - bake_command(cli, parsed.action) + cli.bake(parsed.action) sys.exit(ExitCodes.SUCCESS) elif cli.ops is None: # if not spec was found and we weren't baking, we're doomed diff --git a/linodecli/arg_helpers.py b/linodecli/arg_helpers.py index 2fdd431ba..5cd853ea3 100644 --- a/linodecli/arg_helpers.py +++ b/linodecli/arg_helpers.py @@ -2,16 +2,10 @@ """ Argument parser for the linode CLI """ - -import os import sys from importlib import import_module -import requests -import yaml - from linodecli import plugins -from linodecli.exit_codes import ExitCodes from linodecli.helpers import ( register_args_shared, register_debug_arg, @@ -169,24 +163,3 @@ def remove_plugin(plugin_name, config): config.write_config() return f"Plugin {plugin_name} removed", 0 - - -def bake_command(cli, spec_loc): - """ - Handle a bake command from args - """ - try: - if os.path.exists(os.path.expanduser(spec_loc)): - with open(os.path.expanduser(spec_loc), encoding="utf-8") as f: - spec = yaml.safe_load(f.read()) - else: # try to GET it - resp = requests.get(spec_loc, timeout=120) - if resp.status_code == 200: - spec = yaml.safe_load(resp.content) - else: - raise RuntimeError(f"Request failed to {spec_loc}") - except Exception as e: - print(f"Could not load spec: {e}", file=sys.stderr) - sys.exit(ExitCodes.REQUEST_FAILED) - - cli.bake(spec) diff --git a/linodecli/baked/parsing.py b/linodecli/baked/parsing.py index 5125e45dc..dd41af5ef 100644 --- a/linodecli/baked/parsing.py +++ b/linodecli/baked/parsing.py @@ -9,7 +9,7 @@ # Sentence delimiter, split on a period followed by any type of # whitespace (space, new line, tab, etc.) -REGEX_SENTENCE_DELIMITER = re.compile(r"\.(?:\s|$)") +REGEX_SENTENCE_DELIMITER = re.compile(r"\W(?:\s|$)") # Matches on pattern __prefix__ at the beginning of a description # or after a comma diff --git a/linodecli/cli.py b/linodecli/cli.py index 94f423ad6..775a11e49 100644 --- a/linodecli/cli.py +++ b/linodecli/cli.py @@ -2,11 +2,17 @@ Responsible for managing spec and routing commands to operations. """ +import contextlib +import json import os import pickle import sys +from json import JSONDecodeError from sys import version_info +from typing import IO, Any, ContextManager, Dict +import requests +import yaml from openapi3 import OpenAPI from linodecli.api_request import do_request, get_all_pages @@ -40,11 +46,19 @@ def __init__(self, version, base_url, skip_config=False): self.config = CLIConfig(self.base_url, skip_config=skip_config) self.load_baked() - def bake(self, spec): + def bake(self, spec_location: str): """ - Generates ops and bakes them to a pickle + Generates ops and bakes them to a pickle. + + :param spec_location: The URL or file path of the OpenAPI spec to parse. """ - spec = OpenAPI(spec) + + try: + spec = self._load_openapi_spec(spec_location) + except Exception as e: + print(f"Failed to load spec: {e}") + sys.exit(ExitCodes.REQUEST_FAILED) + self.spec = spec self.ops = {} ext = { @@ -206,3 +220,85 @@ def user_agent(self) -> str: f"linode-api-docs/{self.spec_version} " f"python/{version_info[0]}.{version_info[1]}.{version_info[2]}" ) + + @staticmethod + def _load_openapi_spec(spec_location: str) -> OpenAPI: + """ + Attempts to load the raw OpenAPI spec (YAML or JSON) at the given location. + + :param spec_location: The location of the OpenAPI spec. + This can be a local path or a URL. + + :returns: A tuple containing the loaded OpenAPI object and the parsed spec in + dict format. + """ + + with CLI._get_spec_file_reader(spec_location) as f: + parsed = CLI._parse_spec_file(f) + + return OpenAPI(parsed) + + @staticmethod + @contextlib.contextmanager + def _get_spec_file_reader( + spec_location: str, + ) -> ContextManager[IO]: + """ + Returns a reader for an OpenAPI spec file from the given location. + + :param spec_location: The location of the OpenAPI spec. + This can be a local path or a URL. + + :returns: A context manager yielding the spec file's reader. + """ + + # Case for local file + local_path = os.path.expanduser(spec_location) + if os.path.exists(local_path): + f = open(local_path, "r", encoding="utf-8") + + try: + yield f + finally: + f.close() + + return + + # Case for remote file + resp = requests.get(spec_location, stream=True, timeout=120) + if resp.status_code != 200: + raise RuntimeError(f"Failed to GET {spec_location}") + + # We need to access the underlying urllib + # response here so we can return a reader + # usable in yaml.safe_load(...) and json.load(...) + resp.raw.decode_content = True + + try: + yield resp.raw + finally: + resp.close() + + @staticmethod + def _parse_spec_file(reader: IO) -> Dict[str, Any]: + """ + Parses the given file reader into a dict and returns a dict. + + :param reader: A reader for a YAML or JSON file. + + :returns: The parsed file. + """ + + errors = [] + + try: + return yaml.safe_load(reader) + except yaml.YAMLError as err: + errors.append(str(err)) + + try: + return json.load(reader) + except JSONDecodeError as err: + errors.append(str(err)) + + raise ValueError(f"Failed to parse spec file: {'; '.join(errors)}") diff --git a/tests/fixtures/cli_test_load.json b/tests/fixtures/cli_test_load.json new file mode 100644 index 000000000..ba0a55a87 --- /dev/null +++ b/tests/fixtures/cli_test_load.json @@ -0,0 +1,95 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "API Specification", + "version": "1.0.0" + }, + "servers": [ + { + "url": "http://localhost/v4" + } + ], + "paths": { + "/foo/bar": { + "get": { + "summary": "get info", + "operationId": "fooBarGet", + "description": "This is description", + "responses": { + "200": { + "description": "Successful response", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "data": { + "type": "array", + "items": { + "$ref": "#/components/schemas/OpenAPIResponseAttr" + } + }, + "page": { + "$ref": "#/components/schemas/PaginationEnvelope/properties/page" + }, + "pages": { + "$ref": "#/components/schemas/PaginationEnvelope/properties/pages" + }, + "results": { + "$ref": "#/components/schemas/PaginationEnvelope/properties/results" + } + } + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "OpenAPIResponseAttr": { + "type": "object", + "properties": { + "filterable_result": { + "x-linode-filterable": true, + "type": "string", + "description": "Filterable result value" + }, + "filterable_list_result": { + "x-linode-filterable": true, + "type": "array", + "items": { + "type": "string" + }, + "description": "Filterable result value" + } + } + }, + "PaginationEnvelope": { + "type": "object", + "properties": { + "pages": { + "type": "integer", + "readOnly": true, + "description": "The total number of pages.", + "example": 1 + }, + "page": { + "type": "integer", + "readOnly": true, + "description": "The current page.", + "example": 1 + }, + "results": { + "type": "integer", + "readOnly": true, + "description": "The total number of results.", + "example": 1 + } + } + } + } + } +} diff --git a/tests/fixtures/cli_test_load.yaml b/tests/fixtures/cli_test_load.yaml new file mode 100644 index 000000000..f7dd7704e --- /dev/null +++ b/tests/fixtures/cli_test_load.yaml @@ -0,0 +1,64 @@ +openapi: 3.0.1 +info: + title: API Specification + version: 1.0.0 +servers: + - url: http://localhost/v4 +paths: + /foo/bar: + get: + summary: get info + operationId: fooBarGet + description: This is description + responses: + '200': + description: Successful response + content: + application/json: + schema: + type: object + properties: + data: + type: array + items: + $ref: '#/components/schemas/OpenAPIResponseAttr' + page: + $ref: '#/components/schemas/PaginationEnvelope/properties/page' + pages: + $ref: '#/components/schemas/PaginationEnvelope/properties/pages' + results: + $ref: '#/components/schemas/PaginationEnvelope/properties/results' + +components: + schemas: + OpenAPIResponseAttr: + type: object + properties: + filterable_result: + x-linode-filterable: true + type: string + description: Filterable result value + filterable_list_result: + x-linode-filterable: true + type: array + items: + type: string + description: Filterable result value + PaginationEnvelope: + type: object + properties: + pages: + type: integer + readOnly: true + description: The total number of pages. + example: 1 + page: + type: integer + readOnly: true + description: The current page. + example: 1 + results: + type: integer + readOnly: true + description: The total number of results. + example: 1 \ No newline at end of file diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index fef49ab27..04fc5e79c 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,5 +1,7 @@ import configparser -from typing import List +import contextlib +import os +from typing import ContextManager, List, TextIO import pytest from openapi3 import OpenAPI @@ -23,6 +25,25 @@ LOADED_FILES = {} +FIXTURES_PATH = "tests/fixtures" + + +@contextlib.contextmanager +def open_fixture(filename: str) -> ContextManager[TextIO]: + """ + Gets the reader for a given fixture. + + :returns: A context manager yielding the fixture's reader. + """ + + f = open(os.path.join(FIXTURES_PATH, filename), "r") + + try: + yield f + finally: + f.close() + + def _get_parsed_yaml(filename): """ Returns a python dict that is a parsed yaml file from the tests/fixtures @@ -33,8 +54,9 @@ def _get_parsed_yaml(filename): :type filename: str """ if filename not in LOADED_FILES: - with open("tests/fixtures/" + filename) as f: + with open_fixture(filename) as f: raw = f.read() + parsed = safe_load(raw) LOADED_FILES[filename] = parsed diff --git a/tests/unit/test_arg_helpers.py b/tests/unit/test_arg_helpers.py index e5ad7b3a8..3743061c2 100644 --- a/tests/unit/test_arg_helpers.py +++ b/tests/unit/test_arg_helpers.py @@ -1,5 +1,4 @@ #!/usr/local/bin/python3 -import pytest from linodecli import arg_helpers @@ -150,10 +149,3 @@ def test_remove_plugin_not_available(self, mocked_config): msg, code = arg_helpers.remove_plugin("testing.plugin", mocked_config) assert "not a registered plugin" in msg assert code == 14 - - def test_bake_command_bad_website(self, capsys, mock_cli): - with pytest.raises(SystemExit) as ex: - arg_helpers.bake_command(mock_cli, "https://website.com") - captured = capsys.readouterr() - assert ex.value.code == 2 - assert "Request failed to https://website.com" in captured.out diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index b7f41a79c..e9c2189dc 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -2,12 +2,16 @@ import copy import math +import os import re import pytest import requests +import requests_mock from pytest import MonkeyPatch +from tests.unit.conftest import FIXTURES_PATH, open_fixture + if True: from linodecli import CLI from linodecli.api_request import get_all_pages @@ -78,6 +82,46 @@ def test_user_agent(self, mock_cli: CLI): r"linode-cli/[0-9]+\.[0-9]+\.[0-9]+ linode-api-docs/[0-9]+\.[0-9]+\.[0-9]+ python/[0-9]+\.[0-9]+\.[0-9]+" ).match(mock_cli.user_agent) + def test_load_openapi_spec_json(self): + url_base = "https://localhost/" + path = "cli_test_load.json" + url = f"{url_base}{path}" + + with open_fixture(path) as f: + content = f.read() + + with requests_mock.Mocker() as m: + m.get(url, text=content) + + parsed_json_local = CLI._load_openapi_spec( + str(os.path.join(FIXTURES_PATH, path)) + ) + + parsed_json_http = CLI._load_openapi_spec(url) + + assert m.call_count == 1 + assert parsed_json_http.raw_element == parsed_json_local.raw_element + + def test_load_openapi_spec_yaml(self): + url_base = "https://localhost/" + path = "cli_test_load.yaml" + url = f"{url_base}{path}" + + with open_fixture(path) as f: + content = f.read() + + with requests_mock.Mocker() as m: + m.get(url, text=content) + + parsed_json_local = CLI._load_openapi_spec( + str(os.path.join(FIXTURES_PATH, path)) + ) + + parsed_json_http = CLI._load_openapi_spec(url) + + assert m.call_count == 1 + assert parsed_json_http.raw_element == parsed_json_local.raw_element + def test_get_all_pages( mock_cli: CLI, list_operation: OpenAPIOperation, monkeypatch: MonkeyPatch