From 8ef2947370fffb56e661278f8b1c14b46b2a6171 Mon Sep 17 00:00:00 2001 From: Adam Mihelcsik <18672841+mihivagyok@users.noreply.github.com> Date: Thu, 18 Jun 2026 14:00:58 +0200 Subject: [PATCH 1/2] fix(cpo): use in-cluster URL for oauth-server MasterURL oauth-openshift performs a self-call using the MasterURL when users click 'Display Token'. This can fail depending on whether the URL is public or private. Public URL: fails when outbound connectivity is restricted. Private URL: may fail due to private DNS resolution issues. Use the in-cluster service URL to ensure reliable self-calls. --- .../hostedcontrolplane/pki/oauth.go | 2 +- ...eComponents_oauth_openshift_configmap.yaml | 2 +- ...Components_oauth_openshift_deployment.yaml | 4 +- ...eComponents_oauth_openshift_configmap.yaml | 2 +- ...Components_oauth_openshift_deployment.yaml | 4 +- ...eComponents_oauth_openshift_configmap.yaml | 2 +- ...Components_oauth_openshift_deployment.yaml | 4 +- ...eComponents_oauth_openshift_configmap.yaml | 2 +- ...Components_oauth_openshift_deployment.yaml | 4 +- ...eComponents_oauth_openshift_configmap.yaml | 2 +- ...Components_oauth_openshift_deployment.yaml | 4 +- .../v2/assets/oauth-openshift/deployment.yaml | 2 - .../hostedcontrolplane/v2/oauth/config.go | 13 +- .../v2/oauth/config_test.go | 161 ++++++++++++------ .../hostedcontrolplane/v2/oauth/deployment.go | 16 +- .../v2/oauth/deployment_test.go | 118 +++++++++++++ 16 files changed, 259 insertions(+), 83 deletions(-) create mode 100644 control-plane-operator/controllers/hostedcontrolplane/v2/oauth/deployment_test.go diff --git a/control-plane-operator/controllers/hostedcontrolplane/pki/oauth.go b/control-plane-operator/controllers/hostedcontrolplane/pki/oauth.go index 1550559a1362..0d5a68817208 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/pki/oauth.go +++ b/control-plane-operator/controllers/hostedcontrolplane/pki/oauth.go @@ -15,7 +15,7 @@ func ReconcileOAuthServerCert(secret, ca *corev1.Secret, ownerRef config.OwnerRe if oauthIP != nil { ips = append(ips, externalOAuthAddress) } else { - dnsNames = append(dnsNames, externalOAuthAddress) + dnsNames = append(dnsNames, externalOAuthAddress, "oauth-openshift."+secret.GetNamespace()+".svc.cluster.local") } return reconcileSignedCertWithAddresses(secret, ca, ownerRef, "openshift-oauth", []string{"openshift"}, X509UsageClientServerAuth, dnsNames, ips) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/AROSwift/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/AROSwift/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml index a786203b9c0b..d4aedc500ac0 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/AROSwift/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/AROSwift/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml @@ -33,7 +33,7 @@ data: loginURL: https://:0 masterCA: /etc/kubernetes/certs/master-ca/ca.crt masterPublicURL: https://:0 - masterURL: https://:0 + masterURL: https://oauth-openshift.hcp-namespace.svc.cluster.local:6443 sessionConfig: sessionMaxAgeSeconds: 300 sessionName: ssn diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/AROSwift/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/AROSwift/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml index 42d593217228..2420d63cac86 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/AROSwift/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/AROSwift/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml @@ -28,7 +28,7 @@ spec: metadata: annotations: cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: logs,tmp-dir - component.hypershift.openshift.io/config-hash: 19dc307e3b105d444ebc1fdd60b54b1d741638a5a0dd232cbcc341c5 + component.hypershift.openshift.io/config-hash: 19dc307e3b105d44458a71d460b54b1d741638a5a0dd232cbcc341c5 hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64 labels: app: oauth-openshift @@ -95,7 +95,7 @@ spec: - name: ALL_PROXY value: socks5://127.0.0.1:8090 - name: NO_PROXY - value: kube-apiserver,audit-webhook + value: kube-apiserver,audit-webhook,oauth-openshift.hcp-namespace.svc.cluster.local image: oauth-server imagePullPolicy: IfNotPresent livenessProbe: diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/GCP/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/GCP/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml index a786203b9c0b..d4aedc500ac0 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/GCP/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/GCP/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml @@ -33,7 +33,7 @@ data: loginURL: https://:0 masterCA: /etc/kubernetes/certs/master-ca/ca.crt masterPublicURL: https://:0 - masterURL: https://:0 + masterURL: https://oauth-openshift.hcp-namespace.svc.cluster.local:6443 sessionConfig: sessionMaxAgeSeconds: 300 sessionName: ssn diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/GCP/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/GCP/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml index 3b98ee0a7138..2a66023229b4 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/GCP/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/GCP/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml @@ -28,7 +28,7 @@ spec: metadata: annotations: cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: logs,tmp-dir - component.hypershift.openshift.io/config-hash: 19dc307e3b105d444ebc1fdd60b54b1d741638a5a0dd232cbcc341c5 + component.hypershift.openshift.io/config-hash: 19dc307e3b105d44458a71d460b54b1d741638a5a0dd232cbcc341c5 hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64 labels: app: oauth-openshift @@ -95,7 +95,7 @@ spec: - name: ALL_PROXY value: socks5://127.0.0.1:8090 - name: NO_PROXY - value: kube-apiserver,audit-webhook + value: kube-apiserver,audit-webhook,oauth-openshift.hcp-namespace.svc.cluster.local image: oauth-server imagePullPolicy: IfNotPresent livenessProbe: diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/IBMCloud/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/IBMCloud/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml index a786203b9c0b..d4aedc500ac0 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/IBMCloud/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/IBMCloud/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml @@ -33,7 +33,7 @@ data: loginURL: https://:0 masterCA: /etc/kubernetes/certs/master-ca/ca.crt masterPublicURL: https://:0 - masterURL: https://:0 + masterURL: https://oauth-openshift.hcp-namespace.svc.cluster.local:6443 sessionConfig: sessionMaxAgeSeconds: 300 sessionName: ssn diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/IBMCloud/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/IBMCloud/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml index 430aa1c80ccb..5119983c3132 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/IBMCloud/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/IBMCloud/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml @@ -28,7 +28,7 @@ spec: metadata: annotations: cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: logs,tmp-dir - component.hypershift.openshift.io/config-hash: 19dc307e3b105d444ebc1fdd60b54b1d741638a5a0dd232cbcc341c5 + component.hypershift.openshift.io/config-hash: 19dc307e3b105d44458a71d460b54b1d741638a5a0dd232cbcc341c5 hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64 labels: app: oauth-openshift @@ -95,7 +95,7 @@ spec: - name: ALL_PROXY value: socks5://127.0.0.1:8090 - name: NO_PROXY - value: kube-apiserver,audit-webhook,iam.cloud.ibm.com,iam.test.cloud.ibm.com + value: kube-apiserver,audit-webhook,oauth-openshift.hcp-namespace.svc.cluster.local,iam.cloud.ibm.com,iam.test.cloud.ibm.com image: oauth-server imagePullPolicy: IfNotPresent livenessProbe: diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml index a786203b9c0b..d4aedc500ac0 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml @@ -33,7 +33,7 @@ data: loginURL: https://:0 masterCA: /etc/kubernetes/certs/master-ca/ca.crt masterPublicURL: https://:0 - masterURL: https://:0 + masterURL: https://oauth-openshift.hcp-namespace.svc.cluster.local:6443 sessionConfig: sessionMaxAgeSeconds: 300 sessionName: ssn diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml index 42d593217228..2420d63cac86 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml @@ -28,7 +28,7 @@ spec: metadata: annotations: cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: logs,tmp-dir - component.hypershift.openshift.io/config-hash: 19dc307e3b105d444ebc1fdd60b54b1d741638a5a0dd232cbcc341c5 + component.hypershift.openshift.io/config-hash: 19dc307e3b105d44458a71d460b54b1d741638a5a0dd232cbcc341c5 hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64 labels: app: oauth-openshift @@ -95,7 +95,7 @@ spec: - name: ALL_PROXY value: socks5://127.0.0.1:8090 - name: NO_PROXY - value: kube-apiserver,audit-webhook + value: kube-apiserver,audit-webhook,oauth-openshift.hcp-namespace.svc.cluster.local image: oauth-server imagePullPolicy: IfNotPresent livenessProbe: diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml index a786203b9c0b..d4aedc500ac0 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/zz_fixture_TestControlPlaneComponents_oauth_openshift_configmap.yaml @@ -33,7 +33,7 @@ data: loginURL: https://:0 masterCA: /etc/kubernetes/certs/master-ca/ca.crt masterPublicURL: https://:0 - masterURL: https://:0 + masterURL: https://oauth-openshift.hcp-namespace.svc.cluster.local:6443 sessionConfig: sessionMaxAgeSeconds: 300 sessionName: ssn diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml index 42d593217228..2420d63cac86 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/oauth-openshift/zz_fixture_TestControlPlaneComponents_oauth_openshift_deployment.yaml @@ -28,7 +28,7 @@ spec: metadata: annotations: cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: logs,tmp-dir - component.hypershift.openshift.io/config-hash: 19dc307e3b105d444ebc1fdd60b54b1d741638a5a0dd232cbcc341c5 + component.hypershift.openshift.io/config-hash: 19dc307e3b105d44458a71d460b54b1d741638a5a0dd232cbcc341c5 hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64 labels: app: oauth-openshift @@ -95,7 +95,7 @@ spec: - name: ALL_PROXY value: socks5://127.0.0.1:8090 - name: NO_PROXY - value: kube-apiserver,audit-webhook + value: kube-apiserver,audit-webhook,oauth-openshift.hcp-namespace.svc.cluster.local image: oauth-server imagePullPolicy: IfNotPresent livenessProbe: diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/assets/oauth-openshift/deployment.yaml b/control-plane-operator/controllers/hostedcontrolplane/v2/assets/oauth-openshift/deployment.yaml index 5a5cca7848ee..5f3ba5cdced7 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/assets/oauth-openshift/deployment.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/assets/oauth-openshift/deployment.yaml @@ -33,8 +33,6 @@ spec: value: http://127.0.0.1:8092 - name: ALL_PROXY value: socks5://127.0.0.1:8090 - - name: NO_PROXY - value: kube-apiserver,audit-webhook image: oauth-server imagePullPolicy: IfNotPresent livenessProbe: diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config.go b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config.go index 1826da8b4df5..5cdabd60784d 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config.go @@ -9,6 +9,8 @@ import ( "strings" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/oauth" "github.com/openshift/hypershift/support/api" "github.com/openshift/hypershift/support/config" component "github.com/openshift/hypershift/support/controlplane-component" @@ -38,6 +40,15 @@ type ConfigOverride struct { Challenge *bool `json:"challenge,omitempty"` } +// getOAuthServiceDNS returns the internal cluster DNS name for the OAuth service +// in the given namespace. +func getOAuthServiceDNS(namespace string) string { + if namespace == "" { + return "" + } + return manifests.OauthServerService("").Name + "." + namespace + ".svc.cluster.local" +} + func adaptAuditConfig(cpContext component.WorkloadContext, cm *corev1.ConfigMap) error { auditConfig := cpContext.HCP.Spec.Configuration.GetAuditPolicyConfig() cm.Data[auditPolicyProfileMapKey] = string(auditConfig.Profile) @@ -76,7 +87,7 @@ func adaptOAuthConfig(cpContext component.WorkloadContext, cfg *osinv1.OsinServe Host: net.JoinHostPort(cpContext.InfraStatus.OAuthHost, strconv.Itoa(int(cpContext.InfraStatus.OAuthPort))), }).String() controlPlaneEndpoint := cpContext.HCP.Status.ControlPlaneEndpoint - cfg.OAuthConfig.MasterURL = masterUrl + cfg.OAuthConfig.MasterURL = fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(cpContext.HCP.Namespace), oauth.OAuthServerPort) cfg.OAuthConfig.MasterPublicURL = masterUrl loginHost := controlPlaneEndpoint.Host diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config_test.go b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config_test.go index 7bc31172e059..4e9b13208181 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config_test.go @@ -1,9 +1,11 @@ package oauth import ( + "fmt" "testing" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/infra" @@ -12,74 +14,119 @@ import ( osinv1 "github.com/openshift/api/osin/v1" ) +func TestGetOAuthServiceDNS(t *testing.T) { + t.Parallel() + tests := []struct { + name string + namespace string + expected string + }{ + { + name: "When namespace is provided, it should return the correct OAuth service DNS", + namespace: "clusters-test-cluster", + expected: "oauth-openshift.clusters-test-cluster.svc.cluster.local", + }, + { + name: "When namespace is empty, it should return empty string", + namespace: "", + expected: "", + }, + { + name: "When namespace has special characters, it should include them in the DNS", + namespace: "test-ns-123", + expected: "oauth-openshift.test-ns-123.svc.cluster.local", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + result := getOAuthServiceDNS(tt.namespace) + g.Expect(result).To(Equal(tt.expected)) + }) + } +} + func TestAdaptOAuthConfig(t *testing.T) { + + const ( + testNamespace = "test-cluster" + testInternalServicePort = int32(6443) + ) testCases := []struct { - name string - oauthHost string - oauthPort int32 - cpEndpointHost string - cpEndpointPort int32 - kasDNSName string - loginURLOverride string - expectedLoginURL string - expectedMasterURL string + name string + oauthHost string + oauthPort int32 + cpEndpointHost string + cpEndpointPort int32 + kasDNSName string + loginURLOverride string + expectedLoginURL string + expectedMasterURL string + expectedMasterPublicURL string }{ { - name: "When no custom DNS is set, it should use the control plane endpoint for LoginURL", - oauthHost: "oauth.example.com", - oauthPort: 443, - cpEndpointHost: "api.example.com", - cpEndpointPort: 6443, - expectedLoginURL: "https://api.example.com:6443", - expectedMasterURL: "https://oauth.example.com:443", + name: "When no custom DNS is set, it should use the control plane endpoint for LoginURL", + oauthHost: "oauth.example.com", + oauthPort: 443, + cpEndpointHost: "api.example.com", + cpEndpointPort: 6443, + expectedLoginURL: "https://api.example.com:6443", + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterPublicURL: "https://oauth.example.com:443", }, { - name: "When KubeAPIServerDNSName is set, it should use the custom DNS name for LoginURL", - oauthHost: "oauth.example.com", - oauthPort: 443, - cpEndpointHost: "10.0.0.1", - cpEndpointPort: 6443, - kasDNSName: "api.custom.example.com", - expectedLoginURL: "https://api.custom.example.com:6443", - expectedMasterURL: "https://oauth.example.com:443", + name: "When KubeAPIServerDNSName is set, it should use the custom DNS name for LoginURL", + oauthHost: "oauth.example.com", + oauthPort: 443, + cpEndpointHost: "10.0.0.1", + cpEndpointPort: 6443, + kasDNSName: "api.custom.example.com", + expectedLoginURL: "https://api.custom.example.com:6443", + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterPublicURL: "https://oauth.example.com:443", }, { - name: "When control plane endpoint is an IP and no custom DNS is set, it should use the IP for LoginURL", - oauthHost: "10.0.0.2", - oauthPort: 443, - cpEndpointHost: "10.0.0.1", - cpEndpointPort: 6443, - expectedLoginURL: "https://10.0.0.1:6443", - expectedMasterURL: "https://10.0.0.2:443", + name: "When control plane endpoint is an IP and no custom DNS is set, it should use the IP for LoginURL", + oauthHost: "10.0.0.2", + oauthPort: 443, + cpEndpointHost: "10.0.0.1", + cpEndpointPort: 6443, + expectedLoginURL: "https://10.0.0.1:6443", + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterPublicURL: "https://10.0.0.2:443", }, { - name: "When login URL override annotation is set, it should take precedence over KubeAPIServerDNSName", - oauthHost: "oauth.example.com", - oauthPort: 443, - cpEndpointHost: "10.0.0.1", - cpEndpointPort: 6443, - kasDNSName: "api.custom.example.com", - loginURLOverride: "https://ibm.override.example.com:6443", - expectedLoginURL: "https://ibm.override.example.com:6443", - expectedMasterURL: "https://oauth.example.com:443", + name: "When login URL override annotation is set, it should take precedence over KubeAPIServerDNSName", + oauthHost: "oauth.example.com", + oauthPort: 443, + cpEndpointHost: "10.0.0.1", + cpEndpointPort: 6443, + kasDNSName: "api.custom.example.com", + loginURLOverride: "https://ibm.override.example.com:6443", + expectedLoginURL: "https://ibm.override.example.com:6443", + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterPublicURL: "https://oauth.example.com:443", }, { - name: "When control plane endpoint is an IPv6 address, it should bracket it in the LoginURL", - oauthHost: "oauth.example.com", - oauthPort: 443, - cpEndpointHost: "2001:db8::1", - cpEndpointPort: 6443, - expectedLoginURL: "https://[2001:db8::1]:6443", - expectedMasterURL: "https://oauth.example.com:443", + name: "When control plane endpoint is an IPv6 address, it should bracket it in the LoginURL", + oauthHost: "oauth.example.com", + oauthPort: 443, + cpEndpointHost: "2001:db8::1", + cpEndpointPort: 6443, + expectedLoginURL: "https://[2001:db8::1]:6443", + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterPublicURL: "https://oauth.example.com:443", }, { - name: "When OAuth host is an IPv6 address, it should bracket it in the MasterURL", - oauthHost: "2001:db8::2", - oauthPort: 443, - cpEndpointHost: "api.example.com", - cpEndpointPort: 6443, - expectedLoginURL: "https://api.example.com:6443", - expectedMasterURL: "https://[2001:db8::2]:443", + name: "When OAuth host is an IPv6 address, it should bracket it in the MasterURL", + oauthHost: "2001:db8::2", + oauthPort: 443, + cpEndpointHost: "api.example.com", + cpEndpointPort: 6443, + expectedLoginURL: "https://api.example.com:6443", + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterPublicURL: "https://[2001:db8::2]:443", }, } @@ -88,6 +135,10 @@ func TestAdaptOAuthConfig(t *testing.T) { g := NewWithT(t) hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: testNamespace, + }, Spec: hyperv1.HostedControlPlaneSpec{ KubeAPIServerDNSName: tc.kasDNSName, }, @@ -119,7 +170,7 @@ func TestAdaptOAuthConfig(t *testing.T) { g.Expect(cfg.OAuthConfig.LoginURL).To(Equal(tc.expectedLoginURL)) g.Expect(cfg.OAuthConfig.MasterURL).To(Equal(tc.expectedMasterURL)) - g.Expect(cfg.OAuthConfig.MasterPublicURL).To(Equal(tc.expectedMasterURL)) + g.Expect(cfg.OAuthConfig.MasterPublicURL).To(Equal(tc.expectedMasterPublicURL)) }) } } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/deployment.go b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/deployment.go index e31e102fe6c0..8df0383b02ea 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/deployment.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/deployment.go @@ -52,17 +52,15 @@ func adaptDeployment(cpContext component.WorkloadContext, deployment *appsv1.Dep }, }) } - + noProxy := []string{manifests.KubeAPIServerService("").Name, config.AuditWebhookService, getOAuthServiceDNS(cpContext.HCP.Namespace)} if cpContext.HCP.Spec.Platform.Type == hyperv1.IBMCloudPlatform { - noProxy := []string{ - manifests.KubeAPIServerService("").Name, config.AuditWebhookService, - "iam.cloud.ibm.com", "iam.test.cloud.ibm.com", - } - podspec.UpsertEnvVar(c, corev1.EnvVar{ - Name: "NO_PROXY", - Value: strings.Join(noProxy, ","), - }) + noProxy = append(noProxy, "iam.cloud.ibm.com", "iam.test.cloud.ibm.com") } + + podspec.UpsertEnvVar(c, corev1.EnvVar{ + Name: "NO_PROXY", + Value: strings.Join(noProxy, ","), + }) }) configuration := cpContext.HCP.Spec.Configuration diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/deployment_test.go b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/deployment_test.go new file mode 100644 index 000000000000..5d6cc6e31e05 --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/deployment_test.go @@ -0,0 +1,118 @@ +package oauth + +import ( + "strings" + "testing" + + . "github.com/onsi/gomega" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/assets" + "github.com/openshift/hypershift/support/api" + "github.com/openshift/hypershift/support/config" + component "github.com/openshift/hypershift/support/controlplane-component" + "github.com/openshift/hypershift/support/podspec" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestAdaptDeployment(t *testing.T) { + t.Parallel() + + const testNamespace = "clusters-test" + + tests := []struct { + name string + platformType hyperv1.PlatformType + wantNoProxy []string + }{ + { + name: "When platform is AWS, it should set NO_PROXY with kube-apiserver, audit-webhook, and oauth in-cluster DNS", + platformType: hyperv1.AWSPlatform, + wantNoProxy: []string{ + manifests.KubeAPIServerService("").Name, + config.AuditWebhookService, + getOAuthServiceDNS(testNamespace), + }, + }, + { + name: "When platform is Azure, it should set NO_PROXY with kube-apiserver, audit-webhook, and oauth in-cluster DNS", + platformType: hyperv1.AzurePlatform, + wantNoProxy: []string{ + manifests.KubeAPIServerService("").Name, + config.AuditWebhookService, + getOAuthServiceDNS(testNamespace), + }, + }, + { + name: "When platform is None, it should set NO_PROXY with kube-apiserver, audit-webhook, and oauth in-cluster DNS", + platformType: hyperv1.NonePlatform, + wantNoProxy: []string{ + manifests.KubeAPIServerService("").Name, + config.AuditWebhookService, + getOAuthServiceDNS(testNamespace), + }, + }, + { + name: "When platform is IBMCloud, it should also include IAM endpoints in NO_PROXY", + platformType: hyperv1.IBMCloudPlatform, + wantNoProxy: []string{ + manifests.KubeAPIServerService("").Name, + config.AuditWebhookService, + getOAuthServiceDNS(testNamespace), + "iam.cloud.ibm.com", + "iam.test.cloud.ibm.com", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: testNamespace, + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: tt.platformType, + }, + }, + } + + deployment, err := assets.LoadDeploymentManifest(ComponentName) + g.Expect(err).ToNot(HaveOccurred()) + + cpContext := component.WorkloadContext{ + Client: fake.NewClientBuilder().WithScheme(api.Scheme).Build(), + HCP: hcp, + } + + err = adaptDeployment(cpContext, deployment) + g.Expect(err).ToNot(HaveOccurred()) + + container := podspec.FindContainer(ComponentName, deployment.Spec.Template.Spec.Containers) + g.Expect(container).ToNot(BeNil()) + + noProxyEnv := podspec.FindEnvVar("NO_PROXY", container.Env) + g.Expect(noProxyEnv).ToNot(BeNil()) + + noProxyEntries := strings.Split(noProxyEnv.Value, ",") + for _, entry := range tt.wantNoProxy { + g.Expect(noProxyEntries).To(ContainElement(entry), "expected NO_PROXY to contain %q", entry) + } + + // Ensure no unexpected IBM entries for non-IBM platforms + if tt.platformType != hyperv1.IBMCloudPlatform { + g.Expect(noProxyEnv.Value).ToNot(ContainSubstring("iam.cloud.ibm.com")) + g.Expect(noProxyEnv.Value).ToNot(ContainSubstring("iam.test.cloud.ibm.com")) + } + }) + } +} From 284c54026b5104f937002e100cac6008186756a3 Mon Sep 17 00:00:00 2001 From: Adam Mihelcsik <18672841+mihivagyok@users.noreply.github.com> Date: Wed, 24 Jun 2026 13:07:58 +0200 Subject: [PATCH 2/2] fix(cpo): update unit tests use constants from the same package instead of creating new consts --- .../hostedcontrolplane/v2/oauth/config.go | 4 ++-- .../hostedcontrolplane/v2/oauth/config_test.go | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config.go b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config.go index 5cdabd60784d..d859f612e97c 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config.go @@ -10,7 +10,6 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" - "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/oauth" "github.com/openshift/hypershift/support/api" "github.com/openshift/hypershift/support/config" component "github.com/openshift/hypershift/support/controlplane-component" @@ -28,6 +27,7 @@ const ( auditPolicyProfileMapKey = "profile" defaultAccessTokenMaxAgeSeconds int32 = 86400 + OAuthServerPort int32 = 6443 ) // ConfigOverride defines the oauth parameters that can be overridden in special use cases. The only supported @@ -87,7 +87,7 @@ func adaptOAuthConfig(cpContext component.WorkloadContext, cfg *osinv1.OsinServe Host: net.JoinHostPort(cpContext.InfraStatus.OAuthHost, strconv.Itoa(int(cpContext.InfraStatus.OAuthPort))), }).String() controlPlaneEndpoint := cpContext.HCP.Status.ControlPlaneEndpoint - cfg.OAuthConfig.MasterURL = fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(cpContext.HCP.Namespace), oauth.OAuthServerPort) + cfg.OAuthConfig.MasterURL = fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(cpContext.HCP.Namespace), OAuthServerPort) cfg.OAuthConfig.MasterPublicURL = masterUrl loginHost := controlPlaneEndpoint.Host diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config_test.go b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config_test.go index 4e9b13208181..01e45bc5f057 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/oauth/config_test.go @@ -5,13 +5,14 @@ import ( "testing" . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/infra" component "github.com/openshift/hypershift/support/controlplane-component" osinv1 "github.com/openshift/api/osin/v1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestGetOAuthServiceDNS(t *testing.T) { @@ -50,8 +51,7 @@ func TestGetOAuthServiceDNS(t *testing.T) { func TestAdaptOAuthConfig(t *testing.T) { const ( - testNamespace = "test-cluster" - testInternalServicePort = int32(6443) + testNamespace = "test-cluster" ) testCases := []struct { name string @@ -72,7 +72,7 @@ func TestAdaptOAuthConfig(t *testing.T) { cpEndpointHost: "api.example.com", cpEndpointPort: 6443, expectedLoginURL: "https://api.example.com:6443", - expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), OAuthServerPort), expectedMasterPublicURL: "https://oauth.example.com:443", }, { @@ -83,7 +83,7 @@ func TestAdaptOAuthConfig(t *testing.T) { cpEndpointPort: 6443, kasDNSName: "api.custom.example.com", expectedLoginURL: "https://api.custom.example.com:6443", - expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), OAuthServerPort), expectedMasterPublicURL: "https://oauth.example.com:443", }, { @@ -93,7 +93,7 @@ func TestAdaptOAuthConfig(t *testing.T) { cpEndpointHost: "10.0.0.1", cpEndpointPort: 6443, expectedLoginURL: "https://10.0.0.1:6443", - expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), OAuthServerPort), expectedMasterPublicURL: "https://10.0.0.2:443", }, { @@ -105,7 +105,7 @@ func TestAdaptOAuthConfig(t *testing.T) { kasDNSName: "api.custom.example.com", loginURLOverride: "https://ibm.override.example.com:6443", expectedLoginURL: "https://ibm.override.example.com:6443", - expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), OAuthServerPort), expectedMasterPublicURL: "https://oauth.example.com:443", }, { @@ -115,7 +115,7 @@ func TestAdaptOAuthConfig(t *testing.T) { cpEndpointHost: "2001:db8::1", cpEndpointPort: 6443, expectedLoginURL: "https://[2001:db8::1]:6443", - expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), OAuthServerPort), expectedMasterPublicURL: "https://oauth.example.com:443", }, { @@ -125,7 +125,7 @@ func TestAdaptOAuthConfig(t *testing.T) { cpEndpointHost: "api.example.com", cpEndpointPort: 6443, expectedLoginURL: "https://api.example.com:6443", - expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), testInternalServicePort), + expectedMasterURL: fmt.Sprintf("https://%s:%d", getOAuthServiceDNS(testNamespace), OAuthServerPort), expectedMasterPublicURL: "https://[2001:db8::2]:443", }, }