OCPBUGS-98571: Exclude ARO HCP from internal LB scope for PublicAndPrivate topology

[zgalor]

Summary

Adds an IsAroHCPByHCP guard to the HCCO's params.go to prevent setting loadBalancerScope=InternalLoadBalancer on ARO HCP clusters with PublicAndPrivate topology. This matches the existing guard pattern in router.go.

Problem

When CS sets topology=PublicAndPrivate on Swift-enabled HostedClusters (as required by the enhancement openshift/enhancements#1985), the HCCO creates the guest cluster's IngressController with loadBalancerScope=Internal. This causes the ingress operator to create a per-cluster internal Azure LB (<infraID>-internal), bypassing the shared ingress router that ARO HCP uses. The *.apps DNS record points to the internal LB private IP (10.0.0.5) instead of the shared ingress public IP, making application routes unreachable externally.

Validation

• Validated on OCP 4.22.3 with topology=PublicAndPrivate on a personal dev environment
• Baseline cluster (no fix): -internal LB created at 10.0.0.5, IngressController scope: Internal
• Fix cluster: after overriding the HCCO image (not just the CPO image) with this fix and deleting the IngressController, the HCCO recreated it with scope: External
• Note: initial testing only overrode the CPO image which did not work — the HCCO is a separate deployment pulled from the release image, so the fix must be applied to the HCCO image specifically

Bug

https://redhat.atlassian.net/browse/OCPBUGS-98571

References

• Enhancement: OCPSTRAT-1986: Enhancement: API-driven Azure topology and Swift private connectivity for HyperShift enhancements#1985
• PR CNTRLPLANE-2172: feat(azure): add private endpoint access topology for self-managed HostedClusters #7821 (added the params.go code): CNTRLPLANE-2172: feat(azure): add private endpoint access topology for self-managed HostedClusters #7821
• router.go (existing ARO HCP guard): control-plane-operator/controllers/hostedcontrolplane/ingress/router.go line 48

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@zgalor: This pull request references Jira Issue OCPBUGS-98571, which is invalid:

• expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

Adds an IsAroHCPByHCP guard to the HCCO's params.go to prevent setting loadBalancerScope=InternalLoadBalancer on ARO HCP clusters with PublicAndPrivate topology. This matches the existing guard pattern in router.go.

Problem

When CS sets topology=PublicAndPrivate on Swift-enabled HostedClusters (as required by the enhancement openshift/enhancements#1985), the HCCO creates the guest cluster's IngressController with loadBalancerScope=Internal. This causes the ingress operator to create a per-cluster internal Azure LB (<infraID>-internal), bypassing the shared ingress router that ARO HCP uses. The *.apps DNS record points to the internal LB private IP (10.0.0.5) instead of the shared ingress public IP, making application routes unreachable externally.

Important note

Testing showed this fix alone does not fully resolve the issue — the IngressController was still created with scope: Internal even with a CPO built from this change. There appears to be another code path setting the Internal scope that I have not identified. This PR is submitted to assist investigation and address the params.go code path regardless.

Validation

• Validated on OCP 4.22.3 with topology=PublicAndPrivate on a personal dev environment
• Baseline cluster (no fix): -internal LB created at 10.0.0.5, IngressController scope: Internal
• Fix cluster (this change): -internal LB still created — fix is necessary but not sufficient

Bug

https://redhat.atlassian.net/browse/OCPBUGS-98571

References

• Enhancement: OCPSTRAT-1986: Enhancement: API-driven Azure topology and Swift private connectivity for HyperShift enhancements#1985
• PR CNTRLPLANE-2172: feat(azure): add private endpoint access topology for self-managed HostedClusters #7821 (added the params.go code): CNTRLPLANE-2172: feat(azure): add private endpoint access topology for self-managed HostedClusters #7821
• router.go (existing ARO HCP guard): control-plane-operator/controllers/hostedcontrolplane/ingress/router.go line 48

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]

📝 Walkthrough

Walkthrough

NewIngressParams now uses Azure utilities to detect ARO HCP clusters. Azure private and public+private topologies receive an internal load balancer scope only when the cluster is not an ARO HCP cluster. A comment documents the exception.

Suggested reviewers: bryan-cox, nirshal

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
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	Only params.go changed; no Ginkgo test titles were added or modified, so no unstable test names are present.
Test Structure And Quality	✅ Passed	PR only changes params.go; no Ginkgo test code was added or modified, so the test-structure checklist is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	Change only adjusts Azure ingress LB scope in params.go; it adds no replicas, anti-affinity, node selectors, topology spread, or PDB logic.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR only changes params.go; no Ginkgo e2e tests were added, so the IPv6/disconnected compatibility check is not applicable.
No-Weak-Crypto	✅ Passed	PASS: The changed params.go code only adjusts Azure ingress LB scope logic; no weak crypto, custom crypto, or secret/token comparisons appear in the diff.
Container-Privileges	✅ Passed	Only params.go changed; no container/K8s manifest or securityContext changes, and no new privileged/root/hostPID/etc settings were introduced.
No-Sensitive-Data-In-Logs	✅ Passed	Changed file only adjusts Azure LB scope logic and comments; no logging calls or sensitive values were added.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: excluding ARO HCP from internal load balancer scope for PublicAndPrivate topology.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zgalor
Once this PR has been reviewed and has the lgtm label, please assign enxebre for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[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
`@control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/params.go`:
- Around line 51-59: Update the annotation-based loadBalancerScope assignment to
also require !azureutil.IsAroHCPByHCP(hcp), preventing ARO HCP clusters from
receiving InternalLoadBalancer through
hyperv1.IngressControllerLoadBalancerScope. Add a regression test covering an
ARO HCP with that annotation and verify the internal scope is not selected.

🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 0a8afd2c-3bb3-4b1f-9e9b-4d28dea728b8

📥 Commits

Reviewing files that changed from the base of the PR and between 65839bb and add1d86.

📒 Files selected for processing (1)

• control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/params.go

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target file with line numbers.
cat -n control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/params.go | sed -n '1,140p'

# Find related annotation handling and ARO checks.
rg -n "IngressControllerLoadBalancerScope|IsAroHCPByHCP|InternalLoadBalancer|loadBalancerScope" control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress -S

# Look for tests covering this behavior.
rg -n "Aro|ARO|InternalLoadBalancer|IngressControllerLoadBalancerScope|loadBalancerScope" control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress -g '*_test.go' -S

Repository: openshift/hypershift

Length of output: 8765

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the parameter tests around the Azure cases and annotation handling.
cat -n control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/params_test.go | sed -n '1,280p'

Repository: openshift/hypershift

Length of output: 7906

---

Apply the ARO guard to the annotation path too.

hcp.Annotations[hyperv1.IngressControllerLoadBalancerScope] == InternalLoadBalancer still sets loadBalancerScope before the Azure/ARO check runs, so an ARO HCP with that annotation can still get the forbidden internal LB scope. Gate the annotation assignment on !azureutil.IsAroHCPByHCP(hcp) as well, and add a regression test for the ARO + annotation case.

🤖 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
`@control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/params.go`
around lines 51 - 59, Update the annotation-based loadBalancerScope assignment
to also require !azureutil.IsAroHCPByHCP(hcp), preventing ARO HCP clusters from
receiving InternalLoadBalancer through
hyperv1.IngressControllerLoadBalancerScope. Add a regression test covering an
ARO HCP with that annotation and verify the internal scope is not selected.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.79%. Comparing base (845ed6f) to head (d08393d).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8993   +/-   ##
=======================================
  Coverage   43.79%   43.79%           
=======================================
  Files         772      772           
  Lines       96037    96046    +9     
=======================================
+ Hits        42061    42068    +7     
- Misses      51061    51063    +2     
  Partials     2915     2915           

Files with missing lines	Coverage Δ
...igoperator/controllers/resources/ingress/params.go	81.63% <100.00%> (+0.38%)	⬆️

... and 1 file with indirect coverage changes

Flag	Coverage Δ
cmd-support	37.43% <ø> (ø)
cpo-hostedcontrolplane	45.91% <ø> (ø)
cpo-other	45.12% <100.00%> (+<0.01%)	⬆️
hypershift-operator	54.07% <ø> (ø)
other	32.12% <ø> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bryan-cox]

/pipeline required

[openshift-merge-bot]

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-v2-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-gke

[bryan-cox]

/close

In favor of the other PR #8992

[openshift-ci]

@bryan-cox: Closed this PR.

Details

In response to this:

/close

In favor of the other PR

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.

[openshift-ci-robot]

@zgalor: This pull request references Jira Issue OCPBUGS-98571. The bug has been updated to no longer refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

Adds an IsAroHCPByHCP guard to the HCCO's params.go to prevent setting loadBalancerScope=InternalLoadBalancer on ARO HCP clusters with PublicAndPrivate topology. This matches the existing guard pattern in router.go.

Problem

When CS sets topology=PublicAndPrivate on Swift-enabled HostedClusters (as required by the enhancement openshift/enhancements#1985), the HCCO creates the guest cluster's IngressController with loadBalancerScope=Internal. This causes the ingress operator to create a per-cluster internal Azure LB (<infraID>-internal), bypassing the shared ingress router that ARO HCP uses. The *.apps DNS record points to the internal LB private IP (10.0.0.5) instead of the shared ingress public IP, making application routes unreachable externally.

Validation

• Validated on OCP 4.22.3 with topology=PublicAndPrivate on a personal dev environment
• Baseline cluster (no fix): -internal LB created at 10.0.0.5, IngressController scope: Internal
• Fix cluster: after overriding the HCCO image (not just the CPO image) with this fix and deleting the IngressController, the HCCO recreated it with scope: External
• Note: initial testing only overrode the CPO image which did not work — the HCCO is a separate deployment pulled from the release image, so the fix must be applied to the HCCO image specifically

Bug

https://redhat.atlassian.net/browse/OCPBUGS-98571

References

• Enhancement: OCPSTRAT-1986: Enhancement: API-driven Azure topology and Swift private connectivity for HyperShift enhancements#1985
• PR CNTRLPLANE-2172: feat(azure): add private endpoint access topology for self-managed HostedClusters #7821 (added the params.go code): CNTRLPLANE-2172: feat(azure): add private endpoint access topology for self-managed HostedClusters #7821
• router.go (existing ARO HCP guard): control-plane-operator/controllers/hostedcontrolplane/ingress/router.go line 48

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.

[openshift-ci-robot]

@zgalor: This pull request references Jira Issue OCPBUGS-98571, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

Adds an IsAroHCPByHCP guard to the HCCO's params.go to prevent setting loadBalancerScope=InternalLoadBalancer on ARO HCP clusters with PublicAndPrivate topology. This matches the existing guard pattern in router.go.

Problem

When CS sets topology=PublicAndPrivate on Swift-enabled HostedClusters (as required by the enhancement openshift/enhancements#1985), the HCCO creates the guest cluster's IngressController with loadBalancerScope=Internal. This causes the ingress operator to create a per-cluster internal Azure LB (<infraID>-internal), bypassing the shared ingress router that ARO HCP uses. The *.apps DNS record points to the internal LB private IP (10.0.0.5) instead of the shared ingress public IP, making application routes unreachable externally.

Validation

• Validated on OCP 4.22.3 with topology=PublicAndPrivate on a personal dev environment
• Baseline cluster (no fix): -internal LB created at 10.0.0.5, IngressController scope: Internal
• Fix cluster: after overriding the HCCO image (not just the CPO image) with this fix and deleting the IngressController, the HCCO recreated it with scope: External
• Note: initial testing only overrode the CPO image which did not work — the HCCO is a separate deployment pulled from the release image, so the fix must be applied to the HCCO image specifically

Bug

https://redhat.atlassian.net/browse/OCPBUGS-98571

References

• Enhancement: OCPSTRAT-1986: Enhancement: API-driven Azure topology and Swift private connectivity for HyperShift enhancements#1985
• PR CNTRLPLANE-2172: feat(azure): add private endpoint access topology for self-managed HostedClusters #7821 (added the params.go code): CNTRLPLANE-2172: feat(azure): add private endpoint access topology for self-managed HostedClusters #7821
• router.go (existing ARO HCP guard): control-plane-operator/controllers/hostedcontrolplane/ingress/router.go line 48

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.