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

fix: copy resource metadata before normalizing the information in it #5904

Merged
merged 4 commits into from
Sep 14, 2023
Merged

fix: copy resource metadata before normalizing the information in it #5904

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
45bc733
fix: copy resource metadata before normalizing the information in it
mndeveci Sep 7, 2023
ae8e64e
add tests
mndeveci Sep 8, 2023
88b2df2
Merge branch 'develop' into issue_5901
mndeveci Sep 8, 2023
9cc5748
remove redundant function & update test method name
mndeveci Sep 8, 2023
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
8 changes: 4 additions & 4 deletions samcli/lib/samlib/resource_metadata_normalizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import json
import logging
import re
from copy import deepcopy
from pathlib import Path
from typing import Dict

Expand Down Expand Up @@ -61,10 +62,8 @@ def normalize(template_dict, normalize_parameters=False):
resources = template_dict.get(RESOURCES_KEY, {})

for logical_id, resource in resources.items():
resource_metadata = resource.get(METADATA_KEY)
if resource_metadata is None:
resource_metadata = {}
resource[METADATA_KEY] = resource_metadata
# copy metadata to another variable, change its values and assign it back in the end
resource_metadata = deepcopy(resource.get(METADATA_KEY)) or {}

is_normalized = resource_metadata.get(SAM_IS_NORMALIZED, False)
if not is_normalized:
Expand Down Expand Up @@ -97,6 +96,7 @@ def normalize(template_dict, normalize_parameters=False):
resource_metadata,
{SAM_RESOURCE_ID_KEY: ResourceMetadataNormalizer.get_resource_id(resource, logical_id)},
)
resource[METADATA_KEY] = resource_metadata

# This is a work around to allow the customer to use sam deploy or package commands without the need to provide
# values for the CDK auto generated asset parameters. The suggested solution is to let CDK add some metadata to
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/lib/samlib/test_resource_metadata_normalizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from unittest import TestCase

from samcli.lib.samlib.resource_metadata_normalizer import ResourceMetadataNormalizer
from samcli.yamlhelper import yaml_parse


class TestResourceMetadataNormalizer(TestCase):
Expand Down Expand Up @@ -456,6 +457,53 @@ def test_skip_normalizing_already_normalized_resource(self):
self.assertEqual("new path", template_data["Resources"]["Function1"]["Properties"]["Code"])
self.assertEqual("Function1", template_data["Resources"]["Function1"]["Metadata"]["SamResourceId"])

def test_referenced_data(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are we asserting based on the new changes?

Copy link
Contributor Author

@mndeveci mndeveci Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Before this change, SamResourceId is assigned as FirstFunction for all resources due to the reason I explained in the PR desc.

After this change, all resources must have their logical id assigned into SamResourceId, we are asserting them at the end of this changed block.

There is a redundant function that I forgot to remove, please ignore that, will be removing it now.

input_template = """
Resources:
FirstFunction:
Type: AWS::Serverless::Function
Metadata: &GoFunctionMetadata
BuildMethod: go1.x
BuildProperties:
TrimGoPath: true
Properties:
CodeUri: dummy
Handler: bootstrap
Runtime: provided.al2
SecondFunction:
Type: AWS::Serverless::Function
Metadata: *GoFunctionMetadata
Properties:
CodeUri: dummy
Handler: bootstrap
Runtime: provided.al2
ThirdFunction:
Type: AWS::Serverless::Function
Metadata: *GoFunctionMetadata
Properties:
CodeUri: dummy
Handler: bootstrap
Runtime: provided.al2
"""
template_dict = yaml_parse(input_template)
ResourceMetadataNormalizer.normalize(template_dict)

def _extract_sam_resource_id(template, resource_id):
return template.get("Resources", {}).get(resource_id, {}).get("Metadata", {}).get("SamResourceId")

self.assertEqual(
template_dict.get("Resources", {}).get("FirstFunction", {}).get("Metadata", {}).get("SamResourceId"),
"FirstFunction",
)
self.assertEqual(
template_dict.get("Resources", {}).get("SecondFunction", {}).get("Metadata", {}).get("SamResourceId"),
"SecondFunction",
)
self.assertEqual(
template_dict.get("Resources", {}).get("ThirdFunction", {}).get("Metadata", {}).get("SamResourceId"),
"ThirdFunction",
)


class TestResourceMetadataNormalizerGetResourceId(TestCase):
@parameterized.expand(
Expand Down