Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions samcli/commands/_utils/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@

from samcli.commands.exceptions import UserException
from samcli.lib.samlib.resource_metadata_normalizer import ASSET_PATH_METADATA_KEY, ResourceMetadataNormalizer
from samcli.lib.utils import graphql_api
from samcli.lib.utils.packagetype import IMAGE, ZIP
from samcli.lib.utils.resources import (
AWS_LAMBDA_FUNCTION,
AWS_SERVERLESS_FUNCTION,
AWS_SERVERLESS_GRAPHQLAPI,
METADATA_WITH_LOCAL_PATHS,
RESOURCES_WITH_LOCAL_PATHS,
get_packageable_resource_paths,
Expand Down Expand Up @@ -161,6 +163,18 @@ def _update_relative_paths(template_dict, original_root, new_root):
):
continue

# SAM GraphQLApi has many instances of CODE_ARTIFACT_PROPERTY and all of them must be updated
if resource_type == AWS_SERVERLESS_GRAPHQLAPI and path_prop_name == graphql_api.CODE_ARTIFACT_PROPERTY:
# to be able to set different nested properties to S3 uri, paths are necessary
# jmespath doesn't provide that functionality, thus custom implementation
paths_values = graphql_api.find_all_paths_and_values(path_prop_name, properties)
for property_path, property_value in paths_values:
updated_path = _resolve_relative_to(property_value, original_root, new_root)
if not updated_path:
# This path does not need to get updated
continue
set_value_from_jmespath(properties, property_path, updated_path)

path = jmespath.search(path_prop_name, properties)
updated_path = _resolve_relative_to(path, original_root, new_root)

Expand Down
40 changes: 5 additions & 35 deletions samcli/lib/package/packageable_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import logging
import os
import shutil
from typing import Any, Dict, List, Optional, Tuple, Union, cast
from typing import Dict, Optional, Union, cast

import jmespath
from botocore.utils import set_value_from_jmespath
Expand All @@ -24,6 +24,7 @@
upload_local_artifacts,
upload_local_image_artifacts,
)
from samcli.lib.utils import graphql_api
from samcli.lib.utils.packagetype import IMAGE, ZIP
from samcli.lib.utils.resources import (
AWS_APIGATEWAY_RESTAPI,
Expand Down Expand Up @@ -599,7 +600,7 @@ def get_property_value(self, resource_dict):

class GraphQLApiSchemaResource(ResourceZip):
RESOURCE_TYPE = AWS_SERVERLESS_GRAPHQLAPI
PROPERTY_NAME = RESOURCES_WITH_LOCAL_PATHS[RESOURCE_TYPE][0]
PROPERTY_NAME = graphql_api.SCHEMA_ARTIFACT_PROPERTY
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we import anything we need from graphql_api directly instead of importing the module?

Copy link
Contributor Author

@ssenchenko ssenchenko Jun 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the style I used is against SAM CLI practice? I import everything from that file anyway. But the main reason, I think that graphql_api.SCHEMA_ARTIFACT_PROPERTY and graphql_api.find_all_paths_and_values(self.PROPERTY_NAME, resource_dict) is easier to read than simply SCHEMA_ARTIFACT_PROPERTY and find_all_paths_and_values which show less "belonging". But if it's against the style, I can remove it. It's up to you

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine since graphql_api is not a huge module. And +1 for the point of "belonging".

# Don't package the directory if SchemaUri is omitted.
# Necessary to support SchemaInline
PACKAGE_NULL_PROPERTY = False
Expand Down Expand Up @@ -635,7 +636,7 @@ class GraphQLApiCodeResource(ResourceZip):
"""

RESOURCE_TYPE = AWS_SERVERLESS_GRAPHQLAPI
PROPERTY_NAME = RESOURCES_WITH_LOCAL_PATHS[RESOURCE_TYPE][1]
PROPERTY_NAME = graphql_api.CODE_ARTIFACT_PROPERTY
# if CodeUri is omitted the directory is not packaged because it's necessary to support CodeInline
PACKAGE_NULL_PROPERTY = False

Expand All @@ -648,7 +649,7 @@ def export(self, resource_id: str, resource_dict: Optional[Dict], parent_dir: st

# to be able to set different nested properties to S3 uri, paths are necessary
# jmespath doesn't provide that functionality, thus custom implementation
paths_values = self._find_all_with_property_name(resource_dict)
paths_values = graphql_api.find_all_paths_and_values(self.PROPERTY_NAME, resource_dict)
for property_path, property_value in paths_values:
if isinstance(property_value, dict):
LOG.debug("Property %s of %s resource is not a URL", self.PROPERTY_NAME, resource_id)
Expand All @@ -675,37 +676,6 @@ def export(self, resource_id: str, resource_dict: Optional[Dict], parent_dir: st
if temp_dir:
shutil.rmtree(temp_dir)

def _find_all_with_property_name(self, graphql_dict: Dict[str, Any]) -> List[Tuple[str, Union[str, Dict]]]:
"""Find paths to the all properties with self.PROPERTY_NAME name and their (properties) values.

It leverages the knowledge of GraphQLApi structure instead of doing generic search in the graph.

Parameters
----------
graphql_dict
GraphQLApi resource dict

Returns
-------
list of tuple (path, value) for all found properties which has property_name
"""
# need to look up only in "Resolvers" and "Functions" subtrees
resolvers_and_functions = {k: graphql_dict[k] for k in ("Resolvers", "Functions") if k in graphql_dict}
stack: List[Tuple[Dict[str, Any], str]] = [(resolvers_and_functions, "")]
paths_values: List[Tuple[str, Union[str, Dict]]] = []

while stack:
node, path = stack.pop()
if isinstance(node, dict):
for key, value in node.items():
if key == self.PROPERTY_NAME:
paths_values.append((f"{path}{key}", value))
elif isinstance(value, dict):
stack.append((value, f"{path}{key}."))
# there is no need to handle lists because
# paths to "CodeUri" within "Resolvers" and "Functions" doesn't have lists
return paths_values


RESOURCES_EXPORT_LIST = [
ServerlessFunctionResource,
Expand Down
41 changes: 41 additions & 0 deletions samcli/lib/utils/graphql_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""Helper functions to work with SAM GraphQLApi resource
"""

from typing import Any, Dict, List, Tuple, Union

SCHEMA_ARTIFACT_PROPERTY = "SchemaUri"
CODE_ARTIFACT_PROPERTY = "CodeUri"


def find_all_paths_and_values(property_name: str, graphql_dict: Dict[str, Any]) -> List[Tuple[str, Union[str, Dict]]]:
"""Find paths to the all properties with property_name and their (properties) values.

It leverages the knowledge of GraphQLApi structure instead of doing generic search in the graph.

Parameters
----------
property_name
Name of the property to look up, for example 'CodeUri'
graphql_dict
GraphQLApi resource dict

Returns
-------
list of tuple (path, value) for all found properties which has property_name
"""
# need to look up only in "Resolvers" and "Functions" subtrees
resolvers_and_functions = {k: graphql_dict[k] for k in ("Resolvers", "Functions") if k in graphql_dict}
stack: List[Tuple[Dict[str, Any], str]] = [(resolvers_and_functions, "")]
paths_values: List[Tuple[str, Union[str, Dict]]] = []

while stack:
node, path = stack.pop()
if isinstance(node, dict):
for key, value in node.items():
if key == property_name:
paths_values.append((f"{path}{key}", value))
elif isinstance(value, dict):
stack.append((value, f"{path}{key}."))
# there is no need to handle lists because
# paths to "CodeUri" within "Resolvers" and "Functions" doesn't have lists
return paths_values
4 changes: 3 additions & 1 deletion samcli/lib/utils/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

from collections import defaultdict

from samcli.lib.utils.graphql_api import CODE_ARTIFACT_PROPERTY, SCHEMA_ARTIFACT_PROPERTY

# Lambda
AWS_SERVERLESS_FUNCTION = "AWS::Serverless::Function"
AWS_SERVERLESS_LAYERVERSION = "AWS::Serverless::LayerVersion"
Expand Down Expand Up @@ -62,7 +64,7 @@
METADATA_WITH_LOCAL_PATHS = {AWS_SERVERLESSREPO_APPLICATION: ["LicenseUrl", "ReadmeUrl"]}

RESOURCES_WITH_LOCAL_PATHS = {
AWS_SERVERLESS_GRAPHQLAPI: ["SchemaUri", "CodeUri"],
AWS_SERVERLESS_GRAPHQLAPI: [SCHEMA_ARTIFACT_PROPERTY, CODE_ARTIFACT_PROPERTY],
AWS_SERVERLESS_FUNCTION: ["CodeUri"],
AWS_SERVERLESS_API: ["DefinitionUri"],
AWS_SERVERLESS_HTTPAPI: ["DefinitionUri"],
Expand Down
157 changes: 101 additions & 56 deletions tests/unit/commands/_utils/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,14 @@
import yaml
from botocore.utils import set_value_from_jmespath
from parameterized import parameterized, param
from samcli.lib.utils.graphql_api import CODE_ARTIFACT_PROPERTY, find_all_paths_and_values

from samcli.lib.utils.resources import AWS_SERVERLESS_FUNCTION, AWS_SERVERLESS_API, RESOURCES_WITH_IMAGE_COMPONENT
from samcli.lib.utils.resources import (
AWS_SERVERLESS_FUNCTION,
AWS_SERVERLESS_API,
AWS_SERVERLESS_GRAPHQLAPI,
RESOURCES_WITH_IMAGE_COMPONENT,
)
from samcli.commands._utils.template import (
get_template_data,
METADATA_WITH_LOCAL_PATHS,
Expand Down Expand Up @@ -215,43 +221,28 @@ def test_must_update_relative_metadata_paths(self, resource_type, properties):
@parameterized.expand([(resource_type, props) for resource_type, props in RESOURCES_WITH_LOCAL_PATHS.items()])
def test_must_update_relative_resource_paths(self, resource_type, properties):
for propname in properties:
template_dict = {
"Resources": {
"MyResourceWithRelativePath": {"Type": resource_type, "Properties": {}},
"MyResourceWithS3Path": {"Type": resource_type, "Properties": {propname: self.s3path}},
"MyResourceWithAbsolutePath": {"Type": resource_type, "Properties": {propname: self.abspath}},
"MyResourceWithInvalidPath": {
"Type": resource_type,
"Properties": {
# Path is not a string
propname: {"foo": "bar"}
},
},
"MyResourceWithoutProperties": {"Type": resource_type},
"UnsupportedResourceType": {"Type": "AWS::Ec2::Instance", "Properties": {"Code": "bar"}},
"ResourceWithoutType": {"foo": "bar"},
},
"Parameters": {"a": "b"},
}
template_dict = self._generate_template(resource_type, propname)

set_value_from_jmespath(
template_dict, f"Resources.MyResourceWithRelativePath.Properties.{propname}", self.curpath
)
self._set_property(self.curpath, propname, template_dict, resource_type, "MyResourceWithRelativePath")

expected_template_dict = copy.deepcopy(template_dict)

set_value_from_jmespath(
expected_template_dict,
f"Resources.MyResourceWithRelativePath.Properties.{propname}",
self.expected_result,
self._set_property(
self.expected_result, propname, expected_template_dict, resource_type, "MyResourceWithRelativePath"
)

result = _update_relative_paths(template_dict, self.src, self.dest)

self.maxDiff = None
self.assertEqual(result, expected_template_dict)

@parameterized.expand([(resource_type, props) for resource_type, props in RESOURCES_WITH_LOCAL_PATHS.items()])
@parameterized.expand(
[
(resource_type, props)
for resource_type, props in RESOURCES_WITH_LOCAL_PATHS.items()
if resource_type != AWS_SERVERLESS_GRAPHQLAPI # Metadata path to code artifacts is not supported
]
)
def test_must_update_relative_resource_metadata_paths(self, resource_type, properties):
for propname in properties:
template_dict = {
Expand Down Expand Up @@ -336,34 +327,18 @@ def test_must_skip_only_image_components_and_update_relative_resource_paths(
):
for non_image_propname in non_image_properties:
for image_propname in image_properties:
template_dict = {
"Resources": {
"MyResourceWithRelativePath": {"Type": non_image_resource_type, "Properties": {}},
"MyResourceWithS3Path": {
"Type": non_image_resource_type,
"Properties": {non_image_propname: self.s3path},
},
"MyResourceWithAbsolutePath": {
"Type": non_image_resource_type,
"Properties": {non_image_propname: self.abspath},
},
"MyResourceWithInvalidPath": {
"Type": non_image_resource_type,
"Properties": {
# Path is not a string
non_image_propname: {"foo": "bar"}
},
},
"MyResourceWithoutProperties": {"Type": non_image_resource_type},
"UnsupportedResourceType": {"Type": "AWS::Ec2::Instance", "Properties": {"Code": "bar"}},
"ResourceWithoutType": {"foo": "bar"},
"ImageResource": {"Type": image_resource_type, "Properties": {"PackageType": "Image"}},
},
"Parameters": {"a": "b"},
template_dict = self._generate_template(non_image_resource_type, non_image_resource_type)
template_dict["Resources"]["ImageResource"] = {
"Type": image_resource_type,
"Properties": {"PackageType": "Image"},
}

set_value_from_jmespath(
template_dict, f"Resources.MyResourceWithRelativePath.Properties.{non_image_propname}", self.curpath
self._set_property(
self.curpath,
non_image_propname,
template_dict,
non_image_resource_type,
"MyResourceWithRelativePath",
)

set_value_from_jmespath(
Expand All @@ -372,10 +347,12 @@ def test_must_skip_only_image_components_and_update_relative_resource_paths(

expected_template_dict = copy.deepcopy(template_dict)

set_value_from_jmespath(
expected_template_dict,
f"Resources.MyResourceWithRelativePath.Properties.{non_image_propname}",
self._set_property(
self.expected_result,
non_image_propname,
expected_template_dict,
non_image_resource_type,
"MyResourceWithRelativePath",
)

result = _update_relative_paths(template_dict, self.src, self.dest)
Expand Down Expand Up @@ -420,6 +397,74 @@ def test_must_update_aws_include_also(self):
self.maxDiff = None
self.assertEqual(result, expected_template_dict)

def _generate_template(self, resource_type, property_name):
template = {
"Resources": {
"MyResourceWithRelativePath": {"Type": resource_type, "Properties": {}},
"MyResourceWithS3Path": {"Type": resource_type, "Properties": {}},
"MyResourceWithAbsolutePath": {"Type": resource_type, "Properties": {}},
"MyResourceWithInvalidPath": {
"Type": resource_type,
"Properties": {},
},
"MyResourceWithoutProperties": {"Type": resource_type},
"UnsupportedResourceType": {"Type": "AWS::Ec2::Instance", "Properties": {"Code": "bar"}},
"ResourceWithoutType": {"foo": "bar"},
},
"Parameters": {"a": "b"},
}
if self._is_graphql_code_uri(resource_type, property_name):
template["Resources"]["MyResourceWithRelativePath"]["Properties"] = self._generate_graphql_props(
property_name
)
template["Resources"]["MyResourceWithS3Path"]["Properties"] = self._generate_graphql_props(
property_name, self.s3path
)
template["Resources"]["MyResourceWithAbsolutePath"]["Properties"] = self._generate_graphql_props(
property_name, self.abspath
)
template["Resources"]["MyResourceWithInvalidPath"]["Properties"] = self._generate_graphql_props(
property_name, {"foo": "bar"}
)
else:
template["Resources"]["MyResourceWithS3Path"]["Properties"] = {property_name: self.s3path}
template["Resources"]["MyResourceWithAbsolutePath"]["Properties"] = {property_name: self.abspath}
template["Resources"]["MyResourceWithInvalidPath"]["Properties"] = {property_name: {"foo": "bar"}}
return template

@staticmethod
def _generate_graphql_props(property_name, path=None):
if path is not None:
return {
"Functions": {"Func1": {property_name: path}, "Func2": {property_name: path}},
"Resolvers": {"Mutation": {"Resolver1": {property_name: path}}},
}
return {
"Functions": {"Func1": {}, "Func2": {}},
"Resolvers": {"Mutation": {"Resolver1": {}}},
}

def _set_property(self, value, property_name, template, tested_type, resource_name):
if self._is_graphql_code_uri(tested_type, property_name):
resource_dict = template["Resources"][resource_name]
paths_values = find_all_paths_and_values(property_name, resource_dict)
for property_path, _ in paths_values:
set_value_from_jmespath(template, f"Resources.{resource_name}.{property_path}", value)
else:
set_value_from_jmespath(template, f"Resources.{resource_name}.Properties.{property_name}", value)

@staticmethod
def _is_graphql_code_uri(resource_type, property_name):
return resource_type == AWS_SERVERLESS_GRAPHQLAPI and property_name == CODE_ARTIFACT_PROPERTY

def _assert_templates_are_equal(self, actual, expected, tested_type, property_name):
if self._is_graphql_code_uri(tested_type, property_name):
actual_paths_values = find_all_paths_and_values(property_name, actual)
expepcted_paths_values = find_all_paths_and_values(property_name, expected)
self.assertListEqual(actual_paths_values, expepcted_paths_values)
else:
self.assertEqual(actual, expected)


class Test_resolve_relative_to(TestCase):
def setUp(self):
Expand Down
Loading