Skip to content

CFNV2: handle AWS::NoValue #13000

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

Jump to bottom
Merged
merged 18 commits into from
Aug 15, 2025
Merged

CFNV2: handle AWS::NoValue #13000

merged 18 commits into from
Aug 15, 2025

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Aug 12, 2025

Motivation

We currently model the AWS::NoValue intrinsic value by returning None, however this is not the right approach. We already perform lots of filtering in e.g. visit_node_properties for Nothing values, and AWS::NoValue has a similar place as returning Nothing.

Changes

  • Shortcut the Ref resolving if the arguments are AWS::NoValue and return Nothing in its place
  • Add validated test specifically ensuring the correct behaviour (passes on v1 and v2)

@simonrw simonrw added this to the 4.8 milestone Aug 12, 2025
@simonrw simonrw added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Aug 12, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 12, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 107 tests  ±0   20 372 ✅ ±0   6m 20s ⏱️ -5s
     1 suites ±0    1 735 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 1fe4b7c. ± Comparison against base commit 9c7fd42.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 12, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 13s ⏱️ -4s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 1fe4b7c. ± Comparison against base commit 9c7fd42.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 12, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files  ±    0      5 suites  ±0   2h 23m 40s ⏱️ + 1h 50m 5s
4 989 tests +4 410  4 397 ✅ +4 042  592 💤 +368  0 ❌ ±0 
4 995 runs  +4 410  4 397 ✅ +4 042  598 💤 +368  0 ❌ ±0 

Results for commit 1fe4b7c. ± Comparison against base commit 9c7fd42.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 12, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 43m 18s ⏱️ + 1h 21m 15s
4 630 tests +4 075  4 190 ✅ +3 860  440 💤 +215  0 ❌ ±0 
4 632 runs  +4 075  4 190 ✅ +3 860  442 💤 +215  0 ❌ ±0 

Results for commit 1fe4b7c. ± Comparison against base commit 9c7fd42.

♻️ This comment has been updated with latest results.

@simonrw simonrw marked this pull request as ready for review August 13, 2025 03:42
pinzon
pinzon approved these changes Aug 13, 2025
View reviewed changes
Copy link
Member

@pinzon pinzon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great change 👍
Nit: it would be nice to see a test for the case when AWS::NoValue is used in a Fn::Sub.

@simonrw simonrw force-pushed the cfn/v2/handle-aws-novalue branch from 31809f2 to 1fe4b7c Compare August 14, 2025 22:52
@simonrw
Copy link
Contributor Author

simonrw commented Aug 14, 2025

Great change 👍 Nit: it would be nice to see a test for the case when AWS::NoValue is used in a Fn::Sub.

If I try to use an AWS::NoValue in a Sub expression, e.g.

Resources:
  AnotherParameter:
    Type: AWS::SSM::Parameter
    Properties:
      Type: String
      Value:
        Fn::Sub:
          - "My great value ${foo}"
          - foo: !Ref AWS::NoValue

I get the following error:

Template error: every value of the context object of every Fn::Sub object must be a string or a function that returns a string

Base automatically changed from cfn/v2/resolve-2 to main August 15, 2025 04:44
@simonrw simonrw merged commit f38b38b into main Aug 15, 2025
40 checks passed
@simonrw simonrw deleted the cfn/v2/handle-aws-novalue branch August 15, 2025 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pinzon pinzon pinzon approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

Assignees
No one assigned
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Milestone
4.8
Development

Successfully merging this pull request may close these issues.
2 participants
@simonrw @pinzon