Skip to content

CFn: Protect against in review stacks for GetTemplateSummary #13126

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
simonrw merged 2 commits into from
Sep 11, 2025
Merged

CFn: Protect against in review stacks for GetTemplateSummary #13126

simonrw merged 2 commits into from
Sep 11, 2025

Conversation

@simonrw
Copy link
Contributor

@simonrw simonrw commented Sep 10, 2025

Motivation

A user has experienced an issue when invoking GetTemplateSummary. It seems they tried to invoke the method on a stack which has only a change set but has not been executed. This causes a failed attempt to access a property on the stack that is not defined.

Changes

  • Ensure Stack.template is a typed and initialized property (it was not before)
  • Disallow GetTemplateSummary for stacks in review
  • Add integration test for this functionality

@simonrw simonrw added this to the 4.9 milestone Sep 10, 2025
@simonrw simonrw added aws:cloudformation AWS CloudFormation semver: patch Non-breaking changes which can be included in patch releases review: merge when ready Signals to the reviewer that a PR can be merged if accepted docs: skip Pull request does not require documentation changes labels Sep 10, 2025
@simonrw simonrw marked this pull request as ready for review September 10, 2025 23:31
@github-actions
Copy link

github-actions bot commented Sep 10, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 124 tests  ±0   20 386 ✅ ±0   6m 57s ⏱️ +32s
     1 suites ±0    1 738 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 5b6de69. ± Comparison against base commit aa8c1ba.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

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

Results for commit 5b6de69. ± Comparison against base commit aa8c1ba.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025 •
edited
Loading

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   35m 9s ⏱️ - 1h 23m 37s
559 tests  - 4 096  444 ✅  - 3 880  115 💤  - 216  0 ❌ ±0 
561 runs   - 4 096  444 ✅  - 3 880  117 💤  - 216  0 ❌ ±0 

Results for commit 5b6de69. ± Comparison against base commit aa8c1ba.

This pull request removes 4097 and adds 1 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.cloudformation.api.test_templates ‑ test_get_template_summary_non_executed_change_set

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 11, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

  5 files  ±    0    5 suites  ±0   48m 13s ⏱️ - 1h 48m 49s
583 tests  - 4 446  469 ✅  - 4 069  114 💤  - 377  0 ❌ ±0 
589 runs   - 4 446  469 ✅  - 4 069  120 💤  - 377  0 ❌ ±0 

Results for commit 5b6de69. ± Comparison against base commit aa8c1ba.

This pull request removes 4447 and adds 1 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.cloudformation.api.test_templates ‑ test_get_template_summary_non_executed_change_set

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 11, 2025 •
edited
Loading

Test Results - Alternative Providers

558 tests   - 657   328 ✅  - 369   24m 15s ⏱️ - 13m 55s
  1 suites  -   4   230 💤  - 288 
  1 files    -   4     0 ❌ ±  0 

Results for commit 5b6de69. ± Comparison against base commit aa8c1ba.

This pull request removes 658 and adds 1 tests. Note that renamed tests count towards both. 
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_alarm_lambda_target
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_anomaly_detector_lifecycle
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_aws_sqs_metrics_created
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_breaching_alarm_actions
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_create_metric_stream
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_dashboard_lifecycle
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_default_ordering
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_delete_alarm
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_describe_alarms_converts_date_format_correctly
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_describe_minimal_metric_alarm
…

tests.aws.services.cloudformation.api.test_templates ‑ test_get_template_summary_non_executed_change_set
This pull request removes 289 skipped tests and adds 1 skipped test. Note that renamed tests count towards both. 
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_anomaly_detector_lifecycle
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_create_metric_stream
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_describe_minimal_metric_alarm
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_get_metric_data_different_units_no_unit_in_query[metric_data0]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_get_metric_data_different_units_no_unit_in_query[metric_data1]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_get_metric_data_different_units_no_unit_in_query[metric_data2]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_get_metric_data_pagination
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_get_metric_data_with_zero_and_labels
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_handle_different_units
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_insight_rule
…

tests.aws.services.cloudformation.api.test_templates ‑ test_get_template_summary_non_executed_change_set

♻️ This comment has been updated with latest results.

pinzon
pinzon approved these changes Sep 11, 2025
View reviewed changes
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.

Nice improvement!

@simonrw simonrw merged commit 8b4e782 into main Sep 11, 2025
43 checks passed
@simonrw simonrw deleted the cfn/fix/get-template-summary-review-in-progress branch September 11, 2025 15:56
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

aws:cloudformation AWS CloudFormation docs: skip Pull request does not require documentation changes review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

4.9

Development

Successfully merging this pull request may close these issues.

3 participants

@simonrw @pinzon