-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
feat(terraform): Add provider address to resources #6266
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍻
@@ -139,6 +145,85 @@ def _add_block_data_to_graph(self, idx: int, block: TerraformBlock) -> None: | |||
self.in_edges[idx] = [] | |||
self.out_edges[idx] = [] | |||
|
|||
def _add_provider_attr_to_resources(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add documentation to this function? looks rather complex...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
return cast(str, module_providers[list(module_providers.keys())[0]].replace("$", "").replace("{", "").replace("}", "")) | ||
else: | ||
for p_address in provider_address: | ||
if m_alias.replace("$", "").replace("{", "").replace("}", "") == self.vertices[p_address].name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you extract those strings to consts? you use them multiple times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
def _get_resource_name_and_type_from_name(name: str) -> tuple[str, str]: | ||
resource_name, resource_type = '', '' | ||
split_name = name.split('.') | ||
if len(split_name) >= 2: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will not work if you have for example .
inside a foreach key.
Instead you can use this function get_sanitized_terraform_resource_id
(or a variation of it) from here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove, already implemented logic in another func
@@ -461,6 +462,7 @@ def parse_hcl_module_from_tf_definitions( | |||
) | |||
self.add_tfvars(module, source) | |||
copy_of_tf_definitions = pickle_deepcopy(tf_definitions) | |||
module.temp_tf_definition = tf_definitions # type:ignore # will be TFDefinitionKey and not string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why ignore instead of changing the type in line 51?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already tried to play with it a couple of times and it didn't work, (u can see at the first couple of commits).
provider_address_with_alias = local_graph.vertices[4].attributes.get('__address__') | ||
assert resource_provider_address_with_alias == provider_address_with_alias | ||
|
||
def test_provider_edge_cases(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great job with the tests!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😸
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Description
Checklist: