Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ingress

import (
hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
"github.com/openshift/hypershift/support/azureutil"
"github.com/openshift/hypershift/support/globalconfig"

configv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -47,7 +48,12 @@ func NewIngressParams(hcp *hyperv1.HostedControlPlane) *IngressParams {
loadBalancerScope = v1.InternalLoadBalancer
}
}
// For self-managed Azure clusters, set internal LB scope when topology
// requires private connectivity. ARO HCP clusters are excluded because
// they use shared ingress — the guest cluster's ingress operator should
// not create a per-cluster internal LB that bypasses the shared router.
if hcp.Spec.Platform.Type == hyperv1.AzurePlatform && hcp.Spec.Platform.Azure != nil &&
!azureutil.IsAroHCPByHCP(hcp) &&
(hcp.Spec.Platform.Azure.Topology == hyperv1.AzureTopologyPrivate ||
hcp.Spec.Platform.Azure.Topology == hyperv1.AzureTopologyPublicAndPrivate) {
loadBalancerScope = v1.InternalLoadBalancer
Comment on lines +51 to 59

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Expand Down