Skip to content

CFn v2: support outputs #12536

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

Merged
merged 3 commits into from
Apr 24, 2025
Merged

CFn v2: support outputs #12536

merged 3 commits into from
Apr 24, 2025

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Apr 17, 2025

Motivation

Currently the preprocessor supports evaluating outputs, but the executor does not use these values.

Changes

  • Update the executor execute method to return a structured value which includes the stack outputs
  • Override the visit method to extract the output value
  • Store the outputs on the executor and then update the stack when resolved
  • Update the simple test to use the output values

@simonrw simonrw added aws:cloudformation AWS CloudFormation semver: patch Non-breaking changes which can be included in patch releases labels Apr 17, 2025
@simonrw simonrw self-assigned this Apr 17, 2025
@simonrw simonrw force-pushed the cfn/v2/executor/support-outputs branch from 484cf82 to 451f836 Compare April 17, 2025 14:53
Copy link

github-actions bot commented Apr 17, 2025

LocalStack Community integration with Pro

  2 files    2 suites   20m 47s ⏱️
467 tests 314 ✅ 153 💤 0 ❌
469 runs  314 ✅ 155 💤 0 ❌

Results for commit 271ec4c.

♻️ This comment has been updated with latest results.

@simonrw simonrw force-pushed the cfn/v2/executor/support-outputs branch 2 times, most recently from 6b187b5 to b07c444 Compare April 24, 2025 14:14
@simonrw simonrw marked this pull request as ready for review April 24, 2025 14:17
@simonrw simonrw requested a review from MEPalma April 24, 2025 14:17
Copy link
Contributor

@MEPalma MEPalma left a comment

Choose a reason for hiding this comment

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

Thank you, it looks very good!
Aside from a couple of nits for future consideration, I see some tests in localstack/tests/aws/services/cloudformation/v2 failing due to the v2 returning empty Outputs fields; perhaps we can address this detail in this PR?

"Outputs": [
# TODO(parity): Description, ExportName
# TODO(parity): what happens on describe stack when the stack has not been deployed yet?
{"OutputKey": k, "OutputValue": v}
Copy link
Contributor

Choose a reason for hiding this comment

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

We could instantiate these as localstack.aws.api.cloudformation.Output

@@ -27,10 +30,18 @@
LOG = logging.getLogger(__name__)


@dataclass
class ChangeSetModelExecutorResult:
resources: dict
Copy link
Contributor

Choose a reason for hiding this comment

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

Moving forward we could think of typing these

Copy link
Contributor Author

@simonrw simonrw Apr 24, 2025

Choose a reason for hiding this comment

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

I want to do a proper typing pass when everything has settled 👍 A big issue with the old provider is lack of types

@simonrw simonrw force-pushed the cfn/v2/executor/support-outputs branch from b07c444 to b111505 Compare April 24, 2025 15:56
@simonrw simonrw requested a review from MEPalma April 24, 2025 16:25
Copy link
Contributor

@MEPalma MEPalma left a comment

Choose a reason for hiding this comment

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

LGTM!

@simonrw simonrw merged commit e8d2029 into master Apr 24, 2025
31 checks passed
@simonrw simonrw deleted the cfn/v2/executor/support-outputs branch April 24, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:cloudformation AWS CloudFormation 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.

2 participants