Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: use relative path when resolving nested stack location #4871

Merged
merged 6 commits into from Mar 22, 2023
Merged

fix: use relative path when resolving nested stack location #4871

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
30410c3
fix: use relative path when resolving nested stack location
mndeveci Mar 15, 2023
e95a8a2
add tests
mndeveci Mar 16, 2023
577ec98
Merge branch 'develop' into fix_nested_stack_sis
mndeveci Mar 16, 2023
54f976b
update typing and format
mndeveci Mar 16, 2023
0c05335
use os specific path seperators
mndeveci Mar 16, 2023
cac36bb
Merge branch 'develop' into fix_nested_stack_sis
mndeveci Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 23 additions & 9 deletions samcli/lib/sync/infra_sync_executor.py
View file
Open in desktop
Expand Up @@ -5,7 +5,8 @@
import logging
import re
from datetime import datetime
from typing import Dict, Optional, Set
from pathlib import Path
from typing import TYPE_CHECKING, Dict, Optional, Set

from boto3 import Session
from botocore.exceptions import ClientError
Expand All @@ -14,7 +15,6 @@
from samcli.commands.build.build_context import BuildContext
from samcli.commands.deploy.deploy_context import DeployContext
from samcli.commands.package.package_context import PackageContext
from samcli.commands.sync.sync_context import SyncContext
from samcli.lib.providers.provider import ResourceIdentifier
from samcli.lib.providers.sam_stack_provider import is_local_path
from samcli.lib.utils.boto_utils import get_boto_client_provider_from_session_with_config
Expand All @@ -36,6 +36,9 @@
)
from samcli.yamlhelper import yaml_parse

if TYPE_CHECKING: # pragma: no cover
from samcli.commands.sync.sync_context import SyncContext

LOG = logging.getLogger(__name__)

