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.

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(general): Used jsonpath to update vertex attributes #6852

Merged
merged 16 commits into from
Nov 19, 2024

Conversation

bo156
Copy link
Contributor

@bo156 bo156 commented Nov 18, 2024

User description

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Description

This PR fixes an inner method of updating attributes in vertices for all frameworks using graphs (like cloudformation and terraform). There was an issue in which we could override certain complex attributes mistakenly when setting the rendered value, as we would replace the entire attribute value instead of just the specific part of it. This PR adds jsonpath usage to find the correct location inside the attribute and replace only it.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my feature, policy, or fix is effective and works
  • New and existing tests pass locally with my changes


Generated description

Below is a concise technical summary of the changes proposed in this PR:

This pull request addresses a bug in the method for updating vertex attributes across various frameworks, such as CloudFormation and Terraform, by implementing jsonpath to accurately target and update specific parts of complex attributes. The CloudformationBlock class now includes a method _handle_unique_key_characters to manage unique key characters like Fn::If. Additionally, the Block class has been enhanced with a caching mechanism for parsed JSONPath statements to improve performance. The CloudformationVariableRenderer class has been updated to handle conditions more robustly. New tests have been added to ensure the correctness of these changes, including a performance test adjustment and a new YAML test case for complex JSONPath conditions.

TopicDetails
Vertex Attribute Update Implement jsonpath to update specific parts of complex attributes in vertex updates, preventing unintended overwrites.
Modified files (2)
  • checkov/cloudformation/graph_builder/graph_components/blocks.py
  • checkov/common/graph/graph_builder/graph_components/blocks.py
Latest Contributors(2)
UserCommitDate
taviassaffeat-azure-add-new-pol...November 17, 2024
gruebelchore-replace-policyun...October 23, 2023
Condition Handling Enhance condition handling in CloudformationVariableRenderer to support complex JSONPath conditions.
Modified files (1)
  • checkov/cloudformation/graph_builder/variable_rendering/renderer.py
Latest Contributors(2)
UserCommitDate
bo156feat-cloudformation-Su...November 14, 2024
gruebelbreak-general-remove-P...October 04, 2023
Performance Test Adjustment Adjust performance test thresholds to accommodate changes.
Modified files (1)
  • performance_tests/test_checkov_performance.py
Latest Contributors(2)
UserCommitDate
tomerlevi1983@gmail.comfix-terraform-for_each...May 30, 2024
gruebelfix-terraform-Remove-D...August 06, 2023
Testing Enhancements Add tests to verify the new JSONPath-based attribute update logic and condition handling.
Modified files (3)
  • tests/cloudformation/graph/graph_runner/test_running_graph_checks.py
  • tests/cloudformation/graph/graph_runner/resources/complex_jsonpath_if_condition/example.yaml
  • tests/cloudformation/graph/graph_runner/external_graph_checks/complex_jsonpath_if_condition.yaml
Latest Contributors(2)
UserCommitDate
bo156feat-cloudformation-Su...November 14, 2024
nimrodkor@gmail.comMove-test-yamls-for-k8...January 24, 2022
This pull request is reviewed by Baz. Join @bo156 and the rest of your team on (Baz).

@bo156 bo156 force-pushed the bugfix/cloudformation-if-evaluation-graph branch from 4c2b48c to 85c5c8e Compare November 18, 2024 14:00
@bo156 bo156 force-pushed the bugfix/cloudformation-if-evaluation-graph branch from 9f1f030 to 75b7788 Compare November 18, 2024 14:44
@bo156 bo156 changed the title Bugfix/cloudformation if evaluation graph fix(general): Used jsonpath to update vertex attributes Nov 18, 2024
@bo156 bo156 force-pushed the bugfix/cloudformation-if-evaluation-graph branch from 516009f to 496a202 Compare November 19, 2024 07:46
@bo156 bo156 marked this pull request as ready for review November 19, 2024 08:53
@bo156 bo156 merged commit 6a59caf into main Nov 19, 2024
43 checks passed
@bo156 bo156 deleted the bugfix/cloudformation-if-evaluation-graph branch November 19, 2024 09:55
Saarett pushed a commit that referenced this pull request Nov 19, 2024
* Added calculation of Fn::Sub in case of using Pseudo Parameter as it is a default one from cloudformation and shouldn't exist in the code

* Only do sub in case of pseudo parameter usage

* added example

* Middle of debugging, currently works

* Refactored update attribtue code to a method for jsonpath specifically

* Created test based on example

* Do not return jsonpath key from inner function

* Check key in attribtues before accessing it

* Removed mistakenly changed calculation of end vertices

* Removed tmp example files

* Added try-except to handle errors when parsing the jsonpath attribute

* Removed re module usage to improve performance

* Implemented cache for jsonpath to improve performance

* Updated threshold on linux machine performance from 10 to 11

* Removed bad comment:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants