Skip to content

Conversation

@simonrw
Copy link
Contributor

@simonrw simonrw commented Oct 7, 2025

Motivation

When a template resource which is disabled by a condition references another resource that's disabled by a condition, the CFn engine currently incorrectly raises a ValidationError in the transformer, validator and executor. This is not in parity with AWS. See the references_to_conditions.yml template for an example.

Changes

  • Skip resources and outputs in visit_node_resources/visit_node_outputs entirely in the preproc so we don't even evaluate the resource block during template preprocessing time
  • Add test capturing this behaviour

@simonrw simonrw added docs: skip Pull request does not require documentation changes aws:cloudformation AWS CloudFormation notes: skip Pull request does not have to be mentioned in the release notes semver: patch Non-breaking changes which can be included in patch releases labels Oct 7, 2025
@github-actions
Copy link

github-actions bot commented Oct 7, 2025

Test Results - Preflight, Unit

22 292 tests  ±0   20 549 ✅ ±0   16m 11s ⏱️ +32s
     1 suites ±0    1 743 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit ebf3c17. ± Comparison against base commit b592fbd.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 36s ⏱️ +14s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit ebf3c17. ± Comparison against base commit b592fbd.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

Test Results - Alternative Providers

577 tests   330 ✅  26m 16s ⏱️
  1 suites  247 💤
  1 files      0 ❌

Results for commit ebf3c17.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 39m 36s ⏱️
5 178 tests 4 681 ✅ 497 💤 0 ❌
5 184 runs  4 681 ✅ 503 💤 0 ❌

Results for commit ebf3c17.

@simonrw simonrw marked this pull request as ready for review October 7, 2025 22:58
@github-actions
Copy link

github-actions bot commented Oct 7, 2025

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 59m 59s ⏱️ -48s
4 804 tests +2  4 467 ✅ +3  337 💤  - 1  0 ❌ ±0 
4 806 runs  +2  4 467 ✅ +3  339 💤  - 1  0 ❌ ±0 

Results for commit ebf3c17. ± Comparison against base commit b592fbd.

Copy link
Member

@pinzon pinzon left a comment

Choose a reason for hiding this comment

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

Great improvement.

Copy link
Member

Choose a reason for hiding this comment

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

Praise: Glad the new engine is easily modifiable.


@markers.aws.validated
@pytest.mark.skipif(condition=not is_aws_cloud(), reason="not supported yet")
@skip_if_legacy_engine()
Copy link
Member

Choose a reason for hiding this comment

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

🥳

Copy link
Member

Choose a reason for hiding this comment

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

Nit: Consider adding a validation test about a situation where an output references a conditionally skip resource.

Copy link
Contributor Author

@simonrw simonrw Oct 8, 2025

Choose a reason for hiding this comment

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

Good idea, thanks! I'll tackle this in a follow up

@simonrw simonrw merged commit 8096cdf into main Oct 8, 2025
57 of 60 checks passed
@simonrw simonrw deleted the cfn/fix-conditional-access branch October 8, 2025 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aws:cloudformation AWS CloudFormation docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants