From c34232243a70aa0b666db3f963ed5d6b1240a79b Mon Sep 17 00:00:00 2001 From: enxebre Date: Thu, 26 Oct 2023 13:49:15 +0200 Subject: [PATCH] Remove unncessary kas port exposure on service --- .../cno/clusternetworkoperator.go | 2 +- .../hostedcontrolplane_controller.go | 4 ++-- .../hostedcontrolplane_controller_test.go | 1 - .../hostedcontrolplane/ingress/router.go | 22 ++----------------- .../hostedcontrolplane/kas/params.go | 3 ++- .../controllers/nodepool/haproxy.go | 2 +- .../controllers/nodepool/haproxy_test.go | 8 +++---- ...g_private_cluster_uses_.local_address.yaml | 2 +- ...d_private_cluster_uses_.local_address.yaml | 2 +- ..._cluster_uses_address_from_kubeconfig.yaml | 2 +- support/util/networking.go | 9 -------- 11 files changed, 15 insertions(+), 42 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go b/control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go index a75313067bbf..707976eb576d 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go +++ b/control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go @@ -125,7 +125,7 @@ func NewParams(hcp *hyperv1.HostedControlPlane, version string, releaseImageProv p.DeploymentConfig.SetDefaultSecurityContext = setDefaultSecurityContext if util.IsPrivateHCP(hcp) { p.APIServerAddress = fmt.Sprintf("api.%s.hypershift.local", hcp.Name) - p.APIServerPort = util.InternalAPIPortWithDefault(hcp, config.DefaultAPIServerPort) + p.APIServerPort = 443 } else { p.APIServerAddress = hcp.Status.ControlPlaneEndpoint.Host p.APIServerPort = hcp.Status.ControlPlaneEndpoint.Port diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index e52a46cef50e..666b82055343 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1401,7 +1401,7 @@ func (r *HostedControlPlaneReconciler) reconcileHCPRouterServices(ctx context.Co if util.IsPrivateHCP(hcp) { svc := manifests.PrivateRouterService(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, svc, func() error { - return ingress.ReconcileRouterService(svc, util.InternalAPIPortWithDefault(hcp, config.DefaultAPIServerPort), true, true) + return ingress.ReconcileRouterService(svc, true, true) }); err != nil { return fmt.Errorf("failed to reconcile private router service: %w", err) } @@ -1423,7 +1423,7 @@ func (r *HostedControlPlaneReconciler) reconcileHCPRouterServices(ctx context.Co // When Public access endpoint we need to create a Service type LB external for the KAS. if util.IsPublicHCP(hcp) && exposeKASThroughRouter { if _, err := createOrUpdate(ctx, r.Client, pubSvc, func() error { - return ingress.ReconcileRouterService(pubSvc, util.InternalAPIPortWithDefault(hcp, config.DefaultAPIServerPort), false, util.IsPrivateHCP(hcp)) + return ingress.ReconcileRouterService(pubSvc, false, util.IsPrivateHCP(hcp)) }); err != nil { return fmt.Errorf("failed to reconcile router service: %w", err) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index 37b5ed26fc45..df966ea077f7 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -1361,7 +1361,6 @@ func TestReconcileHCPRouterServices(t *testing.T) { Selector: map[string]string{"app": "private-router"}, Ports: []corev1.ServicePort{ {Name: "https", Port: 443, TargetPort: intstr.FromString("https"), Protocol: corev1.ProtocolTCP}, - {Name: "kube-apiserver", Port: 6443, TargetPort: intstr.FromString("https"), Protocol: corev1.ProtocolTCP}, }, }, } diff --git a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go index 704729089def..ea969bb9214f 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go +++ b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go @@ -226,7 +226,7 @@ func buildHCPRouterContainerMain(image string) func(*corev1.Container) { } } -func ReconcileRouterService(svc *corev1.Service, kasPort int32, internal, crossZoneLoadBalancingEnabled bool) error { +func ReconcileRouterService(svc *corev1.Service, internal, crossZoneLoadBalancingEnabled bool) error { if svc.Annotations == nil { svc.Annotations = map[string]string{} } @@ -247,13 +247,7 @@ func ReconcileRouterService(svc *corev1.Service, kasPort int32, internal, crossZ svc.Spec.Type = corev1.ServiceTypeLoadBalancer svc.Spec.Selector = hcpRouterLabels() foundHTTPS := false - foundKAS := false - // TODO (alberto): why this criteria? - // Introduced here https://github.com/openshift/hypershift/pull/1614/files#diff-62c16653415b8d89921cb26796abc479c31da1654095f7c46b551b470533d66dR368-R372. - if kasPort == 443 { - foundKAS = true - } for i, port := range svc.Spec.Ports { switch port.Name { case "https": @@ -261,11 +255,6 @@ func ReconcileRouterService(svc *corev1.Service, kasPort int32, internal, crossZ svc.Spec.Ports[i].TargetPort = intstr.FromString("https") svc.Spec.Ports[i].Protocol = corev1.ProtocolTCP foundHTTPS = true - case "kube-apiserver": - svc.Spec.Ports[i].Port = kasPort - svc.Spec.Ports[i].TargetPort = intstr.FromString("https") - svc.Spec.Ports[i].Protocol = corev1.ProtocolTCP - foundKAS = true } } if !foundHTTPS { @@ -276,14 +265,7 @@ func ReconcileRouterService(svc *corev1.Service, kasPort int32, internal, crossZ Protocol: corev1.ProtocolTCP, }) } - if !foundKAS { - svc.Spec.Ports = append(svc.Spec.Ports, corev1.ServicePort{ - Name: "kube-apiserver", - Port: kasPort, - TargetPort: intstr.FromString("https"), - Protocol: corev1.ProtocolTCP, - }) - } + return nil } diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/params.go b/control-plane-operator/controllers/hostedcontrolplane/kas/params.go index 0fe8a1951e82..cdf754b15397 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/params.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/params.go @@ -382,8 +382,9 @@ func (p *KubeAPIServerParams) ExternalURL() string { return fmt.Sprintf("https://%s:%d", p.ExternalAddress, p.ExternalPort) } +// InternalURL is used by ReconcileBootstrapKubeconfigSecret. func (p *KubeAPIServerParams) InternalURL() string { - return fmt.Sprintf("https://%s:%d", p.InternalAddress, p.InternalPort) + return fmt.Sprintf("https://%s:%d", p.InternalAddress, 443) } func (p *KubeAPIServerParams) ExternalKubeconfigKey() string { diff --git a/hypershift-operator/controllers/nodepool/haproxy.go b/hypershift-operator/controllers/nodepool/haproxy.go index f17667242cc2..e7f410e237c0 100644 --- a/hypershift-operator/controllers/nodepool/haproxy.go +++ b/hypershift-operator/controllers/nodepool/haproxy.go @@ -66,7 +66,7 @@ func (r *NodePoolReconciler) reconcileHAProxyIgnitionConfig(ctx context.Context, if util.IsPrivateHC(hcluster) { apiServerExternalAddress = fmt.Sprintf("api.%s.hypershift.local", hcluster.Name) - apiServerExternalPort = util.InternalAPIPortFromHostedClusterWithDefault(hcluster, config.DefaultAPIServerPort) + apiServerExternalPort = 443 } else { if hcluster.Status.KubeConfig == nil { return "", true, nil diff --git a/hypershift-operator/controllers/nodepool/haproxy_test.go b/hypershift-operator/controllers/nodepool/haproxy_test.go index 51a36320c13f..4d18ca69cc31 100644 --- a/hypershift-operator/controllers/nodepool/haproxy_test.go +++ b/hypershift-operator/controllers/nodepool/haproxy_test.go @@ -85,7 +85,7 @@ kind: Config` hc.Spec.Networking.ServiceNetwork = []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}} }), - expectedHAProxyConfigContent: []string{"api." + hc().Name + ".hypershift.local:6443"}, + expectedHAProxyConfigContent: []string{"api." + hc().Name + ".hypershift.local:443"}, }, { name: "private cluster uses .local address and custom apiserver port", @@ -104,7 +104,7 @@ kind: Config` hc.Spec.Networking.ServiceNetwork = []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}} }), - expectedHAProxyConfigContent: []string{"api." + hc().Name + ".hypershift.local:6443"}, + expectedHAProxyConfigContent: []string{"api." + hc().Name + ".hypershift.local:443"}, }, { name: "public and private cluster uses .local address and custom apiserver port", @@ -126,11 +126,11 @@ kind: Config` other: []crclient.Object{&corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "kk", Namespace: hc().Namespace}, Data: map[string][]byte{ - "kubeconfig": []byte(kubeconfig(6443)), + "kubeconfig": []byte(kubeconfig(443)), }, }}, - expectedHAProxyConfigContent: []string{"kubeconfig-host:6443"}, + expectedHAProxyConfigContent: []string{"kubeconfig-host:443"}, }, { name: "public cluster uses address from kubeconfig and custom port", diff --git a/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_private_cluster_uses_.local_address.yaml b/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_private_cluster_uses_.local_address.yaml index b5c5981e9937..fe402d4a2316 100644 --- a/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_private_cluster_uses_.local_address.yaml +++ b/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_private_cluster_uses_.local_address.yaml @@ -26,4 +26,4 @@ backend remote_apiserver option httpchk GET /version option log-health-checks default-server inter 10s fall 3 rise 3 - server controlplane api.hc.hypershift.local:6443 + server controlplane api.hc.hypershift.local:443 diff --git a/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_public_and_private_cluster_uses_.local_address.yaml b/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_public_and_private_cluster_uses_.local_address.yaml index b5c5981e9937..fe402d4a2316 100644 --- a/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_public_and_private_cluster_uses_.local_address.yaml +++ b/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_public_and_private_cluster_uses_.local_address.yaml @@ -26,4 +26,4 @@ backend remote_apiserver option httpchk GET /version option log-health-checks default-server inter 10s fall 3 rise 3 - server controlplane api.hc.hypershift.local:6443 + server controlplane api.hc.hypershift.local:443 diff --git a/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_public_cluster_uses_address_from_kubeconfig.yaml b/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_public_cluster_uses_address_from_kubeconfig.yaml index e77ce9010540..939886eb30bb 100644 --- a/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_public_cluster_uses_address_from_kubeconfig.yaml +++ b/hypershift-operator/controllers/nodepool/testdata/zz_fixture_TestReconcileHAProxyIgnitionConfig_public_cluster_uses_address_from_kubeconfig.yaml @@ -26,4 +26,4 @@ backend remote_apiserver option httpchk GET /version option log-health-checks default-server inter 10s fall 3 rise 3 - server controlplane kubeconfig-host:6443 + server controlplane kubeconfig-host:443 diff --git a/support/util/networking.go b/support/util/networking.go index b42419409a41..caf35dd834a0 100644 --- a/support/util/networking.go +++ b/support/util/networking.go @@ -96,15 +96,6 @@ func InternalAPIPortWithDefault(hcp *hyperv1.HostedControlPlane, defaultValue in return defaultValue } -// InternalAPIPortFromHostedClusterWithDefault will retrieve the port to use to contact the APIServer over the Kubernetes service domain -// kube-apiserver.NAMESPACE.svc.cluster.local:INTERNAL_API_PORT -func InternalAPIPortFromHostedClusterWithDefault(hc *hyperv1.HostedCluster, defaultValue int32) int32 { - if hc.Spec.Networking.APIServer != nil && hc.Spec.Networking.APIServer.Port != nil { - return *hc.Spec.Networking.APIServer.Port - } - return defaultValue -} - func AdvertiseAddress(hcp *hyperv1.HostedControlPlane) *string { if hcp != nil && hcp.Spec.Networking.APIServer != nil { return hcp.Spec.Networking.APIServer.AdvertiseAddress