Skip to content

fix(smus): Make AWS partition and endpoint handling GovCloud-resilient - #8843

Open
ziwikiwi wants to merge 1 commit into
aws:masterfrom
ziwikiwi:feat/govcloud-partition-resilience
Open

fix(smus): Make AWS partition and endpoint handling GovCloud-resilient#8843
ziwikiwi wants to merge 1 commit into
aws:masterfrom
ziwikiwi:feat/govcloud-partition-resilience

Conversation

@ziwikiwi

Copy link
Copy Markdown
Contributor

Local IDE hardcoded the commercial aws partition and .amazonaws.com DNS suffix in several ARN and endpoint constructions, which break in GovCloud (aws-us-gov partition). Route these through the existing regionProvider (getPartitionId / getDnsSuffixForRegion) so they resolve correctly per partition.

  • smusUtils.extractAccountIdFromSageMakerArn: match any partition (arn:aws[a-z-]*:) instead of literal arn:aws:
  • detached-server parseArn: match any partition
  • detached-server startSagemakerSession: only override the endpoint when SAGEMAKER_ENDPOINT is set; otherwise let the SDK resolver derive the partition-correct host (this file cannot import vscode/globals)
  • sqlWorkbenchClient: derive partition for the ARN and dnsSuffix for the endpoint from regionProvider
  • auth/sso OidcClient.authorize: derive dnsSuffix for the OIDC endpoint

Added GovCloud (aws-us-gov) test cases for both ARN parsers.

Note: two hardcoded endpoints were intentionally left for follow-up as they need endpoint-format confirmation, not a mechanical fix:

  • datazoneCustomClientHelper datazone.{region}.api.aws
  • smusUtils Identity Center issuer identitycenter.amazonaws.com (coupled to a validation regex in auth/sso/constants.ts)

Problem

Solution


  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

Local IDE hardcoded the commercial `aws` partition and `.amazonaws.com`
DNS suffix in several ARN and endpoint constructions, which break in
GovCloud (aws-us-gov partition). Route these through the existing
regionProvider (getPartitionId / getDnsSuffixForRegion) so they resolve
correctly per partition.

- smusUtils.extractAccountIdFromSageMakerArn: match any partition
  (arn:aws[a-z-]*:) instead of literal arn:aws:
- detached-server parseArn: match any partition
- detached-server startSagemakerSession: only override the endpoint when
  SAGEMAKER_ENDPOINT is set; otherwise let the SDK resolver derive the
  partition-correct host (this file cannot import vscode/globals)
- sqlWorkbenchClient: derive partition for the ARN and dnsSuffix for the
  endpoint from regionProvider
- auth/sso OidcClient.authorize: derive dnsSuffix for the OIDC endpoint

Added GovCloud (aws-us-gov) test cases for both ARN parsers.

Note: two hardcoded endpoints were intentionally left for follow-up as
they need endpoint-format confirmation, not a mechanical fix:
  - datazoneCustomClientHelper `datazone.{region}.api.aws`
  - smusUtils Identity Center issuer `identitycenter.amazonaws.com`
    (coupled to a validation regex in auth/sso/constants.ts)
@ziwikiwi
ziwikiwi requested a review from a team July 30, 2026 23:16
@amazon-inspector-ohio

Copy link
Copy Markdown

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

@amazon-inspector-ohio

Copy link
Copy Markdown

✅ I finished the code review, and didn't find any security or code quality issues.

@github-actions

Copy link
Copy Markdown
  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants