SPLAT-2713: Reapply "Merge pull request #476" to bring in e2e for BYO SG for AWS NLB by mfbonfigli · Pull Request #492 · openshift/cluster-cloud-controller-manager-operator

mfbonfigli · 2026-07-16T09:00:04Z

Summary

Re-applies #476, which was reverted in #491 due to two NLB BYO Security Group transition tests failing in upgrade jobs.

The root cause of the issue is not related to the tests, but related to the fact that during cluster upgrades the master node IAM permissions do not get updated by the installer. This results in a 5.0 cluster without the new SetSecurityGroup permission, which is required for the e2e tests to pass, causing the failures. Since 4.22 > 5.0 upgrade jobs do not run during presubmits this was noticed only after the PR was merged.

Those failures are being addressed separately (openshift/release#81974) by patching the master IAM role in the CI upgrade step (openshift/release) prior to the test execution. This PR is to be merged only after that one is merged.

What this brings back

Updates the vendored cloud-provider-aws e2e test module, pulling in upstream NLB BYO Security Group transition tests

Fixes getRegionFromEnv() to validate the value of LEASED_RESOURCE against the AWS region format before using it as AWS_REGION, this prevents HyperShift CI from poisoning the region with a UUID lease value

Adds Infrastructure object fallback for region discovery when no valid region is found in environment variables

Moves AWSRegionPattern and GetRegionFromInfrastructure to the common package for shared use between main.go and the aws e2e package

Fixes a flaky unit test for BareMetal platform config sync

Summary by CodeRabbit

Bug Fixes
- Improved AWS region detection by validating configured region values and falling back to the cluster’s Infrastructure configuration when environment data is missing or invalid.
- Enhanced AWS region matching to support a broader set of region identifiers.
Tests
- Improved BareMetal config-sync test reliability by waiting for reconciliation to settle before verifying ConfigMap counts.
Chores
- Updated the AWS E2E test module version used by the test suite.

openshift-ci · 2026-07-16T09:00:08Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

coderabbitai · 2026-07-16T09:00:40Z

Walkthrough

AWS test region resolution now uses shared validation and Infrastructure lookup, with environment-based fallback selection. The BareMetal config-sync test now waits for the expected empty ConfigMap state.

Changes

AWS region resolution

Layer / File(s)	Summary
Shared region lookup contract `openshift-tests/ccm-aws-tests/e2e/common/helper.go`	Adds an exported AWS region pattern and an Infrastructure lookup utility that validates the cluster’s AWS region.
Region selection consumers `openshift-tests/ccm-aws-tests/e2e/aws/helper.go`, `openshift-tests/ccm-aws-tests/main.go`, `openshift-tests/ccm-aws-tests/go.mod`	Updates AWS configuration and test initialization to use shared region resolution, validates environment candidates, adds timed Infrastructure fallback, reorders kubeconfig initialization, and updates the e2e module version.

BareMetal config synchronization test

Layer / File(s)	Summary
Eventually-based ConfigMap assertion `pkg/controllers/cloud_config_sync_controller_test.go`	The BareMetal test polls for an empty ConfigMap list over a two-minute timeout.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TestInitialization
  participant Environment
  participant Infrastructure
  participant AWSConfig
  TestInitialization->>Environment: read and validate region candidates
  Environment-->>TestInitialization: valid region or no match
  TestInitialization->>Infrastructure: request cluster AWS region
  Infrastructure-->>TestInitialization: return region
  TestInitialization->>AWSConfig: apply validated region

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Ipv6 And Disconnected Network Test Compatibility	⚠️ Warning	New ecr_creds test uses a public ECR image, and loadbalancer HTTP URLs are built with http://%s:%d, which breaks IPv6 literals.	Use mirrored/internal images or skip disconnected jobs, and build host URLs with net.JoinHostPort (or bracket IPv6 hosts) before running these e2e specs.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	No Ginkgo titles were added or changed in the PR; inspected titles in the touched test file are static and don’t interpolate dynamic data.
Test Structure And Quality	✅ Passed	The only test change uses injected Gomega inside an Eventually with a 2m/5s timeout and matches existing suite cleanup/polling patterns; no quality regressions found.
Microshift Test Compatibility	✅ Passed	Only a controller unit test changed; no new Ginkgo e2e tests were added, so MicroShift compatibility is not implicated.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The only specs with ≥2-node assertions are the vendored node-label tests, and main.go excludes them on SingleReplica; the load balancer tests use one worker node.
Topology-Aware Scheduling Compatibility	✅ Passed	The diff only changes a test assertion; no manifests, operator code, or controller scheduling logic was added or modified.
Ote Binary Stdout Contract	✅ Passed	No process-level fmt.Print/stdout redirection was added; main() only uses logrus/framework logging and there are no init/TestMain stdout writes.
No-Weak-Crypto	✅ Passed	Touched files and vendored updates contain no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons; searches returned none.
Container-Privileges	✅ Passed	No changed manifest or pod-spec fields set privileged/hostPID/hostNetwork/hostIPC/SYS_ADMIN/allowPrivilegeEscalation, and no root-run settings were introduced.
No-Sensitive-Data-In-Logs	✅ Passed	No touched code logs passwords, tokens, PII, or hostnames; added logs only emit AWS region, namespace, or kubeconfig path.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is clearly related to the main change: reapplying the BYO SG for AWS NLB e2e updates.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

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

Inline comments:
In `@pkg/controllers/cloud_config_sync_controller_test.go`:
- Around line 607-611: Update the Eventually callback in the config map cleanup
test to use an injected Gomega instance, replacing the global Expect around
cl.List with g.Expect so transient list errors are retried. Preserve the
existing namespace filter and zero-length assertion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bad9daf6-aecd-49da-a165-e3a0fa5371c9

📥 Commits

Reviewing files that changed from the base of the PR and between 4aa859c and c541aab.

⛔ Files ignored due to path filters (4)

openshift-tests/ccm-aws-tests/go.sum is excluded by !**/*.sum
openshift-tests/ccm-aws-tests/vendor/k8s.io/cloud-provider-aws/tests/e2e/cloudconfig.go is excluded by !**/vendor/**
openshift-tests/ccm-aws-tests/vendor/k8s.io/cloud-provider-aws/tests/e2e/loadbalancer.go is excluded by !**/vendor/**
openshift-tests/ccm-aws-tests/vendor/modules.txt is excluded by !**/vendor/**

📒 Files selected for processing (5)

openshift-tests/ccm-aws-tests/e2e/aws/helper.go
openshift-tests/ccm-aws-tests/e2e/common/helper.go
openshift-tests/ccm-aws-tests/go.mod
openshift-tests/ccm-aws-tests/main.go
pkg/controllers/cloud_config_sync_controller_test.go

…update-cccmo-tests" This reverts commit cfb95ec.

…test Replaces the global Expect with the injected g.Expect inside the Eventually polling function, following Gomega best practices for async assertions. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

mfbonfigli · 2026-07-21T08:31:53Z

Preventing PR from auto merging before the openshift/release PR fix has been merged:

/hold

mfbonfigli · 2026-07-21T08:51:53Z

/retest

mfbonfigli · 2026-07-21T11:40:18Z

/retest

openshift-ci-robot · 2026-07-21T14:41:56Z

@mfbonfigli: This pull request references SPLAT-2713 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Re-applies #476, which was reverted in #491 due to two NLB BYO Security Group transition tests failing in upgrade jobs.

The root cause of the issue is not related to the tests, but related to the fact that during cluster upgrades the master node IAM permissions do not get updated by the installer. This results in a 5.0 cluster without the new SetSecurityGroup permission, which is required for the e2e tests to pass, causing the failures. Since 4.22 > 5.0 upgrade jobs do not run during presubmits this was noticed only after the PR was merged.

Those failures are being addressed separately (openshift/release#81974) by patching the master IAM role in the CI upgrade step (openshift/release) prior to the test execution. This PR is to be merged only after that one is merged.

What this brings back

Updates the vendored cloud-provider-aws e2e test module, pulling in upstream NLB BYO Security Group transition tests

Fixes getRegionFromEnv() to validate the value of LEASED_RESOURCE against the AWS region format before using it as AWS_REGION, this prevents HyperShift CI from poisoning the region with a UUID lease value

Adds Infrastructure object fallback for region discovery when no valid region is found in environment variables

Moves AWSRegionPattern and GetRegionFromInfrastructure to the common package for shared use between main.go and the aws e2e package

Fixes a flaky unit test for BareMetal platform config sync

Summary by CodeRabbit

Bug Fixes

Improved AWS region detection by validating configured region values and falling back to the cluster’s Infrastructure configuration when environment data is missing or invalid.

Enhanced AWS region matching to support a broader set of region identifiers.

Tests

Improved BareMetal config-sync test reliability by waiting for reconciliation to settle before verifying ConfigMap counts.

Chores

Updated the AWS E2E test module version used by the test suite.

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.

mfbonfigli · 2026-07-21T14:43:00Z

/jira refresh

openshift-ci-robot · 2026-07-21T14:43:03Z

@mfbonfigli: This pull request references SPLAT-2713 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

/jira refresh

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.

mfbonfigli · 2026-07-21T14:43:05Z

/retest

mfbonfigli · 2026-07-21T20:29:35Z

/test e2e-aws-ovn-upgrade

mfbonfigli · 2026-07-22T08:20:20Z

/test e2e-aws-ovn-upgrade

openshift-ci · 2026-07-22T10:20:37Z

@mfbonfigli: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

theobarberbany · 2026-07-22T12:26:33Z

/lgtm
/approve

mfbonfigli · 2026-07-22T12:26:53Z

/payload-job-with-prs periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

openshift-ci · 2026-07-22T12:27:14Z

@mfbonfigli: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

mfbonfigli · 2026-07-22T12:29:26Z

/payload-job periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

openshift-ci · 2026-07-22T12:30:38Z

@mfbonfigli: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/fa7103e0-85c8-11f1-99bc-ac22cfa4bfa1-0

openshift-ci · 2026-07-22T12:40:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: theobarberbany

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [theobarberbany]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mfbonfigli · 2026-07-22T13:13:31Z

/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-ccm

openshift-ci · 2026-07-22T13:13:48Z

@mfbonfigli: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-ccm

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/2329c320-85cf-11f1-8dd2-95c12af15598-0

mfbonfigli · 2026-07-22T13:18:16Z

/payload-job periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

openshift-ci · 2026-07-22T13:18:30Z

@mfbonfigli: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/cd437270-85cf-11f1-957a-b00af81a5bc1-0

mfbonfigli · 2026-07-22T14:25:02Z

The job periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade already flaked twice for unrelated reasons. Trying again.

mfbonfigli · 2026-07-22T14:28:58Z

/payload-job periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

openshift-ci · 2026-07-22T14:29:32Z

@mfbonfigli: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ad657160-85d9-11f1-805b-4d9a7351cc8d-0

mfbonfigli · 2026-07-22T15:46:46Z

Hypershift test passed: This confirms that the new e2es for the BYO SG for AWS NLB feat work in Hypershift after the patched hypershift permissions merged in this other PR.

mfbonfigli · 2026-07-22T20:13:50Z

4.22 > 5.0 upgrade job passed: This confirms that the openshift/release changes to inject the new IAM required permission work as expected during upgrade jobs including for the new e2e tests introduced by this PR.

mfbonfigli · 2026-07-23T08:20:43Z

/verified by @mfbonfigli on CI jobs

openshift-ci-robot · 2026-07-23T08:21:11Z

@mfbonfigli: This PR has been marked as verified by @mfbonfigli on CI jobs.

Details

In response to this:

/verified by @mfbonfigli on CI jobs

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.

mfbonfigli · 2026-07-23T08:21:54Z

/hold cancel

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 16, 2026

coderabbitai Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread pkg/controllers/cloud_config_sync_controller_test.go Outdated

mfbonfigli and others added 2 commits July 17, 2026 11:26

Reapply "Merge pull request openshift#476 from mfbonfigli/SPLAT-2713_…

2962974

…update-cccmo-tests" This reverts commit cfb95ec.

mfbonfigli force-pushed the SPLAT-2713_reapply-pr-476 branch from c541aab to 04f1a42 Compare July 17, 2026 09:30

mfbonfigli marked this pull request as ready for review July 21, 2026 08:30

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 21, 2026

openshift-ci Bot requested review from nrb and theobarberbany July 21, 2026 08:31

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2026

mfbonfigli changed the title ~~Reapply "Merge pull request #476 from mfbonfigli/SPLAT-2713_update-cc…~~ SPLAT-2713: Reapply "Merge pull request #476 from mfbonfigli/SPLAT-2713_update-cc… Jul 21, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 21, 2026

mfbonfigli changed the title ~~SPLAT-2713: Reapply "Merge pull request #476 from mfbonfigli/SPLAT-2713_update-cc…~~ SPLAT-2713: Reapply "Merge pull request #476" to bring in e2e for BYO SG for AWS NLB Jul 21, 2026

openshift-ci Bot assigned theobarberbany Jul 22, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2026

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 22, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jul 23, 2026

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 23, 2026

openshift-merge-bot Bot merged commit 38f5e00 into openshift:main Jul 23, 2026
14 checks passed

Uh oh!

Conversation

mfbonfigli commented Jul 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What this brings back

Summary by CodeRabbit

Uh oh!

openshift-ci Bot commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mfbonfigli commented Jul 21, 2026

Uh oh!

mfbonfigli commented Jul 21, 2026

Uh oh!

mfbonfigli commented Jul 21, 2026

Uh oh!

openshift-ci-robot commented Jul 21, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What this brings back

Summary by CodeRabbit

Uh oh!

mfbonfigli commented Jul 21, 2026

Uh oh!

openshift-ci-robot commented Jul 21, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mfbonfigli commented Jul 21, 2026

Uh oh!

mfbonfigli commented Jul 21, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

theobarberbany commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 22, 2026

Uh oh!

mfbonfigli commented Jul 23, 2026

Uh oh!

openshift-ci-robot commented Jul 23, 2026

Uh oh!

mfbonfigli commented Jul 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

openshift-ci-robot commented Jul 21, 2026 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Jul 21, 2026 •

edited by openshift-ci Bot

Loading