fix(terraform): create graph edges for resources using count/for_each#7508
Open
pszypowicz wants to merge 4 commits intobridgecrewio:mainfrom
Open
fix(terraform): create graph edges for resources using count/for_each#7508pszypowicz wants to merge 4 commits intobridgecrewio:mainfrom
pszypowicz wants to merge 4 commits intobridgecrewio:mainfrom
Conversation
The graph edge builder failed to connect resources that use count or for_each because vertex names include an index suffix (e.g. bucket[0]) after expansion, but remove_index_pattern_from_str strips numeric indices from references during lookup -- causing a name mismatch. Add a sanitized-name fallback in _get_possible_vertices and set for_each_index on resource/data blocks to match the module handler.
pszypowicz
requested a deployment
to
scan-security
GitHub Actions
Waiting
Contributor
|
Hi @pszypowicz, thanks for the contribution! |
Contributor
|
Re-running the performance-tests that failed. If it won't pass again, please work to improve the performance of the solution. Please let us know if any assistance is needed. Thanks! |
The fallback in _get_possible_vertices iterated all vertices in a (module, block_type) bucket and sanitized each name on every miss, making edge-building quadratic on graphs with many expanded resources. Maintain a parallel dict keyed by sanitized (un-indexed) name, populated in _add_block_data_to_graph and reset alongside the primary map in _arrange_graph_data. The fallback becomes an O(1) lookup.
pszypowicz
requested a deployment
to
scan-security
GitHub Actions
Waiting
pszypowicz
requested a deployment
to
scan-security
GitHub Actions
Waiting
pszypowicz
requested a deployment
to
scan-security
GitHub Actions
Waiting
Author
|
@Saarett Thanks for feedback, fixes pushed, please let me know if I can do something else to help |
Author
|
@Saarett it looks like there was some other issue? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #7358
countorfor_eachbecause vertex names include an index suffix (e.g.bucket[0]) after expansion, butremove_index_pattern_from_strstrips numeric indices from references during lookup -- causing a name mismatch in_get_possible_vertices()_get_possible_vertices()using the existingget_sanitized_terraform_resource_id()utility, which matches indexed vertex names against the stripped reference namefor_each_indexon resource/data blocks inForeachEntityHandler._create_new_resource(), mirroring the existing behavior inForeachModuleHandler._create_new_module()-- this enables_find_best_match_based_on_foreach_key()to disambiguate between multiple indexed vertices (e.g.bucket[0]vsbucket[1])Test plan
count = 1pass/fail cases toS3BucketLogginggraph check testcount = 2case where both bucket and logging resource use count -- exercises multi-candidate disambiguation viafor_each_index