GENERAL_REMOVAL_MAP = {
Expand Down Expand Up @@ -106,7 +109,7 @@ def __init__(
build_context: BuildContext,
package_context: PackageContext,
deploy_context: DeployContext,
sync_context: SyncContext,
sync_context: "SyncContext",
):
"""Constructs the sync for infra executor.

Expand Down Expand Up @@ -309,12 +312,17 @@ def _auto_skip_infra_sync(
if isinstance(template_location, dict):
continue
# For other scenarios, template location will be a string (local or s3 URL)
elif not self._auto_skip_infra_sync(
resource_dict.get("Properties", {}).get(template_field),
nested_template_location = (
current_built_template.get("Resources", {})
.get(resource_logical_id, {})
.get("Properties", {})
.get(template_field),
.get(template_field)
)
if is_local_path(nested_template_location):
nested_template_location = str(Path(built_template_path).parent.joinpath(nested_template_location))
if not self._auto_skip_infra_sync(
resource_dict.get("Properties", {}).get(template_field),
nested_template_location,
stack_resource_detail.get("StackResourceDetail", {}).get("PhysicalResourceId", ""),
nested_prefix + resource_logical_id + "/" if nested_prefix else resource_logical_id + "/",
):
Expand All @@ -324,7 +332,10 @@ def _auto_skip_infra_sync(
return True

def _sanitize_template(
self, template_dict: Dict, linked_resources: Set[str] = set(), built_template_dict: Optional[Dict] = None
self,
template_dict: Dict,
linked_resources: Optional[Set[str]] = None,
built_template_dict: Optional[Dict] = None,
) -> Set[str]:
"""
Fields skipped during template comparison because sync --code can handle the difference:
Expand Down Expand Up @@ -360,9 +371,11 @@ def _sanitize_template(
Set[str]
The list of resource IDs that got changed during sanitization
"""
linked_resources = linked_resources or set()

resources = template_dict.get("Resources", {})
processed_resources: Set[str] = set()
built_resource_dict = None

for resource_logical_id in resources:
resource_dict = resources.get(resource_logical_id, {})
Expand Down Expand Up @@ -399,7 +412,7 @@ def _remove_resource_field(
resource_logical_id: str,
resource_type: str,
resource_dict: Dict,
linked_resources: Set[str] = set(),
linked_resources: Optional[Set[str]] = None,
built_resource_dict: Optional[Dict] = None,
) -> Optional[str]:
"""
Expand All @@ -413,7 +426,7 @@ def _remove_resource_field(
Resource type
resource_dict: Dict
The resource level dict containing Properties field
linked_resources: Set[str]
linked_resources: Optional[Set[str]]
The corresponding resources in the other template that got processed
built_resource_dict: Optional[Dict]
Only passed in for current template sanitization to determine if local
Expand All @@ -423,6 +436,7 @@ def _remove_resource_field(
Optional[str]
The processed resource ID
"""
linked_resources = linked_resources or set()
processed_logical_id = None

if resource_type == AWS_LAMBDA_FUNCTION:
Expand Down
45 changes: 32 additions & 13 deletions tests/unit/lib/sync/test_infra_sync_executor.py
View file
Open in desktop
@@ -1,5 +1,6 @@
from pathlib import Path
from unittest import TestCase
from unittest.mock import MagicMock, patch
from unittest.mock import MagicMock, patch, call
from samcli.lib.providers.provider import ResourceIdentifier
from samcli.lib.sync.infra_sync_executor import datetime, InfraSyncExecutor
from botocore.exceptions import ClientError
Expand Down Expand Up @@ -287,7 +288,10 @@ def test_auto_skip_infra_sync_all_resources(self, session_mock, get_template_moc
def test_auto_skip_infra_sync_nested_stack(self, session_mock, get_template_mock, local_path_mock):
built_template_dict = {
"Resources": {
"ServerlessApplication": {"Type": "AWS::Serverless::Application", "Properties": {"Location": "local/"}},
"ServerlessApplication": {
"Type": "AWS::Serverless::Application",
"Properties": {"Location": str(Path("local") / "template.yaml")},
},
}
}

Expand All @@ -302,7 +306,7 @@ def test_auto_skip_infra_sync_nested_stack(self, session_mock, get_template_mock

built_nested_dict = {
"Resources": {
"ServerlessFunction": {"Type": "AWS::Serverless::Function", "Properties": {"CodeUri": "local/"}}
"ServerlessFunction": {"Type": "AWS::Serverless::Function", "Properties": {"CodeUri": "function/"}}
}
}

Expand All @@ -320,16 +324,20 @@ def test_auto_skip_infra_sync_nested_stack(self, session_mock, get_template_mock
)
infra_sync_executor._cfn_client.get_template.side_effect = [
{
"TemplateBody": """{
"Resources": {
"ServerlessApplication": {"Type": "AWS::Serverless::Application", "Properties": {"Location": "local/"}}
}
}"""
"TemplateBody": f"""{{
"Resources": {{
"ServerlessApplication": {{
"Type": "AWS::Serverless::Application",
"Properties": {{"Location": "{str(Path("local") / "template.yaml")}"}} }}
}}
}}"""
},
{
"TemplateBody": """{
"Resources": {
"ServerlessFunction": {"Type": "AWS::Serverless::Function", "Properties": {"CodeUri": "local/"}}
"ServerlessFunction": {
"Type": "AWS::Serverless::Function",
"Properties": {"CodeUri": "function/"}}
}
}"""
},
Expand All @@ -342,7 +350,20 @@ def test_auto_skip_infra_sync_nested_stack(self, session_mock, get_template_mock
with patch("botocore.response.StreamingBody") as stream_mock:
stream_mock.read.return_value = packaged_nested_dict.encode("utf-8")
infra_sync_executor._s3_client.get_object.return_value = {"Body": stream_mock}
self.assertTrue(infra_sync_executor._auto_skip_infra_sync("path", "path", "stack_name"))
self.assertTrue(
infra_sync_executor._auto_skip_infra_sync(
str(Path("path") / "packaged-template.yaml"),
str(Path("path") / "built-template.yaml"),
"stack_name",
)
)
get_template_mock.assert_has_calls(
[
call(str(Path("path") / "packaged-template.yaml")),
call(str(Path("path") / "built-template.yaml")),
call(str(Path("path") / "local/template.yaml")),
]
)
self.assertEqual(
infra_sync_executor.code_sync_resources,
{ResourceIdentifier("ServerlessApplication/ServerlessFunction")},
Expand Down Expand Up @@ -399,10 +420,9 @@ def test_auto_skip_infra_sync_nested_stack_with_sar(
self.assertEqual(infra_sync_executor._auto_skip_infra_sync("path", "path2", "stack_name"), expected_result)
self.assertEqual(infra_sync_executor.code_sync_resources, set())

@patch("samcli.lib.sync.infra_sync_executor.is_local_path")
@patch("samcli.lib.sync.infra_sync_executor.get_template_data")
@patch("samcli.lib.sync.infra_sync_executor.Session")
def test_auto_skip_infra_sync_http_template_location(self, session_mock, get_template_mock, local_path_mock):
def test_auto_skip_infra_sync_http_template_location(self, session_mock, get_template_mock):
built_template_dict = {
"Resources": {
"NestedStack": {
Expand All @@ -428,7 +448,6 @@ def test_auto_skip_infra_sync_http_template_location(self, session_mock, get_tem
}"""

get_template_mock.side_effect = [packaged_template_dict, built_template_dict]
local_path_mock.return_value = True

infra_sync_executor = InfraSyncExecutor(
self.build_context, self.package_context, self.deploy_context, self.sync_context
Expand Down