OCPBUGS-99755: OTE: fix AWS endpoint resolution for non-standard partitions#494
OCPBUGS-99755: OTE: fix AWS endpoint resolution for non-standard partitions#494tthvo wants to merge 1 commit into
Conversation
openshift-ci-robot
added
jira/valid-reference
|
@tthvo: This pull request references Jira Issue OCPBUGS-99755, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughAWS ELBv2 and EC2 client endpoints now derive region-specific DNS suffixes through a shared helper, supporting standard and non-standard AWS partitions while retaining ChangesAWS endpoint partition handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@tthvo: This pull request references Jira Issue OCPBUGS-99755, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/cc @mtulio @mfbonfigli |
|
@tthvo: This pull request references Jira Issue OCPBUGS-99755, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/installer#10672 |
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance |
|
@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/840867a0-879b-11f1-8fa2-ca425d91c641-0 |
The BaseEndpoint for ELBv2 and EC2 clients was hardcoded to .amazonaws.com, which is incorrect for non-standard AWS partitions like EUSC (.amazonaws.eu). This caused API calls to fail on EUSC clusters because the requests were sent to the wrong domain. Use partition-aware DNS suffix resolution instead of hardcoding .amazonaws.com, so that all AWS partitions (EUSC, China, GovCloud, ISO, etc.) resolve to the correct public regional endpoint.
tthvo
changed the title
|
@tthvo: This pull request references Jira Issue OCPBUGS-99755, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openshift-tests/ccm-aws-tests/e2e/aws/helper.go (1)
200-220: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd table-driven tests for every partition suffix.
This helper is shared by both ELBv2 and EC2 clients; test
cn-,eusc-, each ISO prefix,us-gov-, and the default case to prevent silent endpoint regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openshift-tests/ccm-aws-tests/e2e/aws/helper.go` around lines 200 - 220, Add table-driven unit tests for dnsSuffixForRegion covering cn-, eusc-, us-iso-, us-isob-, eu-isoe-, us-isof-, us-gov-, and an unmatched region. Assert each input returns its expected partition suffix, including the default amazonaws.com case.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@openshift-tests/ccm-aws-tests/e2e/aws/helper.go`:
- Around line 200-220: Add table-driven unit tests for dnsSuffixForRegion
covering cn-, eusc-, us-iso-, us-isob-, eu-isoe-, us-isof-, us-gov-, and an
unmatched region. Assert each input returns its expected partition suffix,
including the default amazonaws.com case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 002f9648-ade4-4030-9da3-1446579091d8
📒 Files selected for processing (1)
openshift-tests/ccm-aws-tests/e2e/aws/helper.go
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance |
|
@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6e13f750-87a2-11f1-8221-50acf096a3f1-0 |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/installer#10672 |
2 participants
Description
The
BaseEndpointfor ELBv2 and EC2 clients was hardcoded to.amazonaws.com, which is incorrect for non-standard AWS partitions like EUSC (.amazonaws.eu). This caused API calls to fail on EUSC clusters because the requests were sent to the wrong domain.This PR introduces the use of partition-aware DNS suffix resolution instead of hardcoding
.amazonaws.com, so that all AWS partitions (EUSC, China, GovCloud, ISO, etc.) resolve to the correct (public) regional endpoint.See #464 for why we need to set the endpoint for AWS clients.
Originally, I thought it was due to an outdated AWS SDK deps, but the endpoint resolution is fine (without base endpoint override) thanks to the compatible core config module. Thus, there is no longer any need to upgrade go deps.
Summary by CodeRabbit