From 7bdf7300521893b3fc03f36326a3ce5473fff37c Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Thu, 18 May 2023 15:11:03 -0400 Subject: [PATCH 01/13] fix: Resolve issues with absolute paths in OBJ plugin and fix warning suppression logic (#458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description This pull request resolves an issue that would cause the CLI to crash (or 403) if an absolute path was provided when getting an object. Additionally, this change adds support for the `--suppress-warnings` argument in the OBJ plugin. Resolves #451 ## ✔️ How to Test ```bash make test pytest tests/integration/test_obj_plugin.py ``` --- linodecli/arg_helpers.py | 6 ------ linodecli/helpers.py | 7 ++++++ linodecli/plugins/obj/__init__.py | 36 +++++++++++++++++++------------ linodecli/plugins/obj/buckets.py | 8 +++++-- linodecli/plugins/obj/objects.py | 34 ++++++++++++++++++++++++++--- linodecli/plugins/obj/website.py | 12 ++++++++--- requirements.txt | 1 + 7 files changed, 76 insertions(+), 28 deletions(-) diff --git a/linodecli/arg_helpers.py b/linodecli/arg_helpers.py index 7512600ac..206bfb04c 100644 --- a/linodecli/arg_helpers.py +++ b/linodecli/arg_helpers.py @@ -104,12 +104,6 @@ def register_args(parser): help="Suppress default values for arguments. Default values " "are configured on initial setup or with linode-cli configure", ) - parser.add_argument( - "--suppress-warnings", - action="store_true", - help="Suppress warnings that are intended for human users. " - "This is useful for scripting the CLI's behavior.", - ) parser.add_argument( "--no-truncation", action="store_true", diff --git a/linodecli/helpers.py b/linodecli/helpers.py index a056dd159..6b5bf80a7 100644 --- a/linodecli/helpers.py +++ b/linodecli/helpers.py @@ -72,6 +72,13 @@ def register_args_shared(parser): "be configured.", ) + parser.add_argument( + "--suppress-warnings", + action="store_true", + help="Suppress warnings that are intended for human users. " + "This is useful for scripting the CLI's behavior.", + ) + return parser diff --git a/linodecli/plugins/obj/__init__.py b/linodecli/plugins/obj/__init__.py index 37b5a13af..407737f9a 100644 --- a/linodecli/plugins/obj/__init__.py +++ b/linodecli/plugins/obj/__init__.py @@ -72,8 +72,8 @@ def list_objects_or_buckets( - get_client, args -): # pylint: disable=too-many-locals + get_client, args, **kwargs +): # pylint: disable=too-many-locals,unused-argument """ Lists buckets or objects """ @@ -154,7 +154,7 @@ def list_objects_or_buckets( sys.exit(0) -def generate_url(get_client, args): +def generate_url(get_client, args, **kwargs): # pylint: disable=unused-argument """ Generates a URL to an object """ @@ -205,7 +205,7 @@ def generate_url(get_client, args): print(url) -def set_acl(get_client, args): +def set_acl(get_client, args, **kwargs): # pylint: disable=unused-argument """ Modify Access Control List for a Bucket or Objects """ @@ -265,7 +265,7 @@ def set_acl(get_client, args): print("ACL updated") -def show_usage(get_client, args): +def show_usage(get_client, args, **kwargs): # pylint: disable=unused-argument """ Shows space used by all buckets in this cluster, and total space """ @@ -321,7 +321,9 @@ def show_usage(get_client, args): sys.exit(0) -def list_all_objects(get_client, args): +def list_all_objects( + get_client, args, **kwargs +): # pylint: disable=unused-argument """ Lists all objects in all buckets """ @@ -445,18 +447,20 @@ def get_credentials(cli: CLI): return access_key, secret_key -def regenerate_s3_credentials(cli: CLI): +def regenerate_s3_credentials(cli: CLI, suppress_warnings=False): """ Force regenerate object storage access key and secret key. """ print("Regenerating Object Storage keys..") _get_s3_creds(cli, force=True) print("Done.") - print( - "Warning: Your old Object Storage keys _were not_ automatically expired! If you want " - "to expire them, see `linode-cli object-storage keys-list` and " - "`linode-cli object-storage keys-delete [KEYID]`." - ) + + if not suppress_warnings: + print( + "WARNING: Your old Object Storage keys _were not_ automatically expired! If you want " + "to expire them, see `linode-cli object-storage keys-list` and " + "`linode-cli object-storage keys-delete [KEYID]`." + ) def call( @@ -524,11 +528,15 @@ def get_client(): if parsed.command in COMMAND_MAP: try: - COMMAND_MAP[parsed.command](get_client, args) + COMMAND_MAP[parsed.command]( + get_client, args, suppress_warnings=parsed.suppress_warnings + ) except ClientError as e: sys.exit(f"Error: {e}") elif parsed.command == "regenerate-keys": - regenerate_s3_credentials(context.client) + regenerate_s3_credentials( + context.client, suppress_warnings=parsed.suppress_warnings + ) elif parsed.command == "configure": _configure_plugin(context.client) else: diff --git a/linodecli/plugins/obj/buckets.py b/linodecli/plugins/obj/buckets.py index e4ff8da56..fe6f8ca00 100644 --- a/linodecli/plugins/obj/buckets.py +++ b/linodecli/plugins/obj/buckets.py @@ -8,7 +8,9 @@ from linodecli.plugins.obj.config import PLUGIN_BASE -def create_bucket(get_client, args): +def create_bucket( + get_client, args, **kwargs +): # pylint: disable=unused-argument """ Creates a new bucket """ @@ -30,7 +32,9 @@ def create_bucket(get_client, args): sys.exit(0) -def delete_bucket(get_client, args): +def delete_bucket( + get_client, args, **kwargs +): # pylint: disable=unused-argument """ Deletes a bucket """ diff --git a/linodecli/plugins/obj/objects.py b/linodecli/plugins/obj/objects.py index a5ae5316c..9a06e6c12 100644 --- a/linodecli/plugins/obj/objects.py +++ b/linodecli/plugins/obj/objects.py @@ -23,7 +23,9 @@ ) -def upload_object(get_client, args): # pylint: disable=too-many-locals +def upload_object( + get_client, args, **kwargs +): # pylint: disable=too-many-locals,unused-argument """ Uploads an object to object storage """ @@ -101,7 +103,11 @@ def upload_object(get_client, args): # pylint: disable=too-many-locals print("Done.") -def get_object(get_client, args): +# We can't parse suppress_warnings from the parser +# because it is handled at the top-level of this plugin. +def get_object( + get_client, args, suppress_warnings=False, **kwargs +): # pylint: disable=unused-argument """ Retrieves an uploaded object and writes it to a file """ @@ -137,6 +143,26 @@ def get_object(get_client, args): bucket = parsed.bucket key = parsed.file + # Keys should always be relative + if key.startswith("/"): + if parsed.destination is None and not suppress_warnings: + print( + f'WARNING: This file will be saved to the absolute path "{key}".\n' + "If you would like to store this file in a relative path, use the LOCAL_FILE " + "parameter or remove the trailing slash character from the object name.", + file=sys.stderr, + ) + key = key[1:] + + destination_parent = destination.parent + + # In the future we should allow the automatic creation of parent directories + if not destination_parent.exists(): + print( + f"ERROR: Output directory {destination_parent} does not exist locally." + ) + sys.exit(1) + response = client.head_object( Bucket=bucket, Key=key, @@ -154,7 +180,9 @@ def get_object(get_client, args): print("Done.") -def delete_object(get_client, args): +def delete_object( + get_client, args, **kwargs +): # pylint: disable=unused-argument """ Removes a file from a bucket """ diff --git a/linodecli/plugins/obj/website.py b/linodecli/plugins/obj/website.py index 0005d14b9..9d55be173 100644 --- a/linodecli/plugins/obj/website.py +++ b/linodecli/plugins/obj/website.py @@ -8,7 +8,9 @@ from linodecli.plugins.obj.config import BASE_WEBSITE_TEMPLATE, PLUGIN_BASE -def enable_static_site(get_client, args): +def enable_static_site( + get_client, args, **kwargs +): # pylint: disable=unused-argument """ Turns a bucket into a static website """ @@ -66,7 +68,9 @@ def enable_static_site(get_client, args): ) -def static_site_info(get_client, args): +def static_site_info( + get_client, args, **kwargs +): # pylint: disable=unused-argument """ Returns info about a configured static site """ @@ -99,7 +103,9 @@ def static_site_info(get_client, args): print(f"Error document: {error}") -def disable_static_site(get_client, args): +def disable_static_site( + get_client, args, **kwargs +): # pylint: disable=unused-argument """ Disables static site for a bucket """ diff --git a/requirements.txt b/requirements.txt index f89688ee4..33f06a7d2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,3 +2,4 @@ requests PyYAML packaging rich +urllib3<2 From 80e4c6da1b06800d940afbceccb0d988f0d3ea38 Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Fri, 19 May 2023 14:01:09 -0400 Subject: [PATCH 02/13] new: Improve handling for list structures (#456) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description This change improves the handling for nested list structures (e.g. Instance Interfaces) by implicitly detecting when the next list entry should be populated. This means that users do not have to explicitly specify empty values for null fields. For example, see this command: ```bash linode-cli linodes config-update 1234 5678 \ --interfaces.purpose public \ --interfaces.purpose vlan --interfaces.label my-vlan ``` A user would intuitively expect this command to set the first interface to public and the second interface to a VLAN with the label `my-vlan`, however executing this command would generate this request body: ```json { "interfaces":[ { "label":"cool", "purpose":"public" } ] } ``` After this change, the request body is now generated as expected: ```json { "interfaces":[ { "label":null, "ipam_address":null, "purpose":"public" }, { "label":"cool", "purpose":"vlan" } ] } ``` ## ✔️ How to Test ``` make test ``` --- README.rst | 10 +++++ linodecli/operation.py | 54 +++++++++++++++++++++++- tests/unit/test_operation.py | 80 ++++++++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_operation.py diff --git a/README.rst b/README.rst index 539fc193b..f8b14257e 100644 --- a/README.rst +++ b/README.rst @@ -179,6 +179,16 @@ you can execute the following:: linode-cli linodes create --region us-east --type g6-nanode-1 --tags tag1 --tags tag2 +Lists consisting of nested structures can also be expressed through the command line. +For example, to create a Linode with a public interface on ``eth0`` and a VLAN interface +on ``eth1`` you can execute the following:: + + linode-cli linodes create \ + --region us-east --type g6-nanode-1 --image linode/ubuntu22.04 \ + --root_pass "myr00tp4ss123" \ + --interfaces.purpose public \ + --interfaces.purpose vlan --interfaces.label my-vlan + Specifying Nested Arguments """"""""""""""""""""""""""" diff --git a/linodecli/operation.py b/linodecli/operation.py index a06c3671e..4092ee424 100644 --- a/linodecli/operation.py +++ b/linodecli/operation.py @@ -100,6 +100,58 @@ def __call__(self, parser, namespace, values, option_string=None): raise argparse.ArgumentTypeError("Expected a string") +class ListArgumentAction(argparse.Action): + """ + This action is intended to be used only with list arguments. + Its purpose is to aggregate adjacent object fields and produce consistent + lists in the output namespace. + """ + + def __call__(self, parser, namespace, values, option_string=None): + if getattr(namespace, self.dest) is None: + setattr(namespace, self.dest, []) + + dest_list = getattr(namespace, self.dest) + dest_length = len(dest_list) + dest_parent = self.dest.split(".")[:-1] + + # If this isn't a nested structure, + # append and return early + if len(dest_parent) < 1: + dest_list.append(values) + return + + # A list of adjacent fields + adjacent_keys = [ + k + for k in vars(namespace).keys() + if k.split(".")[:-1] == dest_parent + ] + + # Let's populate adjacent fields ahead of time + for k in adjacent_keys: + if getattr(namespace, k) is None: + setattr(namespace, k, []) + + adjacent_items = {k: getattr(namespace, k) for k in adjacent_keys} + + # Find the deepest field so we can know if + # we're starting a new object. + deepest_length = max(len(x) for x in adjacent_items.values()) + + # If we're creating a new list object, append + # None to every non-populated field. + if dest_length >= deepest_length: + for k, item in adjacent_items.items(): + if k == self.dest: + continue + + if len(item) < dest_length: + item.append(None) + + dest_list.append(values) + + TYPES = { "string": str, "integer": int, @@ -244,7 +296,7 @@ def parse_args( parser.add_argument( "--" + arg.path, metavar=arg.name, - action="append", + action=ListArgumentAction, type=TYPES[arg.arg_type], ) list_items.append((arg.path, arg.list_item)) diff --git a/tests/unit/test_operation.py b/tests/unit/test_operation.py new file mode 100644 index 000000000..b3b707dd2 --- /dev/null +++ b/tests/unit/test_operation.py @@ -0,0 +1,80 @@ +import argparse + +from linodecli import operation + + +class TestOperation: + def test_list_arg_action_basic(self): + """ + Tests a basic list argument condition. + """ + + parser = argparse.ArgumentParser( + prog=f"foo", + ) + + for arg_name in ["foo", "bar", "aaa"]: + parser.add_argument( + f"--foo.{arg_name}", + metavar=arg_name, + action=operation.ListArgumentAction, + type=str, + ) + + result = parser.parse_args( + [ + "--foo.foo", + "cool", + "--foo.bar", + "wow", + "--foo.aaa", + "computer", + "--foo.foo", + "test", + "--foo.bar", + "wow", + "--foo.aaa", + "akamai", + ] + ) + assert getattr(result, "foo.foo") == ["cool", "test"] + assert getattr(result, "foo.bar") == ["wow", "wow"] + assert getattr(result, "foo.aaa") == ["computer", "akamai"] + + def test_list_arg_action_missing_attr(self): + """ + Tests that a missing attribute for the first element will be + implicitly populated. + """ + + parser = argparse.ArgumentParser( + prog=f"foo", + ) + + for arg_name in ["foo", "bar", "aaa"]: + parser.add_argument( + f"--foo.{arg_name}", + metavar=arg_name, + action=operation.ListArgumentAction, + type=str, + ) + + result = parser.parse_args( + [ + "--foo.foo", + "cool", + "--foo.aaa", + "computer", + "--foo.foo", + "test", + "--foo.bar", + "wow", + "--foo.foo", + "linode", + "--foo.aaa", + "akamai", + ] + ) + assert getattr(result, "foo.foo") == ["cool", "test", "linode"] + assert getattr(result, "foo.bar") == [None, "wow"] + assert getattr(result, "foo.aaa") == ["computer", None, "akamai"] From 4cdacf84f144ed0f218e26de5a8ebed164b45dad Mon Sep 17 00:00:00 2001 From: Zhiwei Liang <121905282+zliang-akamai@users.noreply.github.com> Date: Tue, 23 May 2023 14:14:47 -0400 Subject: [PATCH 03/13] Add `--all-rows` Flag (#455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description Provide a option for user to output all results without pagination. Should we add some integration/unit tests for this? Will be working on that if necessary. ## ✔️ How to Test Create 1000 stackscripts in your account then run `linode --all-rows stackscripts ls --is_public false` --------- Signed-off-by: Zhiwei Liang --- linodecli/__init__.py | 13 ++++++-- linodecli/api_request.py | 9 +++++- linodecli/arg_helpers.py | 14 ++++++-- linodecli/cli.py | 70 +++++++++++++++++++++++++++++++++++++--- linodecli/helpers.py | 9 +++++- linodecli/output.py | 14 ++++++-- tests/unit/test_cli.py | 53 +++++++++++++++++++++++++++++- 7 files changed, 167 insertions(+), 15 deletions(-) diff --git a/linodecli/__init__.py b/linodecli/__init__.py index b9aaffcb9..2e3f5f88d 100755 --- a/linodecli/__init__.py +++ b/linodecli/__init__.py @@ -77,14 +77,23 @@ def main(): # pylint: disable=too-many-branches,too-many-statements cli.output_handler.columns = "*" if parsed.no_headers: cli.output_handler.headers = False - if parsed.all: - cli.output_handler.columns = "*" + if parsed.all_rows: + cli.pagination = False elif parsed.format: cli.output_handler.columns = parsed.format cli.defaults = not parsed.no_defaults cli.suppress_warnings = parsed.suppress_warnings + if parsed.all_columns or parsed.all: + if parsed.all and not cli.suppress_warnings: + print( + "WARNING: '--all' is a deprecated flag, " + "and will be removed in a future version. " + "Please consider use '--all-columns' instead." + ) + cli.output_handler.columns = "*" + cli.page = parsed.page cli.page_size = parsed.page_size cli.debug_request = parsed.debug diff --git a/linodecli/api_request.py b/linodecli/api_request.py index 9a5fa0f01..8a5ba2da8 100644 --- a/linodecli/api_request.py +++ b/linodecli/api_request.py @@ -9,10 +9,17 @@ import requests from packaging import version +from requests import Response def do_request( - ctx, operation, args, filter_header=None, skip_error_handling=False + ctx, + operation, + args, + filter_header=None, + skip_error_handling=False, +) -> ( + Response ): # pylint: disable=too-many-locals,too-many-branches,too-many-statements """ Makes a request to an operation's URL and returns the resulting JSON, or diff --git a/linodecli/arg_helpers.py b/linodecli/arg_helpers.py index 206bfb04c..207dcca08 100644 --- a/linodecli/arg_helpers.py +++ b/linodecli/arg_helpers.py @@ -88,8 +88,18 @@ def register_args(parser): parser.add_argument( "--all", action="store_true", - help="If set, displays all possible columns instead of " - "the default columns. This may not work well on some terminals.", + help=( + "Deprecated flag. An alias of '--all-columns', " + "scheduled to be removed in a future version." + ), + ) + parser.add_argument( + "--all-columns", + action="store_true", + help=( + "If set, displays all possible columns instead of " + "the default columns. This may not work well on some terminals." + ), ) parser.add_argument( "--format", diff --git a/linodecli/cli.py b/linodecli/cli.py index 75955f8c5..b3bf17881 100644 --- a/linodecli/cli.py +++ b/linodecli/cli.py @@ -2,11 +2,13 @@ Responsible for managing spec and routing commands to operations. """ +import itertools import os import pickle import re import sys from sys import version_info +from typing import Iterable, List from .api_request import do_request from .configuration import CLIConfig @@ -27,6 +29,7 @@ def __init__(self, version, base_url, skip_config=False): self.ops = {} self.spec = {} self.defaults = True # whether to use default values for arguments + self.pagination = True self.page = 1 self.page_size = 100 self.debug_request = False @@ -479,6 +482,60 @@ def _flatten_url_path(tag): new_tag = re.sub(r"[^a-z ]", "", new_tag).replace(" ", "-") return new_tag + @staticmethod + def merge_results_data(results: Iterable[dict]): + """Merge multiple json response into one""" + + iterator = iter(results) + merged_result = next(iterator, None) + if not merged_result: + return None + + if "pages" in merged_result: + merged_result["pages"] = 1 + if "page" in merged_result: + merged_result["page"] = 1 + if "data" in merged_result: + merged_result["data"] += list( + itertools.chain.from_iterable(r["data"] for r in iterator) + ) + return merged_result + + def _get_all_pages_results_generator( + self, + operation: CLIOperation, + args: List[str], + pages_needed: Iterable[int], + ): + for p in pages_needed: + self.page = p + yield do_request(self, operation, args).json() + + def get_all_pages(self, operation: CLIOperation, args: List[str]): + """ + Receive all pages of a resource from multiple + API responses then merge into one page. + """ + + self.page_size = 500 + self.page = 1 + result = do_request(self, operation, args).json() + + total_pages = result.get("pages") + + if total_pages and total_pages > 1: + pages_needed = range(2, total_pages + 1) + + result = self.merge_results_data( + itertools.chain( + (result,), + self._get_all_pages_results_generator( + operation, args, pages_needed + ), + ) + ) + return result + def handle_command(self, command, action, args): """ Given a command, action, and remaining kwargs, finds and executes the @@ -502,17 +559,20 @@ def handle_command(self, command, action, args): print(e, file=sys.stderr) sys.exit(1) - result = do_request(self, operation, args) + if not self.pagination: + result = self.get_all_pages(operation, args) + else: + result = do_request(self, operation, args).json() - operation.process_response_json(result.json(), self.output_handler) + operation.process_response_json(result, self.output_handler) if ( self.output_handler.mode == OutputMode.table - and "pages" in result.json() - and result.json()["pages"] > 1 + and "pages" in result + and result["pages"] > 1 ): print( - f"Page {result.json()['page']} of {result.json()['pages']}. " + f"Page {result['page']} of {result['pages']}. " "Call with --page [PAGE] to load a different page." ) diff --git a/linodecli/helpers.py b/linodecli/helpers.py index 6b5bf80a7..32ec64d17 100644 --- a/linodecli/helpers.py +++ b/linodecli/helpers.py @@ -5,6 +5,7 @@ import glob import os import re +from argparse import ArgumentParser from pathlib import Path from urllib.parse import urlparse @@ -55,7 +56,7 @@ def filter_markdown_links(text): return result -def register_args_shared(parser): +def register_args_shared(parser: ArgumentParser): """ Adds certain arguments to the given ArgumentParser that may be shared across the CLI and plugins. @@ -79,6 +80,12 @@ def register_args_shared(parser): "This is useful for scripting the CLI's behavior.", ) + parser.add_argument( + "--all-rows", + action="store_true", + help="Output all possible rows in the results with pagination", + ) + return parser diff --git a/linodecli/output.py b/linodecli/output.py index bf4cb35e1..77e9f1e33 100644 --- a/linodecli/output.py +++ b/linodecli/output.py @@ -5,12 +5,15 @@ import sys from enum import Enum from sys import stdout +from typing import IO, List, Optional, Union from rich import box from rich import print as rprint from rich.table import Table from rich.text import Text +from linodecli.response import ResponseModel + class OutputMode(Enum): """ @@ -52,7 +55,12 @@ def __init__( # pylint: disable=too-many-arguments self.has_warned = False def print( - self, response_model, data, title=None, to=stdout, columns=None + self, + response_model: ResponseModel, + data: List[Union[str, dict]], + title: Optional[str] = None, + to: IO[str] = stdout, + columns: Optional[List[str]] = None, ): # pylint: disable=too-many-arguments """ :param response_model: The Model corresponding to this response @@ -60,11 +68,11 @@ def print( :param data: The data to display :type data: list[str] or list[dict] :param title: The title to display on a table - :type title: str or None + :type title: Optional[str] :param to: Where to print output to :type to: stdout, stderr or file :param columns: The columns to display - :type columns: list[str] + :type columns: Optional[List[str]] """ # We need to use lambdas here since we don't want unused function params diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 27194a89f..be76b2ba7 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -1,6 +1,34 @@ +from __future__ import annotations + import copy +import math +import re +from typing import TYPE_CHECKING import pytest +import requests +from pytest import MonkeyPatch + +if TYPE_CHECKING: + from linodecli import CLI, CLIOperation + + +class MockResponse: + def __init__( + self, page: int, pages: int, results: int, status_code: int = 200 + ): + self.page = page + self.pages = pages + self.results = results + self.status_code = status_code + + def json(self): + return { + "data": ["test_data" for _ in range(500)], + "page": self.page, + "pages": self.pages, + "results": self.results, + } class TestCLI: @@ -8,7 +36,7 @@ class TestCLI: Unit tests for linodecli.cli """ - def test_find_operation(self, mock_cli, list_operation): + def test_find_operation(self, mock_cli: CLI, list_operation: CLIOperation): target_operation = list_operation target_operation.command = "foo" target_operation.action = "list" @@ -35,3 +63,26 @@ def test_find_operation(self, mock_cli, list_operation): with pytest.raises(ValueError, match=r"No action *"): mock_cli.find_operation("foo", "cool") mock_cli.find_operation("cool", "cool") + + +def test_get_all_pages( + mock_cli: CLI, list_operation: CLIOperation, monkeypatch: MonkeyPatch +): + TOTAL_DATA = 2000 + + def mock_get(url: str, *args, **kwargs): + # assume page_size is always 500 + page = int(re.search(r"\?page=(.*?)&page_size", url).group(1)) + pages = math.ceil(TOTAL_DATA / 500) + if page > pages: + page = pages + return MockResponse(page, pages, pages * 500) + + monkeypatch.setattr(requests, "get", mock_get) + + merged_result = mock_cli.get_all_pages(list_operation, []) + + assert len(merged_result["data"]) == TOTAL_DATA + assert merged_result["page"] == 1 + assert merged_result["pages"] == 1 + assert merged_result["results"] == TOTAL_DATA From facb0b0df60143c942058f559e70f2f69731d5a0 Mon Sep 17 00:00:00 2001 From: ykim-1 <126618609+ykim-1@users.noreply.github.com> Date: Thu, 1 Jun 2023 08:16:28 -0700 Subject: [PATCH 04/13] test: Add workflow for Windows (#461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description **What does this PR do and why is this change necessary?** ## ✔️ How to Test Unit tests are executed upon push Integration tests run via /acctest command ## 📷 Preview **If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.** --- .github/workflows/e2e-suite-pr.yml | 68 +++++++++++++++++++++++++++- .github/workflows/unit-tests.yml | 33 +++++++++++++- setup.py | 17 ++++--- tests/integration/ssh/test_ssh.py | 6 ++- tests/integration/test_plugin_ssh.py | 4 +- tests/unit/conftest.py | 6 +++ tests/unit/test_plugin_ssh.py | 12 +++-- 7 files changed, 132 insertions(+), 14 deletions(-) diff --git a/.github/workflows/e2e-suite-pr.yml b/.github/workflows/e2e-suite-pr.yml index 31c3e7434..441e0ee0e 100644 --- a/.github/workflows/e2e-suite-pr.yml +++ b/.github/workflows/e2e-suite-pr.yml @@ -7,7 +7,7 @@ name: PR E2E Tests jobs: # Maintainer has commented /acctest on a pull request - integration-fork: + integration-fork-ubuntu: runs-on: ubuntu-latest if: github.event_name == 'repository_dispatch' && @@ -79,3 +79,69 @@ jobs: conclusion: process.env.conclusion }); return result; + + integration-fork-windows: + runs-on: windows-latest + if: + github.event_name == 'repository_dispatch' && + github.event.client_payload.slash_command.sha != '' && + github.event.client_payload.pull_request.head.sha == github.event.client_payload.slash_command.sha + + steps: + - uses: actions-ecosystem/action-regex-match@v2 + id: validate-tests + with: + text: ${{ github.event.client_payload.slash_command.tests }} + regex: '[^a-z0-9-:.\/_]' # Tests validation + flags: gi + + # Check out merge commit + - name: Checkout PR + uses: actions/checkout@v3 + with: + ref: ${{ github.event.client_payload.slash_command.sha }} + + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: '3.x' + + - name: Install Python deps + run: pip install -r requirements.txt -r requirements-dev.txt wheel boto3 + + - name: Install the CLI + run: make install + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - run: make INTEGRATION_TEST_PATH="${{ github.event.client_payload.slash_command.tests }}" testint + env: + LINODE_CLI_TOKEN: ${{ secrets.LINODE_TOKEN }} + + - uses: actions/github-script@v5 + id: update-check-run + if: ${{ always() }} + env: + number: ${{ github.event.client_payload.pull_request.number }} + job: ${{ github.job }} + conclusion: ${{ job.status }} + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const { data: pull } = await github.rest.pulls.get({ + ...context.repo, + pull_number: process.env.number + }); + const ref = pull.head.sha; + const { data: checks } = await github.rest.checks.listForRef({ + ...context.repo, + ref + }); + const check = checks.check_runs.filter(c => c.name === process.env.job); + const { data: result } = await github.rest.checks.update({ + ...context.repo, + check_run_id: check[0].id, + status: 'completed', + conclusion: process.env.conclusion + }); + return result; \ No newline at end of file diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index fd3229a42..6375953db 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -4,8 +4,7 @@ on: push: pull_request: jobs: - unit-tests: - name: Run unit tests + unit-tests-on-ubuntu: runs-on: ubuntu-latest steps: - name: Clone Repository @@ -35,3 +34,33 @@ jobs: - name: Run the unit test suite run: make test + + unit-tests-on-windows: + runs-on: windows-latest + steps: + - name: Clone Repository + uses: actions/checkout@v3 + + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: '3.x' + + - name: Install Python wheel + run: pip install wheel boto3 + + - name: Update cert + run: pip install certifi -U + + - name: Install deps + run: pip install -r requirements.txt -r requirements-dev.txt + + - name: Install Package + shell: pwsh + run: | + make install + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Run the unit test suite + run: make test \ No newline at end of file diff --git a/setup.py b/setup.py index fe9f65c2b..876ae727c 100755 --- a/setup.py +++ b/setup.py @@ -1,14 +1,17 @@ #!/usr/bin/env python3 +import pathlib import subprocess -from os import path +import sys +import platform from setuptools import setup, find_packages +from os import path -here = path.abspath(path.dirname(__file__)) +here = pathlib.Path().absolute() # get the long description from the README.rst -with open(path.join(here, "README.rst"), encoding="utf-8") as f: +with open(here / "README.rst", encoding="utf-8") as f: long_description = f.read() @@ -20,8 +23,10 @@ def get_baked_files(): """ data_files = [] - if path.isfile("linode-cli.sh"): - data_files.append(("/etc/bash_completion.d", ["linode-cli.sh"])) + completion_dir = "/etc/bash_completion.d" + + if path.isfile("linode-cli.sh") and platform.system() != "Windows": + data_files.append((completion_dir, ["linode-cli.sh"])) return data_files @@ -30,7 +35,7 @@ def get_version(): """ Uses the version file to calculate this package's version """ - return subprocess.check_output(["./version"]).decode("utf-8").rstrip() + return subprocess.check_output([sys.executable, "./version"]).decode("utf-8").rstrip() def get_baked_version(): diff --git a/tests/integration/ssh/test_ssh.py b/tests/integration/ssh/test_ssh.py index 5983d634b..031f2a9a0 100644 --- a/tests/integration/ssh/test_ssh.py +++ b/tests/integration/ssh/test_ssh.py @@ -13,7 +13,11 @@ @pytest.fixture(scope="session", autouse=True) -def test_create_a_linode_in_running_state(ssh_key_pair_generator): +def test_create_a_linode_in_running_state( + ssh_key_pair_generator, platform_os_type +): + if platform_os_type == "Windows": + pytest.skip("This test does not run on Windows") pubkey_file, privkey_file = ssh_key_pair_generator with open(pubkey_file, "r") as f: diff --git a/tests/integration/test_plugin_ssh.py b/tests/integration/test_plugin_ssh.py index 281426ef9..ec1cbe4c5 100644 --- a/tests/integration/test_plugin_ssh.py +++ b/tests/integration/test_plugin_ssh.py @@ -24,7 +24,9 @@ @pytest.fixture -def target_instance(ssh_key_pair_generator): +def target_instance(ssh_key_pair_generator, platform_os_type): + if platform_os_type == "Windows": + pytest.skip("This pluggin is not supported on Windows") instance_label = f"cli-test-{get_random_text(length=6)}" pubkey_file, privkey_file = ssh_key_pair_generator diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 69fcace74..c33ad819b 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,4 +1,5 @@ import configparser +import platform import pytest @@ -171,3 +172,8 @@ def write_config(self): # pylint: disable=missing-function-docstring pass return Config() + + +@pytest.fixture +def platform_os_type(): + return platform.system() diff --git a/tests/unit/test_plugin_ssh.py b/tests/unit/test_plugin_ssh.py index a8ed1f2cd..fd915544c 100644 --- a/tests/unit/test_plugin_ssh.py +++ b/tests/unit/test_plugin_ssh.py @@ -8,7 +8,9 @@ from linodecli.plugins import PluginContext -def test_print_help(capsys: CaptureFixture): +def test_print_help(capsys: CaptureFixture, platform_os_type): + if platform_os_type == "Windows": + pytest.skip("This test does not run on Windows") with pytest.raises(SystemExit) as err: plugin.call(["--help"], None) @@ -30,7 +32,9 @@ def test_windows_error(capsys: CaptureFixture): assert "This plugin is not currently supported in Windows." in captured_text -def test_target_not_running(mock_cli, capsys: CaptureFixture): +def test_target_not_running(mock_cli, capsys: CaptureFixture, platform_os_type): + if platform_os_type == "Windows": + pytest.skip("This test does not run on Windows") test_label = "totally-real-label" def mock_call_operation(*a, filters=None): @@ -53,7 +57,9 @@ def mock_call_operation(*a, filters=None): ) -def test_target_success(mock_cli, capsys: CaptureFixture): +def test_target_success(mock_cli, capsys: CaptureFixture, platform_os_type): + if platform_os_type == "Windows": + pytest.skip("This test does not run on Windows") test_label = "totally-real-label" test_user = "test" test_ip = "123.123.123.123" From 15bf3bee806a037e87d259969faab3b927191042 Mon Sep 17 00:00:00 2001 From: ykim-1 <126618609+ykim-1@users.noreply.github.com> Date: Fri, 2 Jun 2023 08:50:41 -0700 Subject: [PATCH 05/13] update readme file to reflect changes in integration tests (#462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description update readme to reflect migration of integration tests from bats to pytest Also gave some options in Makefile to allow users to run specific test suite/case/package ## ✔️ How to Test **What are the steps to reproduce the issue or verify the changes?** **How do I run the relevant unit/integration tests?** ## 📷 Preview **If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.** --- .github/workflows/e2e-suite-pr.yml | 4 +-- Makefile | 10 ++++-- README.rst | 36 ++++++------------- .../{ => image}/test_plugin_image_upload.py | 0 .../integration/{ => ssh}/test_plugin_ssh.py | 0 tests/{integration => obj}/test_obj_plugin.py | 0 6 files changed, 20 insertions(+), 30 deletions(-) rename tests/integration/{ => image}/test_plugin_image_upload.py (100%) rename tests/integration/{ => ssh}/test_plugin_ssh.py (100%) rename tests/{integration => obj}/test_obj_plugin.py (100%) diff --git a/.github/workflows/e2e-suite-pr.yml b/.github/workflows/e2e-suite-pr.yml index 441e0ee0e..be7f824e8 100644 --- a/.github/workflows/e2e-suite-pr.yml +++ b/.github/workflows/e2e-suite-pr.yml @@ -47,7 +47,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - run: make INTEGRATION_TEST_PATH="${{ github.event.client_payload.slash_command.tests }}" testint + - run: make INTEGRATION_TEST_PATH="${{ github.event.client_payload.slash_command.tests }}" TEST_CASE="${{ github.event.client_payload.slash_command.testcase }}" testint if: ${{ steps.validate-tests.outputs.match == '' }} env: LINODE_CLI_TOKEN: ${{ secrets.LINODE_TOKEN }} @@ -114,7 +114,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - run: make INTEGRATION_TEST_PATH="${{ github.event.client_payload.slash_command.tests }}" testint + - run: make INTEGRATION_TEST_PATH="${{ github.event.client_payload.slash_command.tests }}" TEST_CASE="${{ github.event.client_payload.slash_command.testcase }}" testint env: LINODE_CLI_TOKEN: ${{ secrets.LINODE_TOKEN }} diff --git a/Makefile b/Makefile index ffb5c5464..20ab44a67 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,13 @@ # # Makefile for more convenient building of the Linode CLI and its baked content # - INTEGRATION_TEST_PATH := +TEST_CASE_COMMAND := + +ifdef TEST_CASE +TEST_CASE_COMMAND = -k $(TEST_CASE) +endif + SPEC_VERSION ?= latest ifndef SPEC @@ -49,13 +54,12 @@ testunit: .PHONY: testint testint: - pytest tests/integration/${INTEGRATION_TEST_PATH} + pytest tests/integration/${INTEGRATION_TEST_PATH} ${TEST_CASE_COMMAND} .PHONY: testall testall: pytest tests - # Alias for unit; integration tests should be explicit .PHONY: test test: testunit diff --git a/README.rst b/README.rst index f8b14257e..5efb94a38 100644 --- a/README.rst +++ b/README.rst @@ -422,45 +422,31 @@ Testing with the account. It is only recommended to run these tests if you are an advanced user. -Installation -^^^^^^^^^^^^ - -The CLI uses the Bash Automated Testing System (BATS) for testing. To install run the following: - -**OSX users**:: +Running the Tests +^^^^^^^^^^^^^^^^^ - brew install bats-core +Running the tests locally is simple. The only requirements are that you export Linode API token as LINODE_CLI_TOKEN:: -**Installing Bats from source** + export LINODE_CLI_TOKEN="your_token" -Check out a copy of the Bats repository. Then, either add the Bats bin directory to your -$PATH, or run the provided install.sh command with the location to the prefix in which you -want to install Bats. For example, to install Bats into /usr/local:: - git clone https://github.com/bats-core/bats-core.git - cd bats-core - ./install.sh /usr/local -Running the Tests -^^^^^^^^^^^^^^^^^ +More information on Managing Linode API tokens can be found here - https://www.linode.com/docs/products/tools/api/guides/manage-api-tokens/ -Running the tests is simple. The only requirements are that you have a .linode-cli in your user folder containing your test user token:: +In order to run the full integration test, run:: - ./test/test-runner.sh + make testint -**Running Tests via Docker** +To run specific test package, use environment variable `INTEGRATION_TEST_PATH` with `testint` command:: -The openapi spec must first be saved to the base of the linode-cli project: + make INTEGRATION_TEST_PATH="cli" testint - curl -o ./openapi.yaml https://www.linode.com/docs/api/openapi.yaml -Run the following command to build the tests container: - docker build -f Dockerfile-bats -t linode-cli-tests . +Lastly, to run specific test case, use environment variables `TEST_CASE` with `testint` command:: -Run the following command to run the test + make TEST_CASE=test_help_page_for_non_aliased_actions testint - docker run -e TOKEN_1=$INSERT_YOUR_TOKEN_HERE -e TOKEN_2=$INSERT_YOUR_TOKEN_HERE --rm linode-cli-tests Contributing ------------ diff --git a/tests/integration/test_plugin_image_upload.py b/tests/integration/image/test_plugin_image_upload.py similarity index 100% rename from tests/integration/test_plugin_image_upload.py rename to tests/integration/image/test_plugin_image_upload.py diff --git a/tests/integration/test_plugin_ssh.py b/tests/integration/ssh/test_plugin_ssh.py similarity index 100% rename from tests/integration/test_plugin_ssh.py rename to tests/integration/ssh/test_plugin_ssh.py diff --git a/tests/integration/test_obj_plugin.py b/tests/obj/test_obj_plugin.py similarity index 100% rename from tests/integration/test_obj_plugin.py rename to tests/obj/test_obj_plugin.py From ae01bbd8d87755befe4b1a5dd37efb9ec5d3bc4c Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Fri, 2 Jun 2023 14:19:31 -0400 Subject: [PATCH 06/13] fix: Resolve `--no-headers` flag for tabular output (#463) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description This change resolves an issue with the `--no-headers` flag in table output mode which would cause the first row of content to be dropped. ## ✔️ How to Test ``` make testunit ``` --- linodecli/output.py | 10 ++++------ tests/unit/test_output.py | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/linodecli/output.py b/linodecli/output.py index 77e9f1e33..6e7eb851a 100644 --- a/linodecli/output.py +++ b/linodecli/output.py @@ -139,23 +139,21 @@ def _table_output( content = self._build_output_content( data, columns, - header=header, value_transform=lambda attr, v: self._attempt_truncate_value( attr.render_value(v) ), ) - tab = Table(*content[0], header_style="", box=box.SQUARE) - for row in content[1:]: + tab = Table( + *header, header_style="", box=box.SQUARE, show_header=self.headers + ) + for row in content: row = [Text.from_ansi(item) for item in row] tab.add_row(*row) if title is not None: tab.title = title - if not self.headers: - tab.show_header = False - rprint(tab, file=to) def _delimited_output(self, header, data, columns, to): diff --git a/tests/unit/test_output.py b/tests/unit/test_output.py index 19aa11823..cbb79a588 100644 --- a/tests/unit/test_output.py +++ b/tests/unit/test_output.py @@ -162,6 +162,32 @@ def test_table_output_models(self, mock_cli): assert output.getvalue() == mock_table.getvalue() + def test_table_output_models_no_headers(self, mock_cli): + mock_cli.output_handler.headers = False + + output = io.StringIO() + header = ["h1"] + data = [ + { + "cool": "foo", + }, + {"cool": "bar"}, + ] + columns = [ModelAttr("cool", True, True, "string")] + + mock_cli.output_handler._table_output( + header, data, columns, "cool table", output + ) + + mock_table = io.StringIO() + tab = Table(header_style="", show_header=False, box=box.SQUARE) + for row in [["foo"], ["bar"]]: + tab.add_row(*row) + tab.title = "cool table" + rprint(tab, file=mock_table) + + assert output.getvalue() == mock_table.getvalue() + def test_get_columns_from_model(self, mock_cli): output_handler = mock_cli.output_handler From 56d4c8051b0c2889262344a7a9d88138bba5381c Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Fri, 2 Jun 2023 15:00:57 -0400 Subject: [PATCH 07/13] new: Dynamically resolve CLI automated release version (#464) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description This change alters the cross-repo automated release workflow to dynamically determine the released CLI version depending on whether the provided spec version is a minor or patch release. This change makes use of Javascript rather than external Actions due to the increased complexity of version calculation. Test Runs: - Minor Bump: https://github.com/lgarber-akamai/linode-cli/actions/runs/5157696494/jobs/9290437845 - Patch Bump: https://github.com/lgarber-akamai/linode-cli/actions/runs/5157685698/jobs/9290412752 --- .github/workflows/remote-release-trigger.yml | 33 +++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/.github/workflows/remote-release-trigger.yml b/.github/workflows/remote-release-trigger.yml index e2765e809..2aa9a4a84 100644 --- a/.github/workflows/remote-release-trigger.yml +++ b/.github/workflows/remote-release-trigger.yml @@ -27,15 +27,38 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - name: Get the next minor version - id: semvers - uses: WyriHaximus/github-action-next-semvers@d079934efaf011a4cf8912d4637097fe35d32b93 # pin@v1 + - name: Calculate the desired release version + id: calculate_version + uses: actions/github-script@v6 + env: + SPEC_VERSION: ${{ github.event.client_payload.spec_version }} + PREVIOUS_CLI_VERSION: ${{ steps.previoustag.outputs.tag }} with: + result-encoding: string version: ${{ steps.previoustag.outputs.tag }} + script: | + let spec_version_segments = process.env.SPEC_VERSION.replace("v", "").split("."); + let cli_version_segments = process.env.PREVIOUS_CLI_VERSION.replace("v", "").split("."); + + // Default to a patch version bump + let bump_idx = 2; + + // This is a minor version bump + if (spec_version_segments[2] == "0") { + bump_idx = 1; + + // The patch number should revert to 0 + cli_version_segments[2] = "0" + } + + // Bump the version + cli_version_segments[bump_idx] = (parseInt(cli_version_segments[bump_idx]) + 1).toString() + + return "v" + cli_version_segments.join(".") - uses: rickstaa/action-create-tag@84c90e6ba79b47b5147dcb11ff25d6a0e06238ba # pin@v1 with: - tag: ${{ steps.semvers.outputs.v_minor }} + tag: ${{ steps.calculate_version.outputs.result }} - name: Release uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844 # pin@v1 @@ -43,4 +66,4 @@ jobs: target_commitish: 'main' token: ${{ steps.generate_token.outputs.token }} body: Built from Linode OpenAPI spec ${{ github.event.client_payload.spec_version }} - tag_name: ${{ steps.semvers.outputs.v_minor }} + tag_name: ${{ steps.calculate_version.outputs.result }} From 714c6c6fbe1f1ab46a19c13994eda8f05e12326a Mon Sep 17 00:00:00 2001 From: ykim-1 <126618609+ykim-1@users.noreply.github.com> Date: Wed, 7 Jun 2023 13:44:12 -0700 Subject: [PATCH 08/13] fix integration tests for windows and improve flaky tests (#465) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description Recent Windows fork workflow causes some tests to fail as they were originally meant for linux environment Fixed flaky tests ## ✔️ How to Test make testint /acctest **How do I run the relevant unit/integration tests?** ## 📷 Preview **If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.** --- .github/workflows/e2e-suite-pr.yml | 2 +- Makefile | 2 +- tests/integration/conftest.py | 64 +++------ .../domains/test_domain_records.py | 2 +- .../domains/test_master_domains.py | 73 ++++------ .../integration/domains/test_slave_domains.py | 6 +- tests/integration/events/test_events.py | 81 +++++++---- tests/integration/firewalls/test_firewalls.py | 9 +- .../firewalls/test_firewalls_rules.py | 4 +- tests/integration/helpers.py | 55 +++++--- .../image/test_plugin_image_upload.py | 13 +- tests/integration/linodes/helpers_linodes.py | 102 +++++++------- tests/integration/linodes/test_linodes.py | 6 +- .../integration/linodes/test_power_status.py | 4 +- tests/integration/linodes/test_rebuild.py | 16 ++- tests/integration/linodes/test_resize.py | 68 ++++++--- tests/integration/lke/test_clusters.py | 32 ++++- .../integration/networking/test_networking.py | 2 +- .../nodebalancers/test_node_balancers.py | 33 +++-- tests/integration/ssh/test_plugin_ssh.py | 9 +- tests/integration/ssh/test_ssh.py | 48 +++++-- .../stackscripts/test_stackscripts.py | 129 ++++++++++-------- tests/integration/tags/test_tags.py | 5 - tests/integration/users/test_users.py | 4 +- tests/integration/volumes/test_volumes.py | 2 +- .../volumes/test_volumes_resize.py | 2 +- tests/unit/conftest.py | 6 - tests/unit/test_plugin_ssh.py | 22 +-- 28 files changed, 455 insertions(+), 346 deletions(-) diff --git a/.github/workflows/e2e-suite-pr.yml b/.github/workflows/e2e-suite-pr.yml index be7f824e8..74b377197 100644 --- a/.github/workflows/e2e-suite-pr.yml +++ b/.github/workflows/e2e-suite-pr.yml @@ -116,7 +116,7 @@ jobs: - run: make INTEGRATION_TEST_PATH="${{ github.event.client_payload.slash_command.tests }}" TEST_CASE="${{ github.event.client_payload.slash_command.testcase }}" testint env: - LINODE_CLI_TOKEN: ${{ secrets.LINODE_TOKEN }} + LINODE_CLI_TOKEN: ${{ secrets.LINODE_TOKEN_2 }} - uses: actions/github-script@v5 id: update-check-run diff --git a/Makefile b/Makefile index 20ab44a67..ec73bf5d5 100644 --- a/Makefile +++ b/Makefile @@ -54,7 +54,7 @@ testunit: .PHONY: testint testint: - pytest tests/integration/${INTEGRATION_TEST_PATH} ${TEST_CASE_COMMAND} + pytest tests/integration/${INTEGRATION_TEST_PATH} ${TEST_CASE_COMMAND} --disable-warnings .PHONY: testall testall: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 9dfb22aac..c7ac36610 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -20,6 +20,7 @@ get_random_text, ) from tests.integration.linodes.helpers_linodes import ( + DEFAULT_LINODE_TYPE, DEFAULT_RANDOM_PASS, DEFAULT_REGION, DEFAULT_TEST_IMAGE, @@ -152,7 +153,7 @@ def create_master_domain(): @pytest.fixture def create_slave_domain(): - timestamp = str(int(time.time())) + timestamp = str(int(time.time()) + randint(10, 1000)) domain_id = ( exec_test_command( @@ -184,6 +185,8 @@ def create_slave_domain(): # Test helpers specific to Linodes test suite @pytest.fixture def create_linode_with_label(): + timestamp = str(int(time.time()) + randint(10, 1000)) + label = "cli" + timestamp result = ( exec_test_command( LINODE_BASE_CMD @@ -196,7 +199,7 @@ def create_linode_with_label(): "--image", DEFAULT_TEST_IMAGE, "--label", - "cli-1", + label, "--root_pass", DEFAULT_RANDOM_PASS, "--text", @@ -254,48 +257,25 @@ def create_linode_min_req(): @pytest.fixture def create_linode_wo_image(): - linode_type = ( - os.popen( - "linode-cli linodes types --text --no-headers --format=id | xargs | awk '{ print $1 }'" - ) - .read() - .rstrip() - ) - linode_region = ( - os.popen( - "linode-cli regions list --format=id --text --no-headers | xargs | awk '{ print $1 }'" - ) - .read() - .rstrip() - ) - - exec_test_command( - LINODE_BASE_CMD - + [ - "create", - "--no-defaults", - "--label", - "cli-2", - "--type", - linode_type, - "--region", - linode_region, - "--root_pass", - DEFAULT_RANDOM_PASS, - ] - ).stdout.decode() - + label = "cli" + str(int(time.time()) + randint(10, 1000)) linode_id = ( exec_test_command( LINODE_BASE_CMD + [ - "list", + "create", + "--no-defaults", "--label", - "cli-2", - "--text", - "--no-headers", + label, + "--type", + DEFAULT_LINODE_TYPE, + "--region", + DEFAULT_REGION, + "--root_pass", + DEFAULT_RANDOM_PASS, "--format", "id", + "--no-headers", + "--text", ] ) .stdout.decode() @@ -309,14 +289,6 @@ def create_linode_wo_image(): @pytest.fixture def create_linode_backup_enabled(): - linode_type = ( - os.popen( - "linode-cli linodes types --text --no-headers --format='id' | xargs | awk '{ print $1 }'" - ) - .read() - .rstrip() - ) - # create linode with backups enabled linode_id = ( exec_test_command( @@ -327,7 +299,7 @@ def create_linode_backup_enabled(): "--backups_enabled", "true", "--type", - linode_type, + DEFAULT_LINODE_TYPE, "--region", DEFAULT_REGION, "--image", diff --git a/tests/integration/domains/test_domain_records.py b/tests/integration/domains/test_domain_records.py index 010c30f03..415083619 100644 --- a/tests/integration/domains/test_domain_records.py +++ b/tests/integration/domains/test_domain_records.py @@ -12,7 +12,7 @@ BASE_CMD = ["linode-cli", "domains"] -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture def domain_records_setup(): timestamp = str(int(time.time())) # Create domain diff --git a/tests/integration/domains/test_master_domains.py b/tests/integration/domains/test_master_domains.py index 9998adadc..37e922296 100644 --- a/tests/integration/domains/test_master_domains.py +++ b/tests/integration/domains/test_master_domains.py @@ -1,19 +1,18 @@ -import os import re import time import pytest from tests.integration.helpers import ( - FAILED_STATUS_CODE, delete_target_id, + exec_failing_test_command, exec_test_command, ) BASE_CMD = ["linode-cli", "domains"] -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture def setup_master_domains(): timestamp = str(int(time.time())) # Create domain @@ -48,59 +47,39 @@ def test_create_domain_fails_without_spcified_type(): # get debug output from linode-cli to a temporary file.. # not all output from the linode-cli goes to stdout, stderr - os.system( - 'linode-cli domains create \ - --domain "BC' - + timestamp - + '-example.com" \ - --soa_email="pthiel@linode.com" \ - --text \ - --no-header 2>&1 | tee /tmp/test.txt' - ) - - status = os.system( - 'linode-cli domains create \ - --domain "BC' - + timestamp - + '-example.com" \ - --soa_email="pthiel@linode.com" \ - --text \ - --no-header' - ) - result = exec_test_command(["cat", "/tmp/test.txt"]).stdout.decode() + result = exec_failing_test_command( + BASE_CMD + + [ + "create", + "--domain", + "example.bc-" + timestamp + ".com", + "--soa_email", + "pthiel@linode.com", + "--text", + "--no-headers", + ] + ).stderr.decode() - assert status == FAILED_STATUS_CODE assert "Request failed: 400" in result assert "type is required" in result def test_create_master_domain_fails_without_soa_email(): timestamp = str(int(time.time())) + result = exec_failing_test_command( + BASE_CMD + + [ + "create", + "--type", + "master", + "--domain", + "example.bc-" + timestamp + ".com", + "--text", + "--no-headers", + ] + ).stderr.decode() - os.system( - 'linode-cli domains create \ - --type master \ - --domain "BC$' - + timestamp - + '-example.com" \ - --text \ - --no-header 2>&1 | tee /tmp/test.txt' - ) - - status = os.system( - 'linode-cli domains create \ - --type master \ - --domain "BC$' - + timestamp - + '-example.com" \ - --text \ - --no-header' - ) - - result = exec_test_command(["cat", "/tmp/test.txt"]).stdout.decode() - - assert status == FAILED_STATUS_CODE assert "Request failed: 400" in result assert "soa_email soa_email required when type=master" in result diff --git a/tests/integration/domains/test_slave_domains.py b/tests/integration/domains/test_slave_domains.py index 30fef3871..41b3234d7 100644 --- a/tests/integration/domains/test_slave_domains.py +++ b/tests/integration/domains/test_slave_domains.py @@ -15,7 +15,7 @@ timestamp = str(int(time.time())) -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture def slave_domain_setup(): # Create domain slave_domain_id = ( @@ -65,11 +65,11 @@ def test_create_slave_domain(create_slave_domain): assert re.search("[0-9]+", domain_id) -def test_list_slave_domain(): +def test_list_slave_domain(create_slave_domain): result = exec_test_command( BASE_CMD + ["list", "--text", "--no-header"] ).stdout.decode() - assert timestamp + "-example.com" in result + assert "-example.com" in result @pytest.mark.skip(reason="BUG 872") diff --git a/tests/integration/events/test_events.py b/tests/integration/events/test_events.py index 9c8547088..18cf4fa20 100644 --- a/tests/integration/events/test_events.py +++ b/tests/integration/events/test_events.py @@ -1,4 +1,3 @@ -import os import re import time @@ -9,7 +8,7 @@ BASE_CMD = ["linode-cli", "events"] -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture def events_setup(): timestamp = str(int(time.time())) # Create domain @@ -60,15 +59,22 @@ def test_list_events(): def test_view_events(): - event_id_arr = ( - os.popen( - 'linode-cli events list --format "id" --text --no-headers | xargs | awk "{ print $1 }"' + event_id = ( + exec_test_command( + [ + "linode-cli", + "events", + "list", + "--format", + "id", + "--no-headers", + "--text", + ] ) - .read() + .stdout.decode() .rstrip() - .split() + .split()[0] ) - event_id = event_id_arr[0] result = exec_test_command( BASE_CMD @@ -78,15 +84,22 @@ def test_view_events(): def test_mark_event_seen(): - event_id_arr = ( - os.popen( - 'linode-cli events list --format "id" --text --no-headers | xargs | awk "{ print $1 }"' + event_id = ( + exec_test_command( + [ + "linode-cli", + "events", + "list", + "--format", + "id", + "--no-headers", + "--text", + ] ) - .read() + .stdout.decode() .rstrip() - .split() + .split()[0] ) - event_id = event_id_arr[0] # mark event as seen exec_test_command( @@ -103,17 +116,24 @@ def test_mark_event_seen(): def test_mark_event_read(): - event_id_arr = ( - os.popen( - 'linode-cli events list --format "id" --text --no-headers | xargs | awk "{ print $1 }"' + event_id = ( + exec_test_command( + [ + "linode-cli", + "events", + "list", + "--format", + "id", + "--no-headers", + "--text", + ] ) - .read() + .stdout.decode() .rstrip() - .split() + .split()[0] ) - event_id = event_id_arr[0] - # mark event as seen + # mark event as read exec_test_command( BASE_CMD + ["mark-read", event_id, "--text", "--no-headers", "--delimiter", ","] @@ -128,15 +148,22 @@ def test_mark_event_read(): def test_filter_events_by_entity_id(): - event_id_arr = ( - os.popen( - 'linode-cli events list --format "id" --text --no-headers | xargs | awk "{ print $1 }"' + event_id = ( + exec_test_command( + [ + "linode-cli", + "events", + "list", + "--format", + "id", + "--no-headers", + "--text", + ] ) - .read() + .stdout.decode() .rstrip() - .split() + .split()[0] ) - event_id = event_id_arr[0] result = exec_test_command( BASE_CMD diff --git a/tests/integration/firewalls/test_firewalls.py b/tests/integration/firewalls/test_firewalls.py index 90a921325..ce846b7f9 100644 --- a/tests/integration/firewalls/test_firewalls.py +++ b/tests/integration/firewalls/test_firewalls.py @@ -10,10 +10,10 @@ ) BASE_CMD = ["linode-cli", "firewalls"] -FIREWALL_LABEL = "example-firewall-label" +FIREWALL_LABEL = "label-fw-test" + str(int(time.time())) -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture def firewalls_setup(): # Create one domain for some tests in this suite firewall_id = ( @@ -220,9 +220,10 @@ def test_create_firewall_with_inbound_and_outbound_args(): def test_update_firewall(firewalls_setup): + timestamp = str(int(time.time())) firewall_id = firewalls_setup - updated_tag = "updated-tag" - updated_label = "updated-label" + updated_tag = "updated-tag" + timestamp + updated_label = "updated-" + timestamp result = ( exec_test_command( diff --git a/tests/integration/firewalls/test_firewalls_rules.py b/tests/integration/firewalls/test_firewalls_rules.py index f0d32d1f5..532b60c3f 100644 --- a/tests/integration/firewalls/test_firewalls_rules.py +++ b/tests/integration/firewalls/test_firewalls_rules.py @@ -6,10 +6,10 @@ from tests.integration.helpers import delete_target_id, exec_test_command BASE_CMD = ["linode-cli", "firewalls", "rules-update"] -FIREWALL_LABEL = "example-firewall-label" +FIREWALL_LABEL = "label-fw-test" + str(int(time.time())) -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture def create_firewall(): firewall_id = ( exec_test_command( diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 818b2b385..3ea350f05 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -1,4 +1,3 @@ -import os import random import subprocess import time @@ -80,11 +79,20 @@ def delete_target_id(target: str, id: str): def remove_lke_clusters(): cluster_ids = ( - os.popen( - "linode-cli --text --no-headers lke clusters-list --format id | grep -v 'linuke-keep' | awk '{ print $1 }' | xargs" + exec_test_command( + [ + "linode-cli", + "--text", + "--no-headers", + "lke", + "clusters-list", + "--format", + "id", + ] ) - .read() - .split() + .stdout.decode() + .rstrip() + .splitlines() ) for id in cluster_ids: exec_test_command(["linode-cli", "lke", "cluster-delete", id]) @@ -99,23 +107,36 @@ def remove_all(target: str): entity_ids = "" if target == "stackscripts": entity_ids = ( - os.popen( - "linode-cli --is_public=false --text --no-headers " - + target - + " list --format='id,tags' | grep -v 'linuke-keep' | awk '{ print $1 }' | xargs" + exec_test_command( + [ + "linode-cli", + "--is_public=false", + "--text", + "--no-headers", + target, + "list", + "--format", + "id", + ] ) - .read() - .split() + .stdout.decode() + .splitlines() ) else: entity_ids = ( - os.popen( - "linode-cli --text --no-headers " - + target - + " list --format='id,tags' | grep -v 'linuke-keep' | awk '{ print $1 }' | xargs" + exec_test_command( + [ + "linode-cli", + "--text", + "--no-headers", + target, + "list", + "--format", + "id", + ] ) - .read() - .split() + .stdout.decode() + .splitlines() ) for id in entity_ids: diff --git a/tests/integration/image/test_plugin_image_upload.py b/tests/integration/image/test_plugin_image_upload.py index 5df17546e..21e3a48fd 100644 --- a/tests/integration/image/test_plugin_image_upload.py +++ b/tests/integration/image/test_plugin_image_upload.py @@ -1,7 +1,9 @@ import json import os +import re import subprocess import tempfile +from sys import platform from typing import List import pytest @@ -59,6 +61,7 @@ def test_invalid_file( assert f"No file at {file_path}" in output +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") def test_file_upload( fake_image_file, ): @@ -74,9 +77,11 @@ def test_file_upload( output = process.stdout.decode() assert process.returncode == 0 - assert label in output - assert description in output - assert "pending_upload" in output + + # Assertions now using keywords due to some chars getting cut off from lack of terminal space + assert re.search("cli-test", output) + assert re.search("test", output) + assert re.search("pending", output) # Get the new image from the API process = exec_test_command( @@ -86,7 +91,7 @@ def test_file_upload( image = json.loads(process.stdout.decode()) - assert image[0]["label"] in output + assert image[0]["label"] == label # Delete the image process = exec_test_command(["linode-cli", "images", "rm", image[0]["id"]]) diff --git a/tests/integration/linodes/helpers_linodes.py b/tests/integration/linodes/helpers_linodes.py index d85917caf..6f2578033 100644 --- a/tests/integration/linodes/helpers_linodes.py +++ b/tests/integration/linodes/helpers_linodes.py @@ -1,4 +1,3 @@ -import os import time from tests.integration.helpers import exec_test_command @@ -9,38 +8,46 @@ .rstrip() ) DEFAULT_REGION = "us-east" + DEFAULT_TEST_IMAGE = ( - os.popen( - 'linode-cli images list --format id --text --no-header | egrep "linode\/.*" | head -n 1' + exec_test_command( + [ + "linode-cli", + "images", + "list", + "--text", + "--format", + "id", + "--no-headers", + "--is_public", + "True", + ] ) - .read() + .stdout.decode() .rstrip() + .splitlines()[0] ) + DEFAULT_LINODE_TYPE = ( - os.popen( - "linode-cli linodes types --text --no-headers --format='id' | xargs | awk '{ print $1 }'" + exec_test_command( + [ + "linode-cli", + "linodes", + "types", + "--text", + "--no-headers", + "--format", + "id", + ] ) - .read() + .stdout.decode() .rstrip() + .splitlines()[0] ) -DEFAULT_LABEL = "cli-default" -BASE_CMD = ["linode-cli", "linodes"] +DEFAULT_LABEL = "cli-default" -def generate_random_ssh_key(): - key_path = "/tmp/cli-e2e-key" - os.popen("ssh-keygen -q -t rsa -N ' ' -f " + key_path) - time.sleep(1) - private_ssh_key = ( - exec_test_command(["cat", key_path]).stdout.decode().rstrip() - ) - public_ssh_key = ( - exec_test_command(["cat", key_path + ".pub"]).stdout.decode().rstrip() - ) - - private_keypath = key_path - public_keypath = key_path + ".pub" - return private_ssh_key, public_ssh_key, private_keypath, public_keypath +BASE_CMD = ["linode-cli", "linodes"] def wait_until(linode_id: "str", timeout, status: "str", period=5): @@ -66,20 +73,6 @@ def wait_until(linode_id: "str", timeout, status: "str", period=5): def create_linode(): region = "us-east" - linode_type = ( - os.popen( - "linode-cli linodes types --text --no-headers --format='id' | xargs | awk '{ print $1 }'" - ) - .read() - .rstrip() - ) - test_image = ( - os.popen( - 'linode-cli images list --format id --text --no-header | egrep "linode\/.*" | head -n 1' - ) - .read() - .rstrip() - ) # create linode linode_id = ( @@ -89,11 +82,11 @@ def create_linode(): "linodes", "create", "--type", - linode_type, + DEFAULT_LINODE_TYPE, "--region", region, "--image", - test_image, + DEFAULT_TEST_IMAGE, "--root_pass", DEFAULT_RANDOM_PASS, "--format=id", @@ -110,23 +103,38 @@ def create_linode(): def shutdown_linodes(): linode_ids = ( - os.popen( - 'linode-cli --text --no-headers linodes list --format "id" | grep -v "linuke-keep" | awk "{ print $1 }" | xargs' + exec_test_command( + [ + BASE_CMD, + "linodes", + "list", + "--format", + "id", + ] ) - .read() - .split() + .stdout.decode() + .rstrip() + .splitlines() ) + for id in linode_ids: exec_test_command(["linode-cli", "linodes", "shutdown", id]) def remove_linodes(): linode_ids = ( - os.popen( - 'linode-cli --text --no-headers linodes list --format "id" | grep -v "linuke-keep" | awk "{ print $1 }" | xargs' + exec_test_command( + [ + BASE_CMD, + "linodes", + "list", + "--format", + "id", + ] ) - .read() - .split() + .stdout.decode() + .rstrip() + .splitlines() ) for id in linode_ids: diff --git a/tests/integration/linodes/test_linodes.py b/tests/integration/linodes/test_linodes.py index 6d4c814e6..2fb997bef 100644 --- a/tests/integration/linodes/test_linodes.py +++ b/tests/integration/linodes/test_linodes.py @@ -16,11 +16,11 @@ wait_until, ) -timestamp = str(time.time()) +timestamp = str(int(time.time())) linode_label = DEFAULT_LABEL + timestamp -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="package", autouse=True) def setup_linodes(): linode_id = ( exec_test_command( @@ -67,7 +67,7 @@ def test_create_linodes_with_a_label(create_linode_with_label): result = create_linode_with_label assert re.search( - "cli-1,us-east,g6-standard-2," + DEFAULT_TEST_IMAGE, result + "^cli(.*),us-east,g6-standard-2," + DEFAULT_TEST_IMAGE, result ) diff --git a/tests/integration/linodes/test_power_status.py b/tests/integration/linodes/test_power_status.py index c9aac35c0..bb6dc6482 100644 --- a/tests/integration/linodes/test_power_status.py +++ b/tests/integration/linodes/test_power_status.py @@ -9,7 +9,7 @@ ) -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture def setup_power_status(): linode_id = create_linode() @@ -18,7 +18,7 @@ def setup_power_status(): delete_target_id(target="linodes", id=linode_id) -@pytest.fixture(scope="session") +@pytest.fixture def create_linode_in_running_state(): linode_id = create_linode_and_wait() diff --git a/tests/integration/linodes/test_rebuild.py b/tests/integration/linodes/test_rebuild.py index 9d617d56a..1aae7e0c3 100644 --- a/tests/integration/linodes/test_rebuild.py +++ b/tests/integration/linodes/test_rebuild.py @@ -15,7 +15,7 @@ ) -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture def setup_rebuild(): linode_id = create_linode_and_wait() @@ -71,11 +71,19 @@ def test_rebuild_fails_with_invalid_image(setup_rebuild): def test_rebuild_a_linode(setup_rebuild): linode_id = setup_rebuild rebuild_image = ( - os.popen( - "linode-cli images list --text --no-headers --format=id | sed -n 3p" + exec_test_command( + [ + "linode-cli", + "images", + "list", + "--text", + "--no-headers" "--format", + "id", + ] ) - .read() + .stdout.decode() .rstrip() + .splitlines()[4] ) # trigger rebuild diff --git a/tests/integration/linodes/test_resize.py b/tests/integration/linodes/test_resize.py index 62363628c..4629fd517 100644 --- a/tests/integration/linodes/test_resize.py +++ b/tests/integration/linodes/test_resize.py @@ -14,15 +14,23 @@ ) -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="session") def setup_resize(): - # create linode plan = ( - os.popen( - 'linode-cli linodes types --format="id" --text --no-headers | sed -n 2p' + exec_test_command( + [ + "linode-cli", + "linodes", + "types", + "--format", + "id", + "--text", + "--no-headers", + ] ) - .read() + .stdout.decode() .rstrip() + .splitlines()[1] ) linode_id = create_linode_and_wait(test_plan=plan) @@ -34,23 +42,27 @@ def setup_resize(): def test_resize_fails_to_the_same_plan(setup_resize): linode_id = setup_resize linode_plan = ( - os.popen( - "linode-cli linodes view " - + linode_id - + ' --format="type" --text --no-headers' + exec_test_command( + [ + "linode-cli", + "linodes", + "view", + linode_id, + "--format", + "type", + "--text", + "--no-headers", + ] ) - .read() + .stdout.decode() .rstrip() ) - print("linode id is", linode_id, " plan is", linode_plan) - result = exec_failing_test_command( BASE_CMD + ["resize", "--type", linode_plan, "--text", "--no-headers", linode_id] ).stderr.decode() - print("result is:", result) assert "Request failed: 400" in result assert "Linode is already running this service plan." in result @@ -58,11 +70,20 @@ def test_resize_fails_to_the_same_plan(setup_resize): def test_resize_fails_to_smaller_plan(setup_resize): linode_id = setup_resize smaller_plan = ( - os.popen( - 'linode-cli linodes types --format="id" --text --no-headers | sed -n 1p' + exec_test_command( + [ + "linode-cli", + "linodes", + "types", + "--format", + "id", + "--text", + "--no-headers", + ] ) - .read() + .stdout.decode() .rstrip() + .splitlines()[0] ) result = exec_failing_test_command( @@ -111,11 +132,20 @@ def test_resize_fail_to_invalid_plan(setup_resize): def test_resize_to_next_size_plan(setup_resize): linode_id = setup_resize larger_plan = ( - os.popen( - 'linode-cli linodes types --format="id" --text --no-headers | sed -n 3p' + exec_test_command( + [ + "linode-cli", + "linodes", + "types", + "--format", + "id", + "--text", + "--no-headers", + ] ) - .read() + .stdout.decode() .rstrip() + .splitlines()[2] ) exec_test_command( diff --git a/tests/integration/lke/test_clusters.py b/tests/integration/lke/test_clusters.py index d616a1515..92f205398 100644 --- a/tests/integration/lke/test_clusters.py +++ b/tests/integration/lke/test_clusters.py @@ -1,22 +1,35 @@ +import time +from random import randint + import pytest -from tests.integration.helpers import exec_test_command, os, remove_lke_clusters +from tests.integration.helpers import exec_test_command, remove_lke_clusters BASE_CMD = ["linode-cli", "lke"] -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(autouse=True) def setup_test_clusters(): yield "setup" - # just clean up method required for this test suite remove_lke_clusters() def test_deploy_an_lke_cluster(): + timestamp = str(int(time.time()) + randint(10, 1000)) + label = "cluster_test" + timestamp + lke_version = ( - os.popen("linode-cli lke versions-list --text --no-headers | head -1") - .read() + exec_test_command( + BASE_CMD + + [ + "versions-list", + "--text", + "--no-headers", + ] + ) + .stdout.decode() .rstrip() + .splitlines()[0] ) result = exec_test_command( @@ -26,7 +39,7 @@ def test_deploy_an_lke_cluster(): "--region", "us-east", "--label", - "cli-test-1", + label, "--node_pools.type", "g6-standard-1", "--node_pools.count", @@ -45,4 +58,9 @@ def test_deploy_an_lke_cluster(): ] ).stdout.decode() - assert "cli-test-1,us-east," + lke_version in result + assert label + ",us-east," + lke_version in result + + # Sleep needed here for proper deletion of linodes that are related to lke cluster + time.sleep(15) + + remove_lke_clusters() diff --git a/tests/integration/networking/test_networking.py b/tests/integration/networking/test_networking.py index 97c4bde5a..685de31c8 100644 --- a/tests/integration/networking/test_networking.py +++ b/tests/integration/networking/test_networking.py @@ -8,7 +8,7 @@ BASE_CMD = ["linode-cli", "networking"] -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="package") def setup_test_networking(): linode_id = create_linode_and_wait() diff --git a/tests/integration/nodebalancers/test_node_balancers.py b/tests/integration/nodebalancers/test_node_balancers.py index 0dd7b4b88..661f2c894 100644 --- a/tests/integration/nodebalancers/test_node_balancers.py +++ b/tests/integration/nodebalancers/test_node_balancers.py @@ -13,7 +13,7 @@ nodebalancer_created = "[0-9]+,balancer[0-9]+,us-east,[0-9]+-[0-9]+-[0-9]+-[0-9]+.ip.linodeusercontent.com,0" -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="package") def setup_test_node_balancers(): # create a default nodebalancer nodebalancer_id = ( @@ -175,7 +175,7 @@ def test_create_nodebalancer_with_default_conf( assert re.search(nodebalancer_created, result) -def test_list_nodebalancers_and_status(): +def test_list_nodebalancers_and_status(setup_test_node_balancers): result = exec_test_command( BASE_CMD + [ @@ -350,7 +350,7 @@ def test_update_node_port(setup_test_node_balancers): ] ).stdout.decode() - assert ("[0-9]+,.," + new_address + ",Unknown,100,accept", result) + assert "[0-9]+,.," + new_address + ",Unknown,100,accept", result def test_fail_to_update_node_to_public_ipv4_address(setup_test_node_balancers): @@ -390,8 +390,6 @@ def test_remove_node_from_configuration_profile(setup_test_node_balancers): ) -# Test below this needs to be ran last and in order -@pytest.fixture(scope="session") def test_update_the_port_of_a_configuration_profile(setup_test_node_balancers): nodebalancer_id = setup_test_node_balancers[0] config_id = setup_test_node_balancers[1] @@ -418,7 +416,6 @@ def test_update_the_port_of_a_configuration_profile(setup_test_node_balancers): ) -@pytest.fixture(scope="session") def test_add_additional_configuration_profile(setup_test_node_balancers): nodebalancer_id = setup_test_node_balancers[0] @@ -437,17 +434,27 @@ def test_add_additional_configuration_profile(setup_test_node_balancers): ).stdout.decode() assert re.search( - "[0-9]+,81,http,roundrobin,none,True,recommended,,", result + "^[0-9](.*),81,http,roundrobin,none,True,recommended,,", result ) -@pytest.mark.usefixtures( - "test_add_additional_configuration_profile", - "test_update_the_port_of_a_configuration_profile", -) def test_list_multiple_configuration_profile(setup_test_node_balancers): nodebalancer_id = setup_test_node_balancers[0] + result = exec_test_command( + BASE_CMD + + [ + "config-create", + nodebalancer_id, + "--text", + "--no-headers", + "--delimiter", + ",", + "--port", + "83", + ] + ) + result = exec_test_command( BASE_CMD + [ @@ -461,8 +468,8 @@ def test_list_multiple_configuration_profile(setup_test_node_balancers): ).stdout.decode() assert re.search( - "[0-9]+,81,http,roundrobin,none,True,recommended,,", result + "[0-9]+,8[0-1],http,roundrobin,none,True,recommended,,", result ) assert re.search( - "[0-9]+,10700,tcp,roundrobin,none,True,recommended,,", result + "[0-9]+,83,http,roundrobin,none,True,recommended,,", result ) diff --git a/tests/integration/ssh/test_plugin_ssh.py b/tests/integration/ssh/test_plugin_ssh.py index ec1cbe4c5..664765e81 100644 --- a/tests/integration/ssh/test_plugin_ssh.py +++ b/tests/integration/ssh/test_plugin_ssh.py @@ -1,5 +1,6 @@ import json import subprocess +from sys import platform from typing import Any, Dict, List, Optional import pytest @@ -23,10 +24,9 @@ POLL_INTERVAL = 5 +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") @pytest.fixture -def target_instance(ssh_key_pair_generator, platform_os_type): - if platform_os_type == "Windows": - pytest.skip("This pluggin is not supported on Windows") +def target_instance(ssh_key_pair_generator): instance_label = f"cli-test-{get_random_text(length=6)}" pubkey_file, privkey_file = ssh_key_pair_generator @@ -70,6 +70,7 @@ def exec_test_command(args: List[str], timeout=None): return process +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") def test_help(): process = exec_test_command(BASE_CMD + ["--help"]) output = process.stdout.decode() @@ -79,6 +80,7 @@ def test_help(): assert "uses the Linode's SLAAC address for SSH" in output +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") def test_ssh_instance_provisioning(target_instance: Dict[str, Any]): process = exec_test_command(BASE_CMD + ["root@" + target_instance["label"]]) assert process.returncode == 2 @@ -87,6 +89,7 @@ def test_ssh_instance_provisioning(target_instance: Dict[str, Any]): assert "is not running" in output +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") def test_ssh_instance_ready( ssh_key_pair_generator, target_instance: Dict[str, Any] ): diff --git a/tests/integration/ssh/test_ssh.py b/tests/integration/ssh/test_ssh.py index 031f2a9a0..68c2fc9e0 100644 --- a/tests/integration/ssh/test_ssh.py +++ b/tests/integration/ssh/test_ssh.py @@ -1,6 +1,7 @@ import os import re import time +from sys import platform import pytest @@ -12,30 +13,47 @@ SSH_SLEEP_PERIOD = 50 -@pytest.fixture(scope="session", autouse=True) -def test_create_a_linode_in_running_state( - ssh_key_pair_generator, platform_os_type -): - if platform_os_type == "Windows": - pytest.skip("This test does not run on Windows") +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") +@pytest.fixture(scope="package") +def test_create_a_linode_in_running_state(ssh_key_pair_generator): pubkey_file, privkey_file = ssh_key_pair_generator with open(pubkey_file, "r") as f: pubkey = f.read().rstrip() - alpine_image = ( - os.popen( - "linode-cli images list --format id --text --no-headers | grep 'alpine' | xargs | awk '{ print $1 }'" + res = ( + exec_test_command( + [ + "linode-cli", + "images", + "list", + "--format", + "id", + "--text", + "--no-headers", + ] ) - .read() + .stdout.decode() .rstrip() ) + + alpine_image = re.findall("linode/alpine[^\s]+", res)[0] + plan = ( - os.popen( - "linode-cli linodes types --text --no-headers --format=id | xargs | awk '{ print $1 }'" + exec_test_command( + [ + "linode-cli", + "linodes", + "types", + "--format", + "id", + "--text", + "--no-headers", + ] ) - .read() + .stdout.decode() .rstrip() + .splitlines()[0] ) linode_id = create_linode_and_wait( @@ -46,12 +64,14 @@ def test_create_a_linode_in_running_state( delete_target_id(target="linodes", id=linode_id) +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") def test_display_ssh_plugin_usage_info(): result = exec_test_command(BASE_CMD + ["-h"]).stdout.decode() assert "[USERNAME@]LABEL" in result assert "uses the Linode's SLAAC address for SSH" in result +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") def test_fail_to_ssh_to_nonexistent_linode(): os.system("linode-cli ssh root@aasdkjlf 2>&1 | tee /tmp/output_file.txt") @@ -60,6 +80,7 @@ def test_fail_to_ssh_to_nonexistent_linode(): assert "No Linode found for label aasdkjlf" in result +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") def test_ssh_to_linode_and_get_kernel_version( test_create_a_linode_in_running_state, ssh_key_pair_generator ): @@ -97,6 +118,7 @@ def test_ssh_to_linode_and_get_kernel_version( assert re.search("[0-9]\.[0-9]*\.[0-9]*-.*-virt", output) +@pytest.mark.skipif(platform == "win32", reason="Test N/A on Windows") def test_check_vm_for_ipv4_connectivity( test_create_a_linode_in_running_state, ssh_key_pair_generator ): diff --git a/tests/integration/stackscripts/test_stackscripts.py b/tests/integration/stackscripts/test_stackscripts.py index 6338da6d0..3a13234bf 100644 --- a/tests/integration/stackscripts/test_stackscripts.py +++ b/tests/integration/stackscripts/test_stackscripts.py @@ -1,48 +1,45 @@ -import os import re +import time import pytest from tests.integration.helpers import ( + delete_target_id, exec_failing_test_command, exec_test_command, ) from tests.integration.linodes.helpers_linodes import DEFAULT_RANDOM_PASS BASE_CMD = ["linode-cli", "stackscripts"] +DEF_LABEL = "stack_script_" + str(int(time.time())) def get_linode_image_lists(): - images = os.popen( - 'LINODE_CLI_TOKEN=$LINODE_CLI_TOKEN linode-cli images list --format id --text --no-headers | egrep "linode\/.*"' - ).read() - - return images.splitlines() - - -def get_private_stackscript(): - private_stackscript = ( - exec_test_command( - BASE_CMD - + [ - "list", - "--is_public", - "false", - "--text", - "--no-headers", - "--format", - "id", - ] + all_images = ( + ( + exec_test_command( + [ + "linode-cli", + "images", + "list", + "--format", + "id", + "--text", + "--no-headers", + ] + ) ) .stdout.decode() .rstrip() ) - return private_stackscript.splitlines() + images = re.findall("linode/[^\s]+", all_images) + + return images -@pytest.fixture(scope="session", autouse=True) -def test_create_stackscript(): +@pytest.fixture(scope="package", autouse=True) +def create_stackscript(): result = exec_test_command( BASE_CMD + [ @@ -52,7 +49,7 @@ def test_create_stackscript(): "--image", "linode/debian9", "--label", - "testfoo", + DEF_LABEL, "--is_public=false", "--text", "--no-headers", @@ -62,10 +59,18 @@ def test_create_stackscript(): ).stdout.decode() assert re.search( - "[0-9]+,.*,testfoo,linode/debian9,False,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+", + "[0-9]+,.*," + + DEF_LABEL + + ",linode/debian9,False,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+", result, ) + stackscript_id = result.split(",")[0] + + yield stackscript_id + + delete_target_id(target="stackscripts", id=stackscript_id) + def test_list_stackscripts(): result = exec_test_command(BASE_CMD + ["list", "--text"]).stdout.decode() @@ -89,7 +94,7 @@ def test_list_stackscripts(): assert re.search("[0-9]+,([A-z]|[0-9])+,True", output[0]) -def test_create_stackscrip_fails_without_image(): +def test_create_stackscript_fails_without_image(): result = exec_failing_test_command( BASE_CMD + [ @@ -125,40 +130,47 @@ def test_view_private_stackscript(): ).stdout.decode() assert re.search( - "[0-9]+,.*,testfoo,linode/debian9,False,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+", + "[0-9]+,.*," + + DEF_LABEL + + ",linode/debian9,False,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+", result, ) -def test_update_stackscript_compatible_image(): +def test_update_stackscript_compatible_image(create_stackscript): images = get_linode_image_lists() - private_stackscript = get_private_stackscript() - result = exec_test_command( - BASE_CMD - + [ - "update", - "--images", - images[0], - private_stackscript[0], - "--text", - "--no-headers", - "--delimiter", - ",", - ] - ).stdout.decode() + private_stackscript = create_stackscript + result = ( + exec_test_command( + BASE_CMD + + [ + "update", + "--images", + images[0], + private_stackscript, + "--text", + "--no-headers", + "--delimiter", + ",", + ] + ) + .stdout.decode() + .rstrip() + ) assert re.search( - "[0-9]+,.*,testfoo," + "[0-9]+,.*,stack_script_[0-9]+," + images[0] + ",False,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+,[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+", result, ) -@pytest.fixture(scope="session") -def test_update_stackscript_to_be_compatible_with_multiple_images(): +def test_update_stackscript_to_be_compatible_with_multiple_images( + create_stackscript, +): images = get_linode_image_lists() - private_stackscript = get_private_stackscript() + private_stackscript = create_stackscript result = exec_test_command( BASE_CMD @@ -168,7 +180,7 @@ def test_update_stackscript_to_be_compatible_with_multiple_images(): images[0], "--images", images[1], - private_stackscript[0], + private_stackscript, "--text", "--no-headers", "--delimiter", @@ -179,8 +191,10 @@ def test_update_stackscript_to_be_compatible_with_multiple_images(): assert images[1] in result -def test_fail_to_deploy_stackscript_to_linode_from_incompatible_image(): - private_stackscript = get_private_stackscript() +def test_fail_to_deploy_stackscript_to_linode_from_incompatible_image( + create_stackscript, +): + private_stackscript = create_stackscript linode_plan = "g6-standard-1" linode_region = "us-east" @@ -190,7 +204,7 @@ def test_fail_to_deploy_stackscript_to_linode_from_incompatible_image(): "linodes", "create", "--stackscript_id", - private_stackscript[0], + private_stackscript, "--type", linode_plan, "--image", @@ -208,11 +222,8 @@ def test_fail_to_deploy_stackscript_to_linode_from_incompatible_image(): assert "Request failed: 400" in result -@pytest.mark.usefixtures( - "test_update_stackscript_to_be_compatible_with_multiple_images" -) -def test_deploy_linode_from_stackscript(): - private_stackscript = get_private_stackscript() +def test_deploy_linode_from_stackscript(create_stackscript): + private_stackscript = create_stackscript images = get_linode_image_lists() linode_plan = "g6-standard-1" linode_region = "us-east" @@ -223,7 +234,7 @@ def test_deploy_linode_from_stackscript(): "linodes", "create", "--stackscript_id", - private_stackscript[0], + private_stackscript, "--type", linode_plan, "--image", @@ -244,3 +255,7 @@ def test_deploy_linode_from_stackscript(): assert re.search( "[0-9]+," + linode_region + "," + linode_plan + "," + images[0], result ) + + linode_id = result.split(",")[0] + + delete_target_id("linodes", linode_id) diff --git a/tests/integration/tags/test_tags.py b/tests/integration/tags/test_tags.py index e86911a13..8829bee21 100644 --- a/tests/integration/tags/test_tags.py +++ b/tests/integration/tags/test_tags.py @@ -11,11 +11,6 @@ unique_tag = str(int(time.time())) + "-tag" -@pytest.fixture(scope="session", autouse=True) -def setup_test_tags(): - yield "setup" - - def test_display_tags(): exec_test_command(BASE_CMD + ["list"]) diff --git a/tests/integration/users/test_users.py b/tests/integration/users/test_users.py index e298b1255..1f33f325c 100644 --- a/tests/integration/users/test_users.py +++ b/tests/integration/users/test_users.py @@ -8,7 +8,7 @@ unique_user = "test-user-" + str(int(time.time())) -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="package", autouse=True) def setup_test_users(): yield "setup" remove_users() @@ -18,7 +18,7 @@ def remove_users(): exec_test_command(BASE_CMD + ["delete", unique_user]) -@pytest.fixture(scope="session") +@pytest.fixture def test_create_user(): exec_test_command( BASE_CMD diff --git a/tests/integration/volumes/test_volumes.py b/tests/integration/volumes/test_volumes.py index 481e46b0c..22dcfdc4d 100644 --- a/tests/integration/volumes/test_volumes.py +++ b/tests/integration/volumes/test_volumes.py @@ -15,7 +15,7 @@ unique_tag = str(int(time.time())) + "-tag" -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="package") def setup_test_volumes(): volume_id = ( exec_test_command( diff --git a/tests/integration/volumes/test_volumes_resize.py b/tests/integration/volumes/test_volumes_resize.py index 8cd1cbe4b..bfa97e47c 100644 --- a/tests/integration/volumes/test_volumes_resize.py +++ b/tests/integration/volumes/test_volumes_resize.py @@ -14,7 +14,7 @@ VOLUME_CREATION_WAIT = 5 -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="package") def setup_test_volumes_resize(): volume_id = ( exec_test_command( diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index c33ad819b..69fcace74 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,5 +1,4 @@ import configparser -import platform import pytest @@ -172,8 +171,3 @@ def write_config(self): # pylint: disable=missing-function-docstring pass return Config() - - -@pytest.fixture -def platform_os_type(): - return platform.system() diff --git a/tests/unit/test_plugin_ssh.py b/tests/unit/test_plugin_ssh.py index fd915544c..11b97660a 100644 --- a/tests/unit/test_plugin_ssh.py +++ b/tests/unit/test_plugin_ssh.py @@ -1,4 +1,5 @@ import argparse +import sys from unittest.mock import patch import pytest @@ -8,9 +9,10 @@ from linodecli.plugins import PluginContext -def test_print_help(capsys: CaptureFixture, platform_os_type): - if platform_os_type == "Windows": - pytest.skip("This test does not run on Windows") +@pytest.mark.skipif( + sys.platform.startswith("win"), reason="Test N/A on Windows" +) +def test_print_help(capsys: CaptureFixture): with pytest.raises(SystemExit) as err: plugin.call(["--help"], None) @@ -32,9 +34,10 @@ def test_windows_error(capsys: CaptureFixture): assert "This plugin is not currently supported in Windows." in captured_text -def test_target_not_running(mock_cli, capsys: CaptureFixture, platform_os_type): - if platform_os_type == "Windows": - pytest.skip("This test does not run on Windows") +@pytest.mark.skipif( + sys.platform.startswith("win"), reason="Test N/A on Windows" +) +def test_target_not_running(mock_cli, capsys: CaptureFixture): test_label = "totally-real-label" def mock_call_operation(*a, filters=None): @@ -57,9 +60,10 @@ def mock_call_operation(*a, filters=None): ) -def test_target_success(mock_cli, capsys: CaptureFixture, platform_os_type): - if platform_os_type == "Windows": - pytest.skip("This test does not run on Windows") +@pytest.mark.skipif( + sys.platform.startswith("win"), reason="Test N/A on Windows" +) +def test_target_success(mock_cli, capsys: CaptureFixture): test_label = "totally-real-label" test_user = "test" test_ip = "123.123.123.123" From 141223845561c58bde2835bb67f106ff37c7cfe1 Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Tue, 13 Jun 2023 14:49:12 -0400 Subject: [PATCH 09/13] new: Add `LINODE_CLI_CA` environment override (#467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description This change adds a new `LINODE_CLI_CA` environment variable that allows users to specify a custom CA file to be used in API requests. Additionally, this change updates the `linode-cli` help page to list out accepted environment variables. ## ✔️ How to Test ``` make testunit ``` --- README.rst | 3 +++ linodecli/api_request.py | 4 +++- linodecli/arg_helpers.py | 27 +++++++++++++++++++++++++++ linodecli/configuration/auth.py | 11 +++++++++-- linodecli/helpers.py | 5 +++++ tests/unit/test_api_request.py | 4 ++-- 6 files changed, 49 insertions(+), 5 deletions(-) diff --git a/README.rst b/README.rst index 5efb94a38..5ec7818fa 100644 --- a/README.rst +++ b/README.rst @@ -251,6 +251,9 @@ the ``obj`` plugin that ships with the CLI. To do so, simply set appropriate values. This allows using Linode Object Storage through the CLI without having a configuration file, which is desirable in some situations. +You may also specify the path to a custom Certificate Authority file using the ``LINODE_CLI_CA`` +environment variable. + Configurable API URL """""""""""""""""""" diff --git a/linodecli/api_request.py b/linodecli/api_request.py index 8a5ba2da8..32dbc9fc8 100644 --- a/linodecli/api_request.py +++ b/linodecli/api_request.py @@ -11,6 +11,8 @@ from packaging import version from requests import Response +from linodecli.helpers import API_CA_PATH + def do_request( ctx, @@ -51,7 +53,7 @@ def do_request( if ctx.debug_request: _print_request_debug_info(method, url, headers, body) - result = method(url, headers=headers, data=body) + result = method(url, headers=headers, data=body, verify=API_CA_PATH) # Print response debug info is requested if ctx.debug_request: diff --git a/linodecli/arg_helpers.py b/linodecli/arg_helpers.py index 207dcca08..1bdd3793e 100644 --- a/linodecli/arg_helpers.py +++ b/linodecli/arg_helpers.py @@ -9,6 +9,7 @@ import requests import yaml +from rich import box from rich import print as rprint from rich.table import Table @@ -231,10 +232,36 @@ def remove_plugin(plugin_name, config): return f"Plugin {plugin_name} removed", 0 +# pylint: disable=too-many-locals def help_with_ops(ops, config): """ Prints help output with options from the API spec """ + + # Environment variables overrides + print("\nEnvironment variables:") + env_variables = { + "LINODE_CLI_TOKEN": "A Linode Personal Access Token for the CLI to make requests with. " + "If specified, the configuration step will be skipped.", + "LINODE_CLI_CA": "The path to a custom Certificate Authority file to verify " + "API requests against.", + "LINODE_CLI_API_HOST": "Overrides the target host for API requests. " + "(e.g. 'api.linode.com')", + "LINODE_CLI_API_VERSION": "Overrides the target Linode API version for API requests. " + "(e.g. 'v4beta')", + "LINODE_CLI_API_SCHEME": "Overrides the target scheme used for API requests. " + "(e.g. 'https')", + } + + table = Table(show_header=True, header_style="", box=box.SQUARE) + table.add_column("Name") + table.add_column("Description") + + for k, v in env_variables.items(): + table.add_row(k, v) + + rprint(table) + # commands to manage CLI users (don't call out to API) print("\nCLI user management commands:") um_commands = [["configure", "set-user", "show-users"], ["remove-user"]] diff --git a/linodecli/configuration/auth.py b/linodecli/configuration/auth.py index f37b1d487..7f62df7a1 100644 --- a/linodecli/configuration/auth.py +++ b/linodecli/configuration/auth.py @@ -11,6 +11,8 @@ import requests +from linodecli.helpers import API_CA_PATH + TOKEN_GENERATION_URL = "https://cloud.linode.com/profile/tokens" # This is used for web-based configuration OAUTH_CLIENT_ID = "5823b4627e45411d18e9" @@ -76,7 +78,9 @@ def _do_request( headers["Authorization"] = f"Bearer {token}" headers["Content-type"] = "application/json" - result = method(base_url + url, headers=headers, json=body) + result = method( + base_url + url, headers=headers, json=body, verify=API_CA_PATH + ) _handle_response_status( result, exit_on_error=exit_on_error, status_validator=status_validator @@ -92,7 +96,10 @@ def _check_full_access(base_url, token): } result = requests.get( - base_url + "/profile/grants", headers=headers, timeout=120 + base_url + "/profile/grants", + headers=headers, + timeout=120, + verify=API_CA_PATH, ) _handle_response_status(result, exit_on_error=True) diff --git a/linodecli/helpers.py b/linodecli/helpers.py index 32ec64d17..5c3631377 100644 --- a/linodecli/helpers.py +++ b/linodecli/helpers.py @@ -13,6 +13,11 @@ API_VERSION_OVERRIDE = os.getenv("LINODE_CLI_API_VERSION") API_SCHEME_OVERRIDE = os.getenv("LINODE_CLI_API_SCHEME") +# A user-specified path to the CA file for use in API requests. +# This field defaults to True to enable default verification if +# no path is specified. +API_CA_PATH = os.getenv("LINODE_CLI_CA", None) or True + def handle_url_overrides(url): """ diff --git a/tests/unit/test_api_request.py b/tests/unit/test_api_request.py index 29507cee4..5c7b03bf1 100644 --- a/tests/unit/test_api_request.py +++ b/tests/unit/test_api_request.py @@ -96,7 +96,7 @@ def test_build_filter_header(self, list_operation): def test_do_request_get(self, mock_cli, list_operation): mock_response = Mock(status_code=200, reason="OK") - def validate_http_request(url, headers=None, data=None): + def validate_http_request(url, headers=None, data=None, **kwargs): assert url == "http://localhost/foo/bar?page=1&page_size=100" assert headers["X-Filter"] == json.dumps( {"filterable_result": "cool"} @@ -116,7 +116,7 @@ def validate_http_request(url, headers=None, data=None): def test_do_request_post(self, mock_cli, create_operation): mock_response = Mock(status_code=200, reason="OK") - def validate_http_request(url, headers=None, data=None): + def validate_http_request(url, headers=None, data=None, **kwargs): assert url == "http://localhost/foo/bar" assert data == json.dumps( { From 959ca53941fa6eb55abad7cc075c25be4782076e Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Tue, 13 Jun 2023 15:02:18 -0400 Subject: [PATCH 10/13] ref: Remove logic for resolving version from tag; failover to `v0.0.0` (#466) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description This change drops the logic that causes the CLI default to the latest annotated git tag, which causes installation issues when the repo is cloned directly from a branch or does not have the `.git` directory. If no version environment variable or baked file is defined, the version will now default to `v0.0.0`. ## ✔️ How to Test ``` make install ``` --- version | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/version b/version index 5e38002bd..662219f5c 100755 --- a/version +++ b/version @@ -19,23 +19,15 @@ def get_version_env(): def get_version(ref="HEAD"): # We want to override the version if an environment variable is specified. # This is useful for certain release and testing pipelines. - describe = get_version_env() - - if describe is None: - describe = call('git describe {} --tags'.format(ref)) + version_str = get_version_env() or "0.0.0" # Strip the `v` prefix if specified - if describe.startswith("v"): - describe = describe[1:] + if version_str.startswith("v"): + version_str = version_str[1:] - parts = LooseVersion(describe).version[:3] + parts = LooseVersion(version_str).version[:3] return tuple(parts) -def call(cmd): - _, output = subprocess.getstatusoutput(cmd) - return output - - major, minor, patch = get_version() print("{}.{}.{}".format(major, minor, patch)) From 4fa0cb6a2220d0bceb648950284253c9b7fb1627 Mon Sep 17 00:00:00 2001 From: ykim-1 <126618609+ykim-1@users.noreply.github.com> Date: Wed, 14 Jun 2023 08:07:06 -0700 Subject: [PATCH 11/13] Remove acctest command and add hash check (#473) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description Removing acctest command dependencies and add necessary workflow with checks ## ✔️ How to Test **What are the steps to reproduce the issue or verify the changes?** **How do I run the relevant unit/integration tests?** ## 📷 Preview **If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.** --- .github/workflows/e2e-suite-pr.yml | 92 +++++++++++++++++++++++------- 1 file changed, 72 insertions(+), 20 deletions(-) diff --git a/.github/workflows/e2e-suite-pr.yml b/.github/workflows/e2e-suite-pr.yml index 74b377197..72554e44d 100644 --- a/.github/workflows/e2e-suite-pr.yml +++ b/.github/workflows/e2e-suite-pr.yml @@ -1,24 +1,30 @@ on: pull_request: - repository_dispatch: - types: [acctest-command] + workflow_dispatch: + inputs: + test_path: + description: 'Test path to be tested: e.g. integration/cli' + required: false + sha: + description: 'The hash value of the commit.' + required: true + pull_request_number: + description: 'The number of the PR.' + required: false name: PR E2E Tests jobs: - # Maintainer has commented /acctest on a pull request integration-fork-ubuntu: runs-on: ubuntu-latest if: - github.event_name == 'repository_dispatch' && - github.event.client_payload.slash_command.sha != '' && - github.event.client_payload.pull_request.head.sha == github.event.client_payload.slash_command.sha + github.event_name == 'workflow_dispatch' && inputs.sha != '' steps: - uses: actions-ecosystem/action-regex-match@v2 id: validate-tests with: - text: ${{ github.event.client_payload.slash_command.tests }} + text: ${{ inputs.test_path }} regex: '[^a-z0-9-:.\/_]' # Tests validation flags: gi @@ -26,7 +32,30 @@ jobs: - name: Checkout PR uses: actions/checkout@v3 with: - ref: ${{ github.event.client_payload.slash_command.sha }} + ref: ${{ inputs.sha }} + + - name: Get the hash value of the latest commit from the PR branch + uses: octokit/graphql-action@v2.x + id: commit-hash + if: ${{ inputs.pull_request_number != '' }} + with: + query: | + query PRHeadCommitHash($owner: String!, $repo: String!, $pr_num: Int!) { + repository(owner:$owner, name:$repo) { + pullRequest(number: $pr_num) { + headRef { + target { + ... on Commit { + oid + } + } + } + } + } + } + owner: ${{ github.event.repository.owner.login }} + repo: ${{ github.event.repository.name }} + pr_num: ${{ fromJSON(inputs.pull_request_number) }} - name: Update system packages run: sudo apt-get update -y @@ -47,16 +76,16 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - run: make INTEGRATION_TEST_PATH="${{ github.event.client_payload.slash_command.tests }}" TEST_CASE="${{ github.event.client_payload.slash_command.testcase }}" testint + - run: make INTEGRATION_TEST_PATH="${{ inputs.test_path }}" testint if: ${{ steps.validate-tests.outputs.match == '' }} env: LINODE_CLI_TOKEN: ${{ secrets.LINODE_TOKEN }} - - uses: actions/github-script@v5 + - uses: actions/github-script@v6 id: update-check-run - if: ${{ always() }} + if: ${{ inputs.pull_request_number != '' && fromJson(steps.commit-hash.outputs.data).repository.pullRequest.headRef.target.oid == inputs.sha }} env: - number: ${{ github.event.client_payload.pull_request.number }} + number: ${{ inputs.pull_request_number }} job: ${{ github.job }} conclusion: ${{ job.status }} with: @@ -83,15 +112,13 @@ jobs: integration-fork-windows: runs-on: windows-latest if: - github.event_name == 'repository_dispatch' && - github.event.client_payload.slash_command.sha != '' && - github.event.client_payload.pull_request.head.sha == github.event.client_payload.slash_command.sha + github.event_name == 'workflow_dispatch' && inputs.sha != '' steps: - uses: actions-ecosystem/action-regex-match@v2 id: validate-tests with: - text: ${{ github.event.client_payload.slash_command.tests }} + text: ${{ inputs.test_path }} regex: '[^a-z0-9-:.\/_]' # Tests validation flags: gi @@ -99,7 +126,32 @@ jobs: - name: Checkout PR uses: actions/checkout@v3 with: - ref: ${{ github.event.client_payload.slash_command.sha }} + ref: ${{ inputs.sha }} + + - name: Get the hash value of the latest commit from the PR branch + uses: octokit/graphql-action@v2.x + id: commit-hash + if: ${{ inputs.pull_request_number != '' }} + with: + query: | + query PRHeadCommitHash($owner: String!, $repo: String!, $pr_num: Int!) { + repository(owner:$owner, name:$repo) { + pullRequest(number: $pr_num) { + headRef { + target { + ... on Commit { + oid + } + } + } + } + } + } + owner: ${{ github.event.repository.owner.login }} + repo: ${{ github.event.repository.name }} + pr_num: ${{ fromJSON(inputs.pull_request_number) }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Setup Python uses: actions/setup-python@v4 @@ -114,13 +166,13 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - run: make INTEGRATION_TEST_PATH="${{ github.event.client_payload.slash_command.tests }}" TEST_CASE="${{ github.event.client_payload.slash_command.testcase }}" testint + - run: make INTEGRATION_TEST_PATH="${{ inputs.test_path }}" testint env: LINODE_CLI_TOKEN: ${{ secrets.LINODE_TOKEN_2 }} - - uses: actions/github-script@v5 + - uses: actions/github-script@v6 id: update-check-run - if: ${{ always() }} + if: ${{ inputs.pull_request_number != '' && fromJson(steps.commit-hash.outputs.data).repository.pullRequest.headRef.target.oid == inputs.sha }} env: number: ${{ github.event.client_payload.pull_request.number }} job: ${{ github.job }} From bcac1cec01236665c5e547e3043ac503a02c11db Mon Sep 17 00:00:00 2001 From: Zhiwei Liang <121905282+zliang-akamai@users.noreply.github.com> Date: Wed, 14 Jun 2023 11:11:05 -0400 Subject: [PATCH 12/13] Unpin Dev Requirements Version (#471) --- requirements-dev.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 1a9898100..e845dfcc2 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,8 +1,8 @@ -pylint==2.17.4 -pytest==7.3.1 +pylint>=2.17.4 +pytest>=7.3.1 black>=23.1.0 isort>=5.12.0 autoflake>=2.0.1 -pytest-mock==3.10.0 -requests-mock==1.10.0 +pytest-mock>=3.10.0 +requests-mock==1.11.0 boto3-stubs[s3] From 7c9e5495f1dd09eae2dc478d2f447ef721a6b61d Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Wed, 14 Jun 2023 12:33:31 -0400 Subject: [PATCH 13/13] new: Add a `No Default` option when running the `configure` command (#472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description This change adds a `No Default` option to the `linode-cli configure` command that allows users to optionally clear a default if it is already defined in the existing config. This differs from the skip (enter) functionality in that the skip functionality retains the previous config value and the `No Default` option drops the config value. ## ✔️ How to Test `make testunit` --- linodecli/configuration/__init__.py | 28 +++++- linodecli/configuration/helpers.py | 73 ++++++++++---- tests/unit/test_configuration.py | 150 ++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 21 deletions(-) diff --git a/linodecli/configuration/__init__.py b/linodecli/configuration/__init__.py index 81360ce32..ad6f44d51 100644 --- a/linodecli/configuration/__init__.py +++ b/linodecli/configuration/__init__.py @@ -15,6 +15,7 @@ ) from .helpers import ( _check_browsers, + _config_get_with_default, _default_thing_input, _get_config, _get_config_path, @@ -382,6 +383,9 @@ def configure( regions, "Default Region (Optional): ", "Please select a valid Region, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "region" + ), ) config["type"] = _default_thing_input( @@ -389,6 +393,9 @@ def configure( types, "Default Type of Linode (Optional): ", "Please select a valid Type, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "type" + ), ) config["image"] = _default_thing_input( @@ -396,6 +403,9 @@ def configure( images, "Default Image (Optional): ", "Please select a valid Image, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "image" + ), ) config["mysql_engine"] = _default_thing_input( @@ -403,6 +413,9 @@ def configure( mysql_engines, "Default Engine (Optional): ", "Please select a valid MySQL Database Engine, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "mysql_engine" + ), ) config["postgresql_engine"] = _default_thing_input( @@ -410,6 +423,9 @@ def configure( postgresql_engines, "Default Engine (Optional): ", "Please select a valid PostgreSQL Database Engine, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "postgresql_engine" + ), ) if auth_users: @@ -418,6 +434,9 @@ def configure( auth_users, "Default Option (Optional): ", "Please select a valid Option, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "authorized_users" + ), ) # save off the new configuration @@ -446,8 +465,13 @@ def configure( print(f"Active user is now {username}") for k, v in config.items(): - if v: - self.config.set(username, k, v) + if v is None: + if self.config.has_option(username, k): + self.config.remove_option(username, k) + + continue + + self.config.set(username, k, v) self.write_config() os.chmod(_get_config_path(), 0o600) diff --git a/linodecli/configuration/helpers.py b/linodecli/configuration/helpers.py index 96db226cd..71abc72f4 100644 --- a/linodecli/configuration/helpers.py +++ b/linodecli/configuration/helpers.py @@ -5,6 +5,7 @@ import configparser import os import webbrowser +from typing import Any, Optional from .auth import _do_get_request @@ -84,7 +85,7 @@ def _check_browsers(): def _default_thing_input( - ask, things, prompt, error, optional=True + ask, things, prompt, error, optional=True, current_value=None ): # pylint: disable=too-many-arguments """ Requests the user choose from a list of things with the given prompt and @@ -92,30 +93,64 @@ def _default_thing_input( enter to not configure this option. """ print(f"\n{ask} Choices are:") + + exists = current_value is not None + + idx_offset = int(exists) + 1 + + # If there is a current value, users should have the option to clear it + if exists: + print(" 1 - No Default") + for ind, thing in enumerate(things): - print(f" {ind + 1} - {thing}") + print(f" {ind + idx_offset} - {thing}") print() - ret = "" while True: - choice = input(prompt) - - if choice: - try: - choice = int(choice) - choice = things[choice - 1] - except: - pass - - if choice in list(things): - ret = choice - break - print(error) - else: + choice_idx = input(prompt) + + if not choice_idx: + # The user wants to skip this config option if optional: - break + return current_value + + print(error) + continue + + try: + choice_idx = int(choice_idx) + except: + # Re-prompt if invalid value + continue + + # The user wants to drop this default + if exists and choice_idx == 1: + return None + + # We need to shift the index to account for the "No Default" option + choice_idx -= idx_offset + + # Validate index + if choice_idx >= len(things) or choice_idx < 0: print(error) - return ret + continue + + # Choice was valid; return + return things[choice_idx] + + +def _config_get_with_default( + config: configparser.ConfigParser, + user: str, + field: str, + default: Any = None, +) -> Optional[Any]: + """ + Gets a ConfigParser value and returns a default value if the key isn't found. + """ + return ( + config.get(user, field) if config.has_option(user, field) else default + ) def _handle_no_default_user(self): # pylint: disable=too-many-branches diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index 62ae29ff3..49a35d402 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -13,6 +13,7 @@ import requests_mock from linodecli import configuration +from linodecli.configuration import _default_thing_input class TestConfiguration: @@ -371,3 +372,152 @@ def mock_input(prompt): # make sure that we set the default engine value according to type of database assert conf.get_value("mysql_engine") == "mysql/test-engine" assert conf.get_value("postgresql_engine") == "postgresql/test-engine" + + def test_default_thing_input_no_current(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("1\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", ["foo", "bar"], "prompt text", "error text" + ) + + output_lines = stdout_buf.getvalue().splitlines() + + assert output_lines == [ + "", + "foo", + " Choices are:", + " 1 - foo", + " 2 - bar", + "", + "prompt text", + ] + + assert result == "foo" + + def test_default_thing_input_skip(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + current_value="foo", + ) + + output_lines = stdout_buf.getvalue().splitlines() + + print(output_lines) + + assert output_lines == [ + "", + "foo", + " Choices are:", + " 1 - No Default", + " 2 - foo", + " 3 - bar", + "", + "prompt text", + ] + + assert result == "foo" + + def test_default_thing_input_no_default(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("1\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + current_value="foo", + ) + + output_lines = stdout_buf.getvalue().splitlines() + + print(output_lines) + + assert output_lines == [ + "", + "foo", + " Choices are:", + " 1 - No Default", + " 2 - foo", + " 3 - bar", + "", + "prompt text", + ] + + assert result is None + + def test_default_thing_input_valid(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("3\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + current_value="foo", + ) + + output_lines = stdout_buf.getvalue().splitlines() + + print(output_lines) + + assert output_lines == [ + "", + "foo", + " Choices are:", + " 1 - No Default", + " 2 - foo", + " 3 - bar", + "", + "prompt text", + ] + + assert result == "bar" + + def test_default_thing_input_valid_no_current(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("3\n1\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + ) + + output = stdout_buf.getvalue() + + assert "error text" in output + + assert result == "foo" + + def test_default_thing_input_out_of_range(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("4\n2\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + current_value="foo", + ) + + output = stdout_buf.getvalue() + assert "error text" in output + + assert result == "foo"