Skip to content

Conversation

@peter-smith-phd
Copy link
Contributor

Motivation

To improve our CloudFormation resource providers, this change makes it easier to scaffold new resource types.

Changes

The following things have changed:

  • We now fetch schema files directly from the CloudFormation service, rather than from a .zip file.
  • The test cases/templates are stored in the individual service directories (no longer in the cloudformation directory).
  • Fixed a bug where --pro flag wasn't always been obeyed.

Some future changes (not included here):

  • Allow for regular auto-updating of schemas as AWS updates them.
  • Separate the data type definitions from the resource provider code, making it easier to automatically update data types without clobbering the custom-written handler methods.

Tests

  • Manually tested generate of an SQS resource (non-pro) and an ECR (pro), and visually inspected that the files appeared in the correct directory, and the content was correct.

Related

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

github-actions bot commented Dec 11, 2025

Test Results - Preflight, Unit

22 978 tests   - 23   21 136 ✅  - 22   6m 5s ⏱️ -18s
     1 suites ± 0    1 842 💤  -  1 
     1 files   ± 0        0 ❌ ± 0 

Results for commit c232f45. ± Comparison against base commit ca6b22b.

This pull request removes 23 tests.
tests.unit.aws.test_connect.TestClientFactory ‑ test_typed_client_creation[iotanalytics]
tests.unit.test_dns_server.TestDNSServer ‑ test_delete_operations_of_nonexistent_entries
tests.unit.test_dns_server.TestDNSServer ‑ test_dns_server_add_alias_lifecycle_with_ids
tests.unit.test_dns_server.TestDNSServer ‑ test_dns_server_add_host_lifecycle
tests.unit.test_dns_server.TestDNSServer ‑ test_dns_server_add_host_lifecycle_with_ids
tests.unit.test_dns_server.TestDNSServer ‑ test_dns_server_add_multiple_hosts
tests.unit.test_dns_server.TestDNSServer ‑ test_dns_server_alias_health_checks
tests.unit.test_dns_server.TestDNSServer ‑ test_dns_server_alias_lifecycle
tests.unit.test_dns_server.TestDNSServer ‑ test_dns_server_clear
tests.unit.test_dns_server.TestDNSServer ‑ test_dns_server_fallback
…

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Dec 11, 2025

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   31m 5s ⏱️ - 1h 24m 18s
584 tests  - 4 557  471 ✅  - 4 274  113 💤  - 283  0 ❌ ±0 
586 runs   - 4 557  471 ✅  - 4 274  115 💤  - 283  0 ❌ ±0 

Results for commit c232f45. ± Comparison against base commit ca6b22b.

This pull request removes 4557 tests.
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]
…

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Test Results (amd64) - Acceptance

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

Results for commit c232f45. ± Comparison against base commit ca6b22b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Test Results (amd64) - Integration, Bootstrap

  5 files  ±    0    5 suites  ±0   43m 17s ⏱️ - 1h 52m 10s
608 tests  - 4 913  496 ✅  - 4 469  112 💤  - 444  0 ❌ ±0 
614 runs   - 4 913  496 ✅  - 4 469  118 💤  - 444  0 ❌ ±0 

Results for commit c232f45. ± Comparison against base commit ca6b22b.

This pull request removes 4913 tests.
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]
…

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Test Results - Alternative Providers

583 tests   - 880   325 ✅  - 557   17m 57s ⏱️ - 15m 14s
  1 suites  -   4   258 💤  - 323 
  1 files    -   4     0 ❌ ±  0 

Results for commit c232f45. ± Comparison against base commit ca6b22b.

This pull request removes 880 tests.
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_basic_operations_multiple_protocols[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_basic_operations_multiple_protocols[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_basic_operations_multiple_protocols[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_multi_protocol_client_fixture[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_multi_protocol_client_fixture[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_multi_protocol_client_fixture[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_alarm_lambda_target
…

♻️ This comment has been updated with latest results.

Copy link
Contributor

@simonrw simonrw left a comment

Choose a reason for hiding this comment

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

This looks like a very sensible change, thanks!

Next step will be to render the scaffolding to a separate directory and update all resource providers to subclass the provider so we can separate autogenerated and manually written code 👍


def schema(self, type_name: ResourceName) -> ResourceSchema:
"""
Given a CloudFormation resource type name (e.g. AWS::S3::Bucket), return the resource schema as dict.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: strictly the argument is a ResourceName which can be created from a str with ResourceName.from_name(type_name)

@peter-smith-phd
Copy link
Contributor Author

Next step will be to render the scaffolding to a separate directory and update all resource providers to subclass the provider so we can separate autogenerated and manually written code 👍

Will do. What are your thoughts on how best to do that? I have a few ideas:

  1. Create a aws_sqs_queue_types.py file that simply contains the type definitions. These will be imported into aws_sqs_queue.py and referenced in the hand-written create, update etc. methods.

  2. Create a aws_sqs_queue_base.py file that contains the types, as well as abstract methods for create, update, etc. The hand-written aws_sqs_queue.py must override those same methods by inheriting from aws_sqs_queue_base.py.

  3. Put all the auto-generated files into a subdirectory (e.g. generated) and then use approach (1) or (2) to import/inherit the symbols. This might have the benefit of keeping the code a little tidier, and making it clear what's auto-generated and what's hand-written.

Sounds like you're suggesting (2) + (3)?

@peter-smith-phd peter-smith-phd merged commit 9d80f0b into main Dec 15, 2025
46 of 51 checks passed
@peter-smith-phd peter-smith-phd deleted the sqs/make-cfn-gold branch December 15, 2025 17:59
@simonrw
Copy link
Contributor

simonrw commented Dec 15, 2025

Next step will be to render the scaffolding to a separate directory and update all resource providers to subclass the provider so we can separate autogenerated and manually written code 👍

Will do. What are your thoughts on how best to do that? I have a few ideas:

  1. Create a aws_sqs_queue_types.py file that simply contains the type definitions. These will be imported into aws_sqs_queue.py and referenced in the hand-written create, update etc. methods.

  2. Create a aws_sqs_queue_base.py file that contains the types, as well as abstract methods for create, update, etc. The hand-written aws_sqs_queue.py must override those same methods by inheriting from aws_sqs_queue_base.py.

  3. Put all the auto-generated files into a subdirectory (e.g. generated) and then use approach (1) or (2) to import/inherit the symbols. This might have the benefit of keeping the code a little tidier, and making it clear what's auto-generated and what's hand-written.

Sounds like you're suggesting (2) + (3)?

I am siding with 2 and 3 because we can scaffold the base class which can include the NotImplemeted stubs and give us a good default fallback.

Also on the topic of types it might be nice to consolidate the types for a service in the future if possible so we are not duplicating common type definitions. I don't know how much value there is in this (ie how many resources share identical types to other resources in the same service) but I'd like to find out!

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: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants