feat(route53): allow specifying an STS region when creating a cross-account zone delegation by naiello · Pull Request #29466 · aws/aws-cdk

naiello · 2024-03-13T04:58:37Z

Issue # (if applicable)

n/a

Reason for this change

In certain circumstances, the default STS region resolution logic will not properly select a region due to the hardcoded mapping between region prefixes and STS regions.

Description of changes

This change allows callers to specify a region in which STS will be called.

Description of how you validated changes

Unit tests were added to aws-cdk-lib. Integ tests were updated and pass on my personal accounts, but I'm not sure how to check if they'll pass in CI.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

aaythapa

Thanks for PR! Left a few comments but nothing major.

Changes look good, could you add README changes and integ tests so the linter is happy? You can use yarn integ --update-on-failed to update the snapshots of existing tests

packages/aws-cdk-lib/aws-route53/test/record-set.test.ts

...-cdk/custom-resource-handlers/lib/aws-route53/cross-account-zone-delegation-handler/index.ts

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

naiello · 2024-03-19T05:23:10Z

Please double-check the integ test changes as this is my first contribution to aws-cdk. The tests are passing on my personal account(s), but I'm not sure how to verify they'll pass properly in the CICD.

packages/aws-cdk-lib/aws-route53/lib/record-set.ts

…ccount zone delegation

naiello · 2024-03-22T23:55:44Z

Snapshot Results:

Tests:    1 failed, 1 total
Failed: /home/nick/projects/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.js

Running integration tests for failed tests...

Running in parallel across profiles cross-region and regions: us-east-1, us-east-2, us-west-2
Running test /home/nick/projects/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.js in cross-region/us-east-1
  SUCCESS    aws-route53/test/integ.cross-account-zone-delegation-Route53CrossAccountInteg/DefaultTest 535.347s
       NO ASSERTIONS

Test Results:

Tests:    1 passed, 1 total
Done in 536.06s.

msambol

LGTM. 👍🏼

...-cdk/custom-resource-handlers/lib/aws-route53/cross-account-zone-delegation-handler/index.ts

...aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.ts

aaythapa

🚢

mergify · 2024-03-28T19:33:01Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2024-03-28T19:36:35Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aaythapa · 2024-03-28T22:52:03Z

@Mergifyio update

mergify · 2024-03-28T22:52:12Z

update

❌ Mergify doesn't have permission to update

Details

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/request-cli-integ-test.yml without workflows permission

Pull request has been modified.

mergify · 2024-03-28T23:29:36Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-03-28T23:57:35Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: a20148d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-03-28T23:57:58Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Mar 13, 2024

aws-cdk-automation requested a review from a team March 13, 2024 04:58

naiello changed the title ~~Route53 cross account sts region~~ Allow specifying a region to call STS in when delegating a cross-account Route53 zone Mar 13, 2024

aws-cdk-automation previously requested changes Mar 13, 2024

View reviewed changes

naiello changed the title ~~Allow specifying a region to call STS in when delegating a cross-account Route53 zone~~ feat(route53): allow specifying an STS region when creating a cross-account zone delegation Mar 13, 2024

aaythapa reviewed Mar 15, 2024

View reviewed changes

github-actions bot mentioned this pull request Mar 18, 2024

Weekly PR metrics report #29527

Closed

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Mar 19, 2024

msambol reviewed Mar 20, 2024

View reviewed changes

packages/aws-cdk-lib/aws-route53/lib/record-set.ts Show resolved Hide resolved

naiello and others added 9 commits March 22, 2024 18:10

feat(route53): allow specifying an STS region when creating a cross-a…

e89e3e3

…ccount zone delegation

chore: rename property

4fb52cd

test: add tests

9aa4da7

test: integ test snapshot updates

28a1f6b

chore: update README

b8f94d2

test: update cross-account-zone-delegation integ test

3b55091

test: update rename-cross-account-zone-delegation

68d3896

chore: update spacing

1063804

Merge branch 'main' into route53-cross-account-sts-region

e2f6367

msambol approved these changes Mar 23, 2024

View reviewed changes

aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Mar 23, 2024

aaythapa reviewed Mar 27, 2024

View reviewed changes

...-cdk/custom-resource-handlers/lib/aws-route53/cross-account-zone-delegation-handler/index.ts Show resolved Hide resolved

...aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.ts Show resolved Hide resolved

chore: update spacing

a89ddf1

aaythapa previously approved these changes Mar 28, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 28, 2024

Merge branch 'main' into route53-cross-account-sts-region

252c297

aaythapa approved these changes Mar 28, 2024

View reviewed changes

Merge branch 'main' into route53-cross-account-sts-region

a20148d

mergify bot merged commit fe4bc1d into aws:main Mar 28, 2024

github-actions bot mentioned this pull request Apr 1, 2024

Monthly PR metrics report #29668

Closed

This was referenced May 23, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#98

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#99

Open

NOUIY mentioned this pull request May 30, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#101

Open

Conversation

naiello commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

aaythapa left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

naiello commented Mar 19, 2024

Uh oh!

Uh oh!

naiello commented Mar 22, 2024

Uh oh!

msambol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aaythapa left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Mar 28, 2024

Uh oh!

mergify bot commented Mar 28, 2024

Uh oh!

aaythapa commented Mar 28, 2024

Uh oh!

mergify bot commented Mar 28, 2024

❌ Mergify doesn't have permission to update

Uh oh!

mergify bot commented Mar 28, 2024

Uh oh!

aws-cdk-automation commented Mar 28, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Mar 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

naiello commented Mar 13, 2024 •

edited

Loading

aaythapa left a comment •

edited

Loading