OCPBUGS-99755: OTE: fix AWS endpoint resolution for non-standard partitions - #494
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAWS load balancer and EC2 test clients no longer override SDK endpoint resolution with hardcoded regional hostnames. ChangesAWS endpoint selection
Estimated code review effort: 1 (Trivial) | ~3 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 |
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 |
de8f63c to
f48822d
Compare
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-e2e-aws-ovn-conformance-ccm-techpreview |
|
@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/84ce8b10-8a3e-11f1-960a-7f6a1726f82c-0 |
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-techpreview |
|
@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/fb874df0-8a3e-11f1-9674-dba2d98d43cc-0 |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/installer#10672 |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/installer#10717 Need to use an installer without merge conflicts 😅 |
|
/test e2e-aws-ovn-upgrade |
We can rely on the default endpoint resolver of the SDK to construct the correct partition-aware public regional endpoint.
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-techpreview |
|
@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/72623570-8a58-11f1-9ae3-326786f070e2-0 |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/installer#10717 |
|
/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/253a58d0-8a9f-11f1-9eda-dec6bcfd384f-0 |
|
/test unit |
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance |
|
@tthvo: trigger 2 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/d88acf60-8ab7-11f1-86bd-c27a5fa60372-0 |
|
/lgtm |
|
The following tests are expected to pass: They should succeed in standard AWS regions, EUSC and hypershift environment. Result analysis 4/4
There are other e2e test failures, which are unrelated and can be ignored. /verified by CI |
openshift-ci-robot
added
the
verified
|
@tthvo: This PR has been marked as verified by 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 APPROVED This pull-request has been approved by: nrb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/tide refresh |
7418962
into
openshift:main
|
@tthvo: Jira Issue Verification Checks: Jira Issue OCPBUGS-99755 Jira Issue OCPBUGS-99755 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
Fix included in release 5.0.0-0.nightly-2026-07-30-040517 |
5 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 removes the
BaseEndpointoverride to let the AWS SDK automatically resolve to an partition-aware endpoint.See also #464.
Note: 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