Skip to content

CFn: ecr repo uses request account and region #13156

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 1 commit into from
Sep 17, 2025
Merged

CFn: ecr repo uses request account and region #13156

merged 1 commit into from
Sep 17, 2025

Conversation

@simonrw
Copy link
Contributor

@simonrw simonrw commented Sep 17, 2025 •
edited
Loading

Motivation

While implementing some fixes for the ECR repository in #13146, we broke the multi-account/multi-region pipeline. It turns out the ECR repository resoure provider hard-codes the account id 000000000000 and region us-east-1 in its response. I presume this has not been caught before as it is not asserted anywhere in the tests?

Changes

  • Use the account id and region of the stack

Testing

No additional tests have been added, but a green CI run with both the normal integration tests and MA/MR pipeline is enough to validate this behaviour.

MA/MR test run

@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 17, 2025
@github-actions
Copy link

github-actions bot commented Sep 17, 2025

LocalStack Community integration with Pro

2 files  2 suites   34s ⏱️
2 tests 0 ✅ 2 💤 0 ❌
4 runs  0 ✅ 4 💤 0 ❌

Results for commit 39dbf3b.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

Test Results - Preflight, Unit

22 160 tests  ±0   20 422 ✅ ±0   6m 36s ⏱️ +5s
     1 suites ±0    1 738 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 39dbf3b. ± Comparison against base commit c8f43c9.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

Test Results (amd64) - Acceptance

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

Results for commit 39dbf3b. ± Comparison against base commit c8f43c9.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

Test Results (amd64) - Integration, Bootstrap

 5 files   5 suites   8m 51s ⏱️
26 tests 24 ✅ 2 💤 0 ❌
32 runs  24 ✅ 8 💤 0 ❌

Results for commit 39dbf3b.

@simonrw simonrw marked this pull request as ready for review September 17, 2025 13:00
@simonrw simonrw requested a review from dfangl as a code owner September 17, 2025 13:00
dfangl
dfangl approved these changes Sep 17, 2025
View reviewed changes
Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

LGTM, nice that this is caught!

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

Test Results (MA/MR) - Preflight, Unit

22 145 tests  ±0   20 407 ✅ ±0   6m 36s ⏱️ -3s
     1 suites ±0    1 738 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 39dbf3b. ± Comparison against base commit ad5b369.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

Test Results (amd64, MA/MR) - Acceptance

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

Results for commit 39dbf3b. ± Comparison against base commit ad5b369.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

Test Results (amd64, MA/MR) - Integration, Bootstrap

 5 files  ±    0   5 suites  ±0   10m 40s ⏱️ - 2h 25m 59s
26 tests  - 5 018  24 ✅  - 4 528  2 💤  - 489  0 ❌  - 1 
32 runs   - 5 018  24 ✅  - 4 528  8 💤  - 489  0 ❌  - 1 

Results for commit 39dbf3b. ± Comparison against base commit ad5b369.

This pull request removes 5018 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]
…

@simonrw simonrw merged commit c33a935 into main Sep 17, 2025
80 checks passed
@simonrw simonrw deleted the fix/cfn/ecr-region branch September 17, 2025 13:25
@alexrashed alexrashed added the notes: skip Pull request does not have to be mentioned in the release notes label Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dfangl dfangl dfangl approved these changes

Assignees

No one assigned

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 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

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@simonrw @dfangl @alexrashed