Skip to content

Commit

Permalink
fix: Resolve bug that prevented nested dict fields in object lists fr…
Browse files Browse the repository at this point in the history
…om being included in request body (#571)
  • Loading branch information
lgarber-akamai authored Jan 16, 2024
1 parent e6db9c2 commit ede396a
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 39 deletions.
2 changes: 1 addition & 1 deletion linodecli/baked/operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ def _add_args_post_put(self, parser) -> List[Tuple[str, str]]:
action=ListArgumentAction,
type=arg_type_handler,
)
list_items.append((arg.path, arg.prefix))
list_items.append((arg.path, arg.list_parent))
else:
if arg.datatype == "string" and arg.format == "password":
# special case - password input
Expand Down
19 changes: 13 additions & 6 deletions linodecli/baked/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class OpenAPIRequestArg:
"""

def __init__(
self, name, schema, required, prefix=None, list_item=False
self, name, schema, required, prefix=None, list_parent=None
): # pylint: disable=too-many-arguments
"""
Parses a single Schema node into a argument the CLI can use when making
Expand Down Expand Up @@ -60,7 +60,12 @@ def __init__(
self.item_type = None

#: Whether the argument is a field in a nested list.
self.list_item = list_item
self.list_item = list_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

#: The path of the path element in the schema.
self.prefix = prefix
Expand All @@ -80,7 +85,7 @@ def __init__(
)


def _parse_request_model(schema, prefix=None, list_of_objects=False):
def _parse_request_model(schema, prefix=None, list_parent=None):
"""
Parses a schema into a list of OpenAPIRequest objects
:param schema: The schema to parse as a request model
Expand All @@ -102,7 +107,9 @@ def _parse_request_model(schema, prefix=None, list_of_objects=False):
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)
args += _parse_request_model(
v, prefix=pref, list_parent=list_parent
)
elif (
v.type == "array"
and v.items
Expand All @@ -113,7 +120,7 @@ def _parse_request_model(schema, prefix=None, list_of_objects=False):
# 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_of_objects=True
v.items, prefix=pref, list_parent=pref
)
else:
# required fields are defined in the schema above the property, so
Expand All @@ -124,7 +131,7 @@ def _parse_request_model(schema, prefix=None, list_of_objects=False):
required = k in schema.required
args.append(
OpenAPIRequestArg(
k, v, required, prefix=prefix, list_item=list_of_objects
k, v, required, prefix=prefix, list_parent=list_parent
)
)

Expand Down
10 changes: 10 additions & 0 deletions tests/fixtures/api_request_test_foobar_post.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@ components:
type: object
description: An arbitrary object.
properties:
field_dict:
type: object
description: An arbitrary nested dict.
properties:
nested_string:
type: string
description: A deeply nested string.
nested_int:
type: number
description: A deeply nested integer.
field_string:
type: string
description: An arbitrary field.
Expand Down
41 changes: 41 additions & 0 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Use random integer as the start point here to avoid
# id conflicts when multiple testings are running.
import json
import logging
import os
import subprocess
Expand Down Expand Up @@ -402,3 +403,43 @@ def get_regions_with_capabilities(capabilities):
regions_with_all_caps.append(region_name)

return regions_with_all_caps


def create_vpc_w_subnet():
"""
Creates and returns a VPC and a corresponding subnet.
This is not directly implemented as a fixture because the teardown
order cannot be guaranteed, causing issues when attempting to
assign Linodes to a VPC in a separate fixture.
See: https://github.com/pytest-dev/pytest/issues/1216
"""

region = get_regions_with_capabilities(["VPCs"])[0]
vpc_label = str(time.time_ns()) + "label"
subnet_label = str(time.time_ns()) + "label"

vpc_json = json.loads(
exec_test_command(
[
"linode-cli",
"vpcs",
"create",
"--label",
vpc_label,
"--region",
region,
"--subnets.ipv4",
"10.0.0.0/24",
"--subnets.label",
subnet_label,
"--json",
"--suppress-warnings",
]
)
.stdout.decode()
.rstrip()
)[0]

return vpc_json
94 changes: 94 additions & 0 deletions tests/integration/linodes/test_interfaces.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import json
import time

import pytest

from tests.integration.conftest import create_vpc_w_subnet
from tests.integration.helpers import delete_target_id, exec_test_command
from tests.integration.linodes.helpers_linodes import (
BASE_CMD,
DEFAULT_LABEL,
DEFAULT_RANDOM_PASS,
DEFAULT_TEST_IMAGE,
)

timestamp = str(time.time_ns())
linode_label = DEFAULT_LABEL + timestamp


@pytest.fixture
def linode_with_vpc_interface():
vpc_json = create_vpc_w_subnet()

vpc_region = vpc_json["region"]
vpc_id = str(vpc_json["id"])
subnet_id = str(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.purpose",
"vpc",
"--interfaces.primary",
"true",
"--interfaces.subnet_id",
subnet_id,
"--interfaces.ipv4.nat_1_1",
"any",
"--interfaces.ipv4.vpc",
"10.0.0.5",
"--interfaces.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 test_with_vpc_interface(linode_with_vpc_interface):
linode_json, vpc_json = linode_with_vpc_interface

config_json = json.loads(
exec_test_command(
BASE_CMD
+ [
"configs-list",
str(linode_json["id"]),
"--json",
"--suppress-warnings",
]
)
.stdout.decode()
.rstrip()
)[0]

vpc_interface = config_json["interfaces"][0]
public_interface = config_json["interfaces"][1]

assert vpc_interface["primary"]
assert vpc_interface["purpose"] == "vpc"
assert vpc_interface["subnet_id"] == vpc_json["subnets"][0]["id"]
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 not public_interface["primary"]
assert public_interface["purpose"] == "public"
35 changes: 6 additions & 29 deletions tests/integration/vpc/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,17 @@

import pytest

from tests.integration.conftest import get_regions_with_capabilities
from tests.integration.conftest import (
create_vpc_w_subnet,
get_regions_with_capabilities,
)
from tests.integration.helpers import delete_target_id, exec_test_command


@pytest.fixture
def test_vpc_w_subnet():
region = get_regions_with_capabilities(["VPCs"])[0]

vpc_label = str(time.time_ns()) + "label"

subnet_label = str(time.time_ns()) + "label"

vpc_id = (
exec_test_command(
[
"linode-cli",
"vpcs",
"create",
"--label",
vpc_label,
"--region",
region,
"--subnets.ipv4",
"10.0.0.0/24",
"--subnets.label",
subnet_label,
"--no-headers",
"--text",
"--format=id",
]
)
.stdout.decode()
.rstrip()
)
vpc_json = create_vpc_w_subnet()
vpc_id = str(vpc_json["id"])

yield vpc_id

Expand Down
13 changes: 10 additions & 3 deletions tests/unit/test_operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,22 +162,29 @@ def test_parse_args_nullable_float(self, create_operation):
def test_parse_args_object_list(self, create_operation):
result = create_operation.parse_args(
[
# First object
"--object_list.field_string",
"test1",
"--object_list.field_int",
"123",
"--object_list.field_dict.nested_string",
"test2",
"--object_list.field_dict.nested_int",
"789",
# Second object
"--object_list.field_int",
"456",
"--object_list.field_dict.nested_string",
"test3",
]
)
assert result.object_list == [
{
"field_string": "test1",
"field_int": 123,
"field_dict": {"nested_string": "test2", "nested_int": 789},
},
{
"field_int": 456,
},
{"field_int": 456, "field_dict": {"nested_string": "test3"}},
]

def test_array_arg_action_basic(self):
Expand Down

0 comments on commit ede396a

Please sign in to comment.