Remove resolve_spec_url; remove GITHUB_TOKEN from workflows and Dockerfile

lgarber-akamai · lgarber-akamai · commit f62c0b1e6abf · 2024-07-25T10:01:57.000-04:00
Add logic to load from JSON; point SPEC at prospective URL
diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml
@@ -15,6 +15,6 @@ jobs:
     steps:
     - uses: actions/checkout@v3
     - name: Build the Docker image
-      run: docker build . --file Dockerfile --tag linode/cli:$(date +%s) --build-arg="github_token=$GITHUB_TOKEN"
+      run: docker build . --file Dockerfile --tag linode/cli:$(date +%s)
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/e2e-suite-pr.yml b/.github/workflows/e2e-suite-pr.yml
@@ -72,8 +72,6 @@ jobs:
 
       - name: Install the CLI
         run: make install
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - run: make INTEGRATION_TEST_PATH="${{ inputs.test_path }}" testint
         if: ${{ steps.validate-tests.outputs.match == '' }}
@@ -162,8 +160,6 @@ jobs:
 
       - name: Install the CLI
         run: make install
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - run: make INTEGRATION_TEST_PATH="${{ inputs.test_path }}" testint
         env:
diff --git a/.github/workflows/e2e-suite.yml b/.github/workflows/e2e-suite.yml
@@ -58,8 +58,6 @@ jobs:
 
       - name: Install Package
         run: make install
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Set LINODE_CLI_TOKEN
         run: |
diff --git a/.github/workflows/nightly-smoke-tests.yml b/.github/workflows/nightly-smoke-tests.yml
@@ -25,8 +25,6 @@ jobs:
 
       - name: Install Linode CLI
         run: make install
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Run smoke tests
         run: |
diff --git a/.github/workflows/publish-oci.yml b/.github/workflows/publish-oci.yml
@@ -57,4 +57,3 @@ jobs:
           tags: linode/cli:${{ steps.cli_version.outputs.result }},linode/cli:latest
           build-args: |
             linode_cli_version=${{ steps.cli_version.outputs.result }}
-            github_token=${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/publish-pypi.yml b/.github/workflows/publish-pypi.yml
@@ -30,7 +30,6 @@ jobs:
       - name: Build the package
         run: make build
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           LINODE_CLI_VERSION: ${{ github.event.release.tag_name }}
 
       - name: Publish the release artifacts to PyPI
diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml
@@ -29,8 +29,6 @@ jobs:
 
       - name: Install Package
         run: make install
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Run the unit test suite
         run: make test
@@ -59,8 +57,6 @@ jobs:
         shell: pwsh
         run: | 
           make install
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Run the unit test suite
         run: make test
diff --git a/Dockerfile b/Dockerfile
@@ -1,7 +1,6 @@
 FROM python:3.11-slim AS builder
 
 ARG linode_cli_version
-ARG github_token
 
 WORKDIR /src
 
@@ -12,7 +11,7 @@ COPY . .
 
 RUN make requirements
 
-RUN LINODE_CLI_VERSION=$linode_cli_version GITHUB_TOKEN=$github_token make build
+RUN LINODE_CLI_VERSION=$linode_cli_version make build
 
 FROM python:3.11-slim
 
diff --git a/Makefile b/Makefile
@@ -1,6 +1,13 @@
 #
 # Makefile for more convenient building of the Linode CLI and its baked content
 #
+
+# The URL of the raw OpenAPI spec to build linode-cli with.
+# TODO: Uncomment before merging
+# SPEC := https://raw.githubusercontent.com/akamai/akamai-apis/main/apis/linode-api/v4/openapi.json
+SPEC := https://gist.githubusercontent.com/lgarber-akamai/1c84a451231195d63e990a89f45788fc/raw/3d4c6c677c06bd28d90ec6dd994c7305da0839af/openapi-new.json
+
+# Test-related arguments
 MODULE :=
 TEST_CASE_COMMAND :=
 TEST_ARGS :=
@@ -9,12 +16,6 @@ ifdef TEST_CASE
 TEST_CASE_COMMAND = -k $(TEST_CASE)
 endif
 
-
-SPEC_VERSION ?= latest
-ifndef SPEC
-override SPEC = $(shell ./resolve_spec_url ${SPEC_VERSION})
-endif
-
 # Version-related variables
 VERSION_FILE := ./linodecli/version.py
 VERSION_MODULE_DOCSTRING ?= \"\"\"\nThe version of the Linode CLI.\n\"\"\"\n\n
diff --git 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
@@ -102,7 +97,7 @@ def main():  # pylint: disable=too-many-branches,too-many-statements
         if parsed.action is None:
             print("No spec provided, cannot bake")
             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
@@ -3,15 +3,9 @@
 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,
@@ -166,24 +160,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}")
-        sys.exit(ExitCodes.REQUEST_FAILED)
-
-    cli.bake(spec)
diff --git 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 = {
@@ -205,3 +219,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/resolve_spec_url b/resolve_spec_url
diff --git a/tests/fixtures/cli_test_load.json b/tests/fixtures/cli_test_load.json
diff --git a/tests/fixtures/cli_test_load.yaml b/tests/fixtures/cli_test_load.yaml
diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py
diff --git a/tests/unit/test_arg_helpers.py b/tests/unit/test_arg_helpers.py
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
