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: fix failing Terraform integration test cases #5218

Merged
merged 4 commits into from
May 30, 2023
Merged

fix: fix failing Terraform integration test cases #5218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c3ed0e
fix: fix the failing terraform integration test cases
moelasmar May 29, 2023
e53f9f0
fix: fix the resource address while accessing the module config resou…
moelasmar May 29, 2023
00cba06
fix: fix checking the experimental log integration test cases
moelasmar May 29, 2023
84f6f81
Merge branch 'develop' into develop_fix_terraform_integration_testcases
moelasmar May 29, 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
10 changes: 8 additions & 2 deletions samcli/hook_packages/terraform/hooks/prepare/translate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,21 @@ def _check_unresolvable_values(root_module: dict, root_tf_module: TFModule) -> N
# iterate over resources for current module
for resource in curr_module.get("resources", []):
resource_type = resource.get("type")
resource_name = resource.get("name")
resource_mode = resource.get("mode")

resource_mapper = RESOURCE_TRANSLATOR_MAPPING.get(resource_type)
if not resource_mapper:
continue

resource_values = resource.get("values")
resource_full_address = resource.get("address")
Copy link
Contributor

Choose a reason for hiding this comment

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

We are not using address field anymore and generating its value with the information that we have? If so, do you think there might be some edge cases if they change the way to construct that field, should we keep using it and prepend data. for the case below?

Copy link
Contributor Author

@moelasmar moelasmar May 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the reason is this address contains the full address of the resource starting from the root module, but when we built the module configuration data structure we used the address in the module (which is <resource type>.<resource name> in case of resources, and data.<resource type>.<resource name> in case of data resource). This is because the same module configuration can be used multiple times to create actual modules

resource_address = (
f"data.{resource_type}.{resource_name}"
if resource_mode == "data"
else f"{resource_type}.{resource_name}"
)

config_resource_address = get_configuration_address(resource_full_address)
config_resource_address = get_configuration_address(resource_address)
config_resource = curr_tf_module.resources[config_resource_address]

for prop_builder in resource_mapper.property_builder_mapping.values():
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/local/invoke/test_invoke_terraform_applications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def test_invoke_function_get_experimental_prompted(self):
"You can also enable this beta feature with 'sam local invoke --beta-features'."
)
self.assertRegex(stdout.decode("utf-8"), terraform_beta_feature_prompted_text)
self.assertTrue(stderr.decode("utf-8").startswith(Colored().yellow(EXPERIMENTAL_WARNING)))
self.assertRegex(stderr.decode("utf-8"), EXPERIMENTAL_WARNING)

response = json.loads(stdout.decode("utf-8").split("\n")[2][85:].strip())
expected_response = json.loads('{"statusCode":200,"body":"{\\"message\\": \\"hello world\\"}"}')
Expand Down
1 change: 0 additions & 1 deletion ...ntegration/testdata/buildcmd/terraform/image_based_lambda_functions_local_backend/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
provider "aws" {
# Make it faster by skipping something
skip_get_ec2_platforms = true
skip_metadata_api_check = true
skip_region_validation = true
skip_credentials_validation = true
Expand Down
1 change: 0 additions & 1 deletion ...s/integration/testdata/buildcmd/terraform/image_based_lambda_functions_s3_backend/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ terraform {

provider "aws" {
# Make it faster by skipping something
skip_get_ec2_platforms = true
skip_metadata_api_check = true
skip_region_validation = true
skip_credentials_validation = true
Expand Down
8 changes: 6 additions & 2 deletions tests/unit/hook_packages/terraform/hooks/prepare/test_translate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1116,8 +1116,12 @@ class TestUnresolvableAttributeCheck:
@patch("samcli.hook_packages.terraform.hooks.prepare.translate.RESOURCE_TRANSLATOR_MAPPING")
@patch("samcli.hook_packages.terraform.hooks.prepare.translate.LOG")
def test_module_contains_unresolvables(self, log_mock, mapping_mock):
config_addr = "addr"
module = {"resources": [{"address": config_addr, "values": Mock()}]}
config_addr = "function.func1"
module = {
"resources": [
{"address": config_addr, "values": Mock(), "type": "function", "mode": "resource", "name": "func1"}
]
}

tf_module = Mock()
tf_module_attr = Mock()
Expand Down