From 592ddb3a4d12aeeaa69c97c86a1cd6ee923533b5 Mon Sep 17 00:00:00 2001 From: Jacob Riddle <87780794+jriddle-linode@users.noreply.github.com> Date: Tue, 5 Mar 2024 11:55:38 -0500 Subject: [PATCH] v5.49.0 (#584) Co-authored-by: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Co-authored-by: Zhiwei Liang <121905282+zliang-akamai@users.noreply.github.com> --- Makefile | 4 ++ linodecli/api_request.py | 10 +-- linodecli/arg_helpers.py | 21 ++++-- linodecli/baked/operation.py | 59 +++++++++++++++-- linodecli/baked/request.py | 50 ++++++++++++--- linodecli/cli.py | 11 ++++ linodecli/plugins/metadata.py | 17 +++-- .../api_request_test_foobar_post.yaml | 5 ++ tests/integration/helpers.py | 31 +-------- tests/integration/linodes/test_interfaces.py | 64 ++++++++++++++++++- .../integration/networking/test_networking.py | 6 +- tests/unit/conftest.py | 2 +- tests/unit/test_api_request.py | 1 + tests/unit/test_arg_helpers.py | 25 ++++++-- tests/unit/test_cli.py | 5 ++ tests/unit/test_operation.py | 52 +++++++++++++++ tests/unit/test_plugin_metadata.py | 7 +- 17 files changed, 298 insertions(+), 72 deletions(-) diff --git a/Makefile b/Makefile index b2a115a59..12148fedc 100644 --- a/Makefile +++ b/Makefile @@ -20,8 +20,12 @@ install: check-prerequisites requirements build .PHONY: bake bake: clean +ifeq ($(SKIP_BAKE), 1) + @echo Skipping bake stage +else python3 -m linodecli bake ${SPEC} --skip-config cp data-3 linodecli/ +endif .PHONY: build build: clean bake diff --git a/linodecli/api_request.py b/linodecli/api_request.py index c61c47632..abb851c75 100644 --- a/linodecli/api_request.py +++ b/linodecli/api_request.py @@ -6,7 +6,6 @@ import json import sys import time -from sys import version_info from typing import Any, Iterable, List, Optional import requests @@ -69,10 +68,7 @@ def do_request( headers = { "Authorization": f"Bearer {ctx.config.get_token()}", "Content-Type": "application/json", - "User-Agent": ( - f"linode-cli:{ctx.version} " - f"python/{version_info[0]}.{version_info[1]}.{version_info[2]}" - ), + "User-Agent": ctx.user_agent, } parsed_args = operation.parse_args(args) @@ -257,11 +253,15 @@ def _build_request_body(ctx, operation, parsed_args) -> Optional[str]: # expand paths for k, v in vars(parsed_args).items(): + if v is None: + continue + cur = expanded_json for part in k.split(".")[:-1]: if part not in cur: cur[part] = {} cur = cur[part] + cur[k.split(".")[-1]] = v return json.dumps(_traverse_request_body(expanded_json)) diff --git a/linodecli/arg_helpers.py b/linodecli/arg_helpers.py index 0b3f50f4f..6add0c7cc 100644 --- a/linodecli/arg_helpers.py +++ b/linodecli/arg_helpers.py @@ -341,7 +341,7 @@ def help_with_ops(ops, config): ) -def action_help(cli, command, action): +def action_help(cli, command, action): # pylint: disable=too-many-branches """ Prints help relevant to the command and action """ @@ -398,11 +398,24 @@ def action_help(cli, command, action): if op.method in {"post", "put"} and arg.required else "" ) - nullable_fmt = " (nullable)" if arg.nullable else "" - print( - f" --{arg.path}: {is_required}{arg.description}{nullable_fmt}" + + extensions = [] + + if arg.format == "json": + extensions.append("JSON") + + if arg.nullable: + extensions.append("nullable") + + if arg.is_parent: + extensions.append("conflicts with children") + + suffix = ( + f" ({', '.join(extensions)})" if len(extensions) > 0 else "" ) + print(f" --{arg.path}: {is_required}{arg.description}{suffix}") + def bake_command(cli, spec_loc): """ diff --git a/linodecli/baked/operation.py b/linodecli/baked/operation.py index b8141adc0..a104710be 100644 --- a/linodecli/baked/operation.py +++ b/linodecli/baked/operation.py @@ -8,9 +8,10 @@ import platform import re import sys +from collections import defaultdict from getpass import getpass from os import environ, path -from typing import List, Tuple +from typing import Any, List, Tuple from openapi3.paths import Operation @@ -427,14 +428,14 @@ def _add_args_post_put(self, parser) -> List[Tuple[str, str]]: action=ArrayAction, type=arg_type_handler, ) - elif arg.list_item: + elif arg.is_child: parser.add_argument( "--" + arg.path, metavar=arg.name, action=ListArgumentAction, type=arg_type_handler, ) - list_items.append((arg.path, arg.list_parent)) + list_items.append((arg.path, arg.parent)) else: if arg.datatype == "string" and arg.format == "password": # special case - password input @@ -463,10 +464,51 @@ def _add_args_post_put(self, parser) -> List[Tuple[str, str]]: return list_items + def _validate_parent_child_conflicts(self, parsed: argparse.Namespace): + """ + This method validates that no child arguments (e.g. --interfaces.purpose) are + specified alongside their parent (e.g. --interfaces). + """ + conflicts = defaultdict(list) + + for arg in self.args: + parent = arg.parent + arg_value = getattr(parsed, arg.path, None) + + if parent is None or arg_value is None: + continue + + # Special case to ignore child arguments that are not specified + # but are implicitly populated by ListArgumentAction. + if isinstance(arg_value, list) and arg_value.count(None) == len( + arg_value + ): + continue + + # If the parent isn't defined, we can + # skip this one + if getattr(parsed, parent) is None: + continue + + # We found a conflict + conflicts[parent].append(arg) + + # No conflicts found + if len(conflicts) < 1: + return + + for parent, args in conflicts.items(): + arg_format = ", ".join([f"--{v.path}" for v in args]) + print( + f"Argument(s) {arg_format} cannot be specified when --{parent} is specified.", + file=sys.stderr, + ) + + sys.exit(2) + @staticmethod def _handle_list_items( - list_items, - parsed, + list_items: List[Tuple[str, str]], parsed: Any ): # pylint: disable=too-many-locals,too-many-branches,too-many-statements lists = {} @@ -563,4 +605,9 @@ def parse_args(self, args): elif self.method in ("post", "put"): list_items = self._add_args_post_put(parser) - return self._handle_list_items(list_items, parser.parse_args(args)) + parsed = parser.parse_args(args) + + if self.method in ("post", "put"): + self._validate_parent_child_conflicts(parsed) + + return self._handle_list_items(list_items, parsed) diff --git a/linodecli/baked/request.py b/linodecli/baked/request.py index 818a8263f..450ba25b6 100644 --- a/linodecli/baked/request.py +++ b/linodecli/baked/request.py @@ -9,7 +9,13 @@ class OpenAPIRequestArg: """ def __init__( - self, name, schema, required, prefix=None, list_parent=None + self, + name, + schema, + required, + prefix=None, + is_parent=False, + parent=None, ): # pylint: disable=too-many-arguments """ Parses a single Schema node into a argument the CLI can use when making @@ -23,6 +29,10 @@ def __init__( :param prefix: The prefix for this arg's path, used in the actual argument to the CLI to ensure unique arg names :type prefix: str + :param is_parent: Whether this argument is a parent to child fields. + :type is_parent: bool + :param parent: If applicable, the path to the parent list for this argument. + :type parent: Optional[str] """ #: The name of this argument, mostly used for display and docs self.name = name @@ -50,6 +60,11 @@ def __init__( schema.extensions.get("linode-cli-format") or schema.format or None ) + # If this is a deeply nested array we should treat it as JSON. + # This allows users to specify fields like --interfaces.ip_ranges. + if is_parent or (schema.type == "array" and parent is not None): + self.format = "json" + #: The type accepted for this argument. This will ultimately determine what #: we accept in the ArgumentParser self.datatype = ( @@ -59,13 +74,16 @@ def __init__( #: The type of item accepted in this list; if None, this is not a list self.item_type = None - #: Whether the argument is a field in a nested list. - self.list_item = list_parent is not None + #: Whether the argument is a parent to child fields. + self.is_parent = is_parent + + #: Whether the argument is a nested field. + self.is_child = parent is not None #: The name of the list this argument falls under. #: This allows nested dictionaries to be specified in lists of objects. #: e.g. --interfaces.ipv4.nat_1_1 - self.list_parent = list_parent + self.parent = parent #: The path of the path element in the schema. self.prefix = prefix @@ -85,7 +103,7 @@ def __init__( ) -def _parse_request_model(schema, prefix=None, list_parent=None): +def _parse_request_model(schema, prefix=None, parent=None): """ Parses a schema into a list of OpenAPIRequest objects :param schema: The schema to parse as a request model @@ -107,8 +125,11 @@ def _parse_request_model(schema, prefix=None, list_parent=None): if v.type == "object" and not v.readOnly and v.properties: # nested objects receive a prefix and are otherwise parsed normally pref = prefix + "." + k if prefix else k + args += _parse_request_model( - v, prefix=pref, list_parent=list_parent + v, + prefix=pref, + parent=parent, ) elif ( v.type == "array" @@ -119,9 +140,20 @@ def _parse_request_model(schema, prefix=None, list_parent=None): # handle lists of objects as a special case, where each property # of the object in the list is its own argument pref = prefix + "." + k if prefix else k - args += _parse_request_model( - v.items, prefix=pref, list_parent=pref + + # Support specifying this list as JSON + args.append( + OpenAPIRequestArg( + k, + v.items, + False, + prefix=prefix, + is_parent=True, + parent=parent, + ) ) + + args += _parse_request_model(v.items, prefix=pref, parent=pref) else: # required fields are defined in the schema above the property, so # we have to check here if required fields are defined/if this key @@ -131,7 +163,7 @@ def _parse_request_model(schema, prefix=None, list_parent=None): required = k in schema.required args.append( OpenAPIRequestArg( - k, v, required, prefix=prefix, list_parent=list_parent + k, v, required, prefix=prefix, parent=parent ) ) diff --git a/linodecli/cli.py b/linodecli/cli.py index b89ee09bb..09a69e2dc 100644 --- a/linodecli/cli.py +++ b/linodecli/cli.py @@ -193,3 +193,14 @@ def find_operation(self, command, action): # Fail if no matching alias was found raise ValueError(f"No action {action} for command {command}") + + @property + def user_agent(self) -> str: + """ + Returns the User-Agent to use when making API requests. + """ + return ( + f"linode-cli/{self.version} " + f"linode-api-docs/{self.spec_version} " + f"python/{version_info[0]}.{version_info[1]}.{version_info[2]}" + ) diff --git a/linodecli/plugins/metadata.py b/linodecli/plugins/metadata.py index 7587da309..2d9597da5 100644 --- a/linodecli/plugins/metadata.py +++ b/linodecli/plugins/metadata.py @@ -57,11 +57,16 @@ def print_ssh_keys_table(data): """ table = Table(show_lines=True) - table.add_column("ssh keys") + table.add_column("user") + table.add_column("ssh key") - if data.users.root is not None: - for key in data.users.root: - table.add_row(key) + for name, keys in data.users.items(): + # Keys will be None if no keys are configured for the user + if keys is None: + continue + + for key in keys: + table.add_row(name, key) rprint(table) @@ -189,7 +194,7 @@ def get_metadata_parser(): return parser -def call(args, _): +def call(args, context): """ The entrypoint for this plugin """ @@ -204,7 +209,7 @@ def call(args, _): # make a client, but only if we weren't printing help and endpoint is valid if "--help" not in args: try: - client = MetadataClient() + client = MetadataClient(user_agent=context.client.user_agent) except ConnectTimeout as exc: raise ConnectionError( "Can't access Metadata service. Please verify that you are inside a Linode." diff --git a/tests/fixtures/api_request_test_foobar_post.yaml b/tests/fixtures/api_request_test_foobar_post.yaml index f7dd8d934..5ce433eb7 100644 --- a/tests/fixtures/api_request_test_foobar_post.yaml +++ b/tests/fixtures/api_request_test_foobar_post.yaml @@ -114,6 +114,11 @@ components: nested_int: type: number description: A deeply nested integer. + field_array: + type: array + description: An arbitrary deeply nested array. + items: + type: string field_string: type: string description: An arbitrary field. diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 95edd2613..05b207417 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -8,9 +8,6 @@ INVALID_HOST = "https://wrongapi.linode.com" SUCCESS_STATUS_CODE = 0 FAILED_STATUS_CODE = 256 -INVALID_HOST = "https://wrongapi.linode.com" -SUCCESS_STATUS_CODE = 0 - COMMAND_JSON_OUTPUT = ["--suppress-warnings", "--no-defaults", "--json"] @@ -32,19 +29,13 @@ def wait_for_condition(interval: int, timeout: int, condition: Callable): def exec_test_command(args: List[str]): - process = subprocess.run( - args, - stdout=subprocess.PIPE, - ) + process = subprocess.run(args, stdout=subprocess.PIPE) assert process.returncode == 0 return process def exec_failing_test_command(args: List[str], expected_code: int = 1): - process = subprocess.run( - args, - stderr=subprocess.PIPE, - ) + process = subprocess.run(args, stderr=subprocess.PIPE) assert process.returncode == expected_code return process @@ -143,23 +134,5 @@ def remove_all(target: str): exec_test_command(["linode-cli", target, "delete", id]) -def exec_test_command(args: List[str]): - process = subprocess.run( - args, - stdout=subprocess.PIPE, - ) - assert process.returncode == 0 - return process - - -def exec_failing_test_command(args: List[str]): - process = subprocess.run( - args, - stderr=subprocess.PIPE, - ) - assert process.returncode == 1 - return process - - def count_lines(text: str): return len(list(filter(len, text.split("\n")))) diff --git a/tests/integration/linodes/test_interfaces.py b/tests/integration/linodes/test_interfaces.py index e963110d0..3e961e716 100644 --- a/tests/integration/linodes/test_interfaces.py +++ b/tests/integration/linodes/test_interfaces.py @@ -1,5 +1,6 @@ import json import time +from typing import Any, Dict import pytest @@ -47,6 +48,8 @@ def linode_with_vpc_interface(): "any", "--interfaces.ipv4.vpc", "10.0.0.5", + "--interfaces.ip_ranges", + json.dumps(["10.0.0.6/32"]), "--interfaces.purpose", "public", "--json", @@ -63,9 +66,57 @@ def linode_with_vpc_interface(): delete_target_id(target="vpcs", id=vpc_id) -def test_with_vpc_interface(linode_with_vpc_interface): - linode_json, vpc_json = linode_with_vpc_interface +@pytest.fixture +def linode_with_vpc_interface_as_json(): + vpc_json = create_vpc_w_subnet() + + vpc_region = vpc_json["region"] + vpc_id = str(vpc_json["id"]) + subnet_id = int(vpc_json["subnets"][0]["id"]) + + linode_json = json.loads( + exec_test_command( + BASE_CMD + + [ + "create", + "--type", + "g6-nanode-1", + "--region", + vpc_region, + "--image", + DEFAULT_TEST_IMAGE, + "--root_pass", + DEFAULT_RANDOM_PASS, + "--interfaces", + json.dumps( + [ + { + "purpose": "vpc", + "primary": True, + "subnet_id": subnet_id, + "ipv4": {"nat_1_1": "any", "vpc": "10.0.0.5"}, + "ip_ranges": ["10.0.0.6/32"], + }, + {"purpose": "public"}, + ] + ), + "--json", + "--suppress-warnings", + ] + ) + .stdout.decode() + .rstrip() + )[0] + + yield linode_json, vpc_json + delete_target_id(target="linodes", id=str(linode_json["id"])) + delete_target_id(target="vpcs", id=vpc_id) + + +def assert_interface_configuration( + linode_json: Dict[str, Any], vpc_json: Dict[str, Any] +): config_json = json.loads( exec_test_command( BASE_CMD @@ -89,6 +140,15 @@ def test_with_vpc_interface(linode_with_vpc_interface): assert vpc_interface["vpc_id"] == vpc_json["id"] assert vpc_interface["ipv4"]["vpc"] == "10.0.0.5" assert vpc_interface["ipv4"]["nat_1_1"] == linode_json["ipv4"][0] + assert vpc_interface["ip_ranges"][0] == "10.0.0.6/32" assert not public_interface["primary"] assert public_interface["purpose"] == "public" + + +def test_with_vpc_interface(linode_with_vpc_interface): + assert_interface_configuration(*linode_with_vpc_interface) + + +def test_with_vpc_interface_as_json(linode_with_vpc_interface_as_json): + assert_interface_configuration(*linode_with_vpc_interface_as_json) diff --git a/tests/integration/networking/test_networking.py b/tests/integration/networking/test_networking.py index 267e03c29..53fdc3d35 100644 --- a/tests/integration/networking/test_networking.py +++ b/tests/integration/networking/test_networking.py @@ -2,6 +2,7 @@ import re import pytest +from _pytest.monkeypatch import MonkeyPatch from tests.integration.helpers import delete_target_id, exec_test_command from tests.integration.linodes.helpers_linodes import ( @@ -121,8 +122,11 @@ def test_allocate_additional_private_ipv4_address(test_linode_id): ) -def test_share_ipv4_address(test_linode_id_shared_ipv4): +def test_share_ipv4_address( + test_linode_id_shared_ipv4, monkeypatch: MonkeyPatch +): target_linode, parent_linode = test_linode_id_shared_ipv4 + monkeypatch.setenv("LINODE_CLI_API_VERSION", "v4beta") # Allocate an IPv4 address on the parent Linode ip_address = json.loads( diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index e6a3e8129..cc1216c23 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -61,7 +61,7 @@ def _get_parsed_spec(filename): @pytest.fixture def mock_cli( - version="DEVELOPMENT", + version="0.0.0", url="http://localhost", defaults=True, ): diff --git a/tests/unit/test_api_request.py b/tests/unit/test_api_request.py index 6e4cade6d..24cfc434f 100644 --- a/tests/unit/test_api_request.py +++ b/tests/unit/test_api_request.py @@ -337,6 +337,7 @@ def validate_http_request(url, headers=None, data=None, **kwargs): ] } ) + assert headers["User-Agent"] == mock_cli.user_agent assert "Authorization" in headers assert data is None diff --git a/tests/unit/test_arg_helpers.py b/tests/unit/test_arg_helpers.py index 99b65fdc3..e934158f0 100644 --- a/tests/unit/test_arg_helpers.py +++ b/tests/unit/test_arg_helpers.py @@ -184,13 +184,22 @@ def test_action_help_post_method(self, capsys, mocker, mock_cli): {"lang": "CLI", "source": "linode-cli command action\n --bar=foo"}, ] - mocked_args = mocker.MagicMock() - mocked_args.read_only = False - mocked_args.required = True - mocked_args.path = "path" - mocked_args.description = "test description" - - mocked_ops.args = [mocked_args] + mocked_ops.args = [ + mocker.MagicMock( + read_only=False, + required=True, + path="path", + description="test description", + ), + mocker.MagicMock( + read_only=False, + required=False, + path="path2", + description="test description 2", + format="json", + nullable=True, + ), + ] mock_cli.find_operation = mocker.Mock(return_value=mocked_ops) @@ -209,7 +218,9 @@ def test_action_help_post_method(self, capsys, mocker, mock_cli): ) in captured.out assert "Arguments" in captured.out assert "test description" in captured.out + assert "test description 2" in captured.out assert "(required)" in captured.out + assert "(JSON, nullable, conflicts with children)" in captured.out assert "filter results" not in captured.out def test_action_help_get_method(self, capsys, mocker, mock_cli): diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index eddfbc4db..b7f41a79c 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -73,6 +73,11 @@ def test_find_operation( mock_cli.find_operation("foo", "cool") mock_cli.find_operation("cool", "cool") + def test_user_agent(self, mock_cli: CLI): + assert re.compile( + r"linode-cli/[0-9]+\.[0-9]+\.[0-9]+ linode-api-docs/[0-9]+\.[0-9]+\.[0-9]+ python/[0-9]+\.[0-9]+\.[0-9]+" + ).match(mock_cli.user_agent) + def test_get_all_pages( mock_cli: CLI, list_operation: OpenAPIOperation, monkeypatch: MonkeyPatch diff --git a/tests/unit/test_operation.py b/tests/unit/test_operation.py index 2f8d518c6..36c583259 100644 --- a/tests/unit/test_operation.py +++ b/tests/unit/test_operation.py @@ -1,4 +1,7 @@ import argparse +import contextlib +import io +import json from linodecli.baked import operation from linodecli.baked.operation import ExplicitEmptyListValue, ExplicitNullValue @@ -171,6 +174,8 @@ def test_parse_args_object_list(self, create_operation): "test2", "--object_list.field_dict.nested_int", "789", + "--object_list.field_array", + json.dumps(["foo", "bar"]), # Second object "--object_list.field_int", "456", @@ -184,11 +189,58 @@ def test_parse_args_object_list(self, create_operation): "field_string": "test1", "field_int": 123, "field_dict": {"nested_string": "test2", "nested_int": 789}, + "field_array": ["foo", "bar"], "nullable_string": None, # We expect this to be filtered out later }, {"field_int": 456, "field_dict": {"nested_string": "test3"}}, ] + def test_parse_args_object_list_json(self, create_operation): + expected = [ + { + "field_string": "test1", + "field_int": 123, + "field_dict": {"nested_string": "test2", "nested_int": 789}, + "field_array": ["foo", "bar"], + }, + {"field_int": 456, "field_dict": {"nested_string": "test3"}}, + ] + + result = create_operation.parse_args( + ["--object_list", json.dumps(expected)] + ) + + assert result.object_list == expected + + def test_parse_args_conflicting_parent_child(self, create_operation): + stderr_buf = io.StringIO() + + try: + with contextlib.redirect_stderr(stderr_buf): + create_operation.parse_args( + [ + "--object_list", + "[]", + "--object_list.field_string", + "test", + "--object_list.field_int", + "123", + "--object_list.field_dict.nested_string", + "cool", + ] + ) + except SystemExit as sys_exit: + assert sys_exit.code == 2 + else: + raise RuntimeError("Expected system exit, got none") + + stderr_result = stderr_buf.getvalue() + assert ( + "Argument(s) --object_list.field_dict.nested_string, --object_list.field_string, " + "--object_list.field_int cannot be specified when --object_list is specified." + in stderr_result + ) + def test_array_arg_action_basic(self): """ Tests a basic array argument condition.. diff --git a/tests/unit/test_plugin_metadata.py b/tests/unit/test_plugin_metadata.py index 599002019..72e3c6df7 100644 --- a/tests/unit/test_plugin_metadata.py +++ b/tests/unit/test_plugin_metadata.py @@ -116,7 +116,9 @@ def test_ssh_key_table(capsys: CaptureFixture): print_ssh_keys_table(SSH_KEYS) captured_text = capsys.readouterr() - assert "ssh keys" in captured_text.out + assert "user" in captured_text.out + assert "ssh key" in captured_text.out + assert "root" in captured_text.out assert "ssh-key-1" in captured_text.out assert "ssh-key-2" in captured_text.out @@ -125,4 +127,5 @@ def test_empty_ssh_key_table(capsys: CaptureFixture): print_ssh_keys_table(SSH_KEYS_EMPTY) captured_text = capsys.readouterr() - assert "ssh keys" in captured_text.out + assert "user" in captured_text.out + assert "ssh key" in captured_text.out