Services updates (LoadBalancerConfig and NodePortConfig) (#329)

* updates to services

- added loadBalancerConfig
- removed service-port
- added logic to not expose services

* Refactor cluster tests to improve readability and maintainability

- Simplified service port expectations by directly accessing elements instead of using `ContainElement`.
- Enhanced clarity of test assertions for `k3s-server-port` and `k3s-etcd-port` attributes.
- Removed redundant code for checking service ports.

* fix ports for ingress expose, update kubeconfig generate

pkg/controller/cluster/agent/virtual.go

@@ -72,7 +72,7 @@ func (v *VirtualAgent) config(ctx context.Context) error {
 }
 
 func virtualAgentData(serviceIP, token string) string {
-	return fmt.Sprintf(`server: https://%s:6443
+	return fmt.Sprintf(`server: https://%s
 token: %s
 with-node-id: true`, serviceIP, token)
 }

pkg/controller/cluster/agent/virtual_test.go

@@ -25,7 +25,7 @@ func Test_virtualAgentData(t *testing.T) {
 				token:     "dnjklsdjnksd892389238",
 			},
 			expectedData: map[string]string{
-				"server":       "https://10.0.0.21:6443",
+				"server":       "https://10.0.0.21",
 				"token":        "dnjklsdjnksd892389238",
 				"with-node-id": "true",
 			},

pkg/controller/cluster/cluster.go

@@ -84,6 +84,7 @@ func Add(ctx context.Context, mgr manager.Manager, sharedAgentImage, sharedAgent
 			MaxConcurrentReconciles: maxConcurrentReconciles,
 		}).
 		Owns(&apps.StatefulSet{}).
+		Owns(&v1.Service{}).
 		Complete(&reconciler)
 }
 

pkg/controller/cluster/cluster_test.go

@@ -92,13 +92,37 @@ var _ = Describe("Cluster Controller", Label("controller"), Label("Cluster"), fu
 				Expect(spec.Ingress).To(Equal([]networkingv1.NetworkPolicyIngressRule{{}}))
 			})
 
-			When("exposing the cluster with nodePort and custom ports", func() {
-				It("will have a NodePort service with the specified port exposed", func() {
+			When("exposing the cluster with nodePort", func() {
+				It("will have a NodePort service", func() {
+					cluster.Spec.Expose = &v1alpha1.ExposeConfig{
+						NodePort: &v1alpha1.NodePortConfig{},
+					}
+
+					err := k8sClient.Update(ctx, cluster)
+					Expect(err).To(Not(HaveOccurred()))
+
+					var service v1.Service
+
+					Eventually(func() v1.ServiceType {
+						serviceKey := client.ObjectKey{
+							Name:      server.ServiceName(cluster.Name),
+							Namespace: cluster.Namespace,
+						}
+
+						err := k8sClient.Get(ctx, serviceKey, &service)
+						Expect(client.IgnoreNotFound(err)).To(Not(HaveOccurred()))
+						return service.Spec.Type
+					}).
+						WithTimeout(time.Second * 30).
+						WithPolling(time.Second).
+						Should(Equal(v1.ServiceTypeNodePort))
+				})
+
+				It("will have the specified ports exposed when specified", func() {
 					cluster.Spec.Expose = &v1alpha1.ExposeConfig{
 						NodePort: &v1alpha1.NodePortConfig{
-							ServerPort:  ptr.To[int32](30010),
-							ServicePort: ptr.To[int32](30011),
-							ETCDPort:    ptr.To[int32](30012),
+							ServerPort: ptr.To[int32](30010),
+							ETCDPort:   ptr.To[int32](30011),
 						},
 					}
 
@@ -123,29 +147,95 @@ var _ = Describe("Cluster Controller", Label("controller"), Label("Cluster"), fu
 
 					servicePorts := service.Spec.Ports
 					Expect(servicePorts).NotTo(BeEmpty())
-					Expect(servicePorts).To(HaveLen(3))
+					Expect(servicePorts).To(HaveLen(2))
 
-					Expect(servicePorts).To(ContainElement(
-						And(
-							HaveField("Name", "k3s-server-port"),
-							HaveField("Port", BeEquivalentTo(6443)),
-							HaveField("NodePort", BeEquivalentTo(30010)),
-						),
-					))
-					Expect(servicePorts).To(ContainElement(
-						And(
-							HaveField("Name", "k3s-service-port"),
-							HaveField("Port", BeEquivalentTo(443)),
-							HaveField("NodePort", BeEquivalentTo(30011)),
-						),
-					))
-					Expect(servicePorts).To(ContainElement(
-						And(
-							HaveField("Name", "k3s-etcd-port"),
-							HaveField("Port", BeEquivalentTo(2379)),
-							HaveField("NodePort", BeEquivalentTo(30012)),
-						),
-					))
+					serverPort := servicePorts[0]
+					Expect(serverPort.Name).To(Equal("k3s-server-port"))
+					Expect(serverPort.Port).To(BeEquivalentTo(443))
+					Expect(serverPort.NodePort).To(BeEquivalentTo(30010))
+
+					etcdPort := servicePorts[1]
+					Expect(etcdPort.Name).To(Equal("k3s-etcd-port"))
+					Expect(etcdPort.Port).To(BeEquivalentTo(2379))
+					Expect(etcdPort.NodePort).To(BeEquivalentTo(30011))
+				})
+
+				It("will not expose the port when out of range", func() {
+					cluster.Spec.Expose = &v1alpha1.ExposeConfig{
+						NodePort: &v1alpha1.NodePortConfig{
+							ETCDPort: ptr.To[int32](2222),
+						},
+					}
+
+					err := k8sClient.Update(ctx, cluster)
+					Expect(err).To(Not(HaveOccurred()))
+
+					var service v1.Service
+
+					Eventually(func() v1.ServiceType {
+						serviceKey := client.ObjectKey{
+							Name:      server.ServiceName(cluster.Name),
+							Namespace: cluster.Namespace,
+						}
+
+						err := k8sClient.Get(ctx, serviceKey, &service)
+						Expect(client.IgnoreNotFound(err)).To(Not(HaveOccurred()))
+						return service.Spec.Type
+					}).
+						WithTimeout(time.Second * 30).
+						WithPolling(time.Second).
+						Should(Equal(v1.ServiceTypeNodePort))
+
+					servicePorts := service.Spec.Ports
+					Expect(servicePorts).NotTo(BeEmpty())
+					Expect(servicePorts).To(HaveLen(1))
+
+					serverPort := servicePorts[0]
+					Expect(serverPort.Name).To(Equal("k3s-server-port"))
+					Expect(serverPort.Port).To(BeEquivalentTo(443))
+					Expect(serverPort.TargetPort.IntValue()).To(BeEquivalentTo(6443))
+				})
+
+			})
+
+			When("exposing the cluster with loadbalancer", func() {
+				It("will have a LoadBalancer service with the default ports exposed", func() {
+					cluster.Spec.Expose = &v1alpha1.ExposeConfig{
+						LoadBalancer: &v1alpha1.LoadBalancerConfig{},
+					}
+
+					err := k8sClient.Update(ctx, cluster)
+					Expect(err).To(Not(HaveOccurred()))
+
+					var service v1.Service
+
+					Eventually(func() v1.ServiceType {
+						serviceKey := client.ObjectKey{
+							Name:      server.ServiceName(cluster.Name),
+							Namespace: cluster.Namespace,
+						}
+
+						err := k8sClient.Get(ctx, serviceKey, &service)
+						Expect(client.IgnoreNotFound(err)).To(Not(HaveOccurred()))
+						return service.Spec.Type
+					}).
+						WithTimeout(time.Second * 30).
+						WithPolling(time.Second).
+						Should(Equal(v1.ServiceTypeLoadBalancer))
+
+					servicePorts := service.Spec.Ports
+					Expect(servicePorts).NotTo(BeEmpty())
+					Expect(servicePorts).To(HaveLen(2))
+
+					serverPort := servicePorts[0]
+					Expect(serverPort.Name).To(Equal("k3s-server-port"))
+					Expect(serverPort.Port).To(BeEquivalentTo(443))
+					Expect(serverPort.TargetPort.IntValue()).To(BeEquivalentTo(6443))
+
+					etcdPort := servicePorts[1]
+					Expect(etcdPort.Name).To(Equal("k3s-etcd-port"))
+					Expect(etcdPort.Port).To(BeEquivalentTo(2379))
+					Expect(etcdPort.TargetPort.IntValue()).To(BeEquivalentTo(2379))
 				})
 			})
 		})

pkg/controller/cluster/server/bootstrap/bootstrap.go

@@ -51,7 +51,7 @@ func GenerateBootstrapData(ctx context.Context, cluster *v1alpha1.Cluster, ip, t
 }
 
 func requestBootstrap(token, serverIP string) (*ControlRuntimeBootstrap, error) {
-	url := "https://" + serverIP + ":6443/v1-k3s/server-bootstrap"
+	url := "https://" + serverIP + "/v1-k3s/server-bootstrap"
 
 	client := http.Client{
 		Transport: &http.Transport{

pkg/controller/cluster/server/config.go

@@ -39,7 +39,7 @@ func (s *Server) Config(init bool, serviceIP string) (*v1.Secret, error) {
 }
 
 func serverConfigData(serviceIP string, cluster *v1alpha1.Cluster, token string) string {
-	return "cluster-init: true\nserver: https://" + serviceIP + ":6443\n" + serverOptions(cluster, token)
+	return "cluster-init: true\nserver: https://" + serviceIP + "\n" + serverOptions(cluster, token)
 }
 
 func initConfigData(cluster *v1alpha1.Cluster, token string) string {

pkg/controller/cluster/server/ingress.go

@@ -11,9 +11,9 @@ import (
 )
 
 const (
-	servicePort = 443
-	serverPort  = 6443
-	etcdPort    = 2379
+	httpsPort     = 443
+	k3sServerPort = 6443
+	etcdPort      = 2379
 )
 
 func IngressName(clusterName string) string {
@@ -64,7 +64,7 @@ func ingressRules(cluster *v1alpha1.Cluster) []networkingv1.IngressRule {
 			Service: &networkingv1.IngressServiceBackend{
 				Name: ServiceName(cluster.Name),
 				Port: networkingv1.ServiceBackendPort{
-					Number: serverPort,
+					Number: httpsPort,
 				},
 			},
 		},

pkg/controller/cluster/server/server.go

@@ -24,8 +24,6 @@ const (
 	serverName         = "server"
 	configName         = "server-config"
 	initConfigName     = "init-server-config"
-
-	ServerPort = 6443
 )
 
 // Server

pkg/controller/cluster/server/service.go

@@ -19,7 +19,6 @@ func Service(cluster *v1alpha1.Cluster) *v1.Service {
 			Namespace: cluster.Namespace,
 		},
 		Spec: v1.ServiceSpec{
-			Type: v1.ServiceTypeClusterIP,
 			Selector: map[string]string{
 				"cluster": cluster.Name,
 				"role":    "server",
@@ -28,16 +27,10 @@ func Service(cluster *v1alpha1.Cluster) *v1.Service {
 	}
 
 	k3sServerPort := v1.ServicePort{
-		Name:     "k3s-server-port",
-		Protocol: v1.ProtocolTCP,
-		Port:     serverPort,
-	}
-
-	k3sServicePort := v1.ServicePort{
-		Name:       "k3s-service-port",
+		Name:       "k3s-server-port",
 		Protocol:   v1.ProtocolTCP,
-		Port:       servicePort,
-		TargetPort: intstr.FromInt(serverPort),
+		Port:       httpsPort,
+		TargetPort: intstr.FromInt(k3sServerPort),
 	}
 
 	etcdPort := v1.ServicePort{
@@ -46,35 +39,90 @@ func Service(cluster *v1alpha1.Cluster) *v1.Service {
 		Port:     etcdPort,
 	}
 
+	// If no expose is specified, default to ClusterIP
+	if cluster.Spec.Expose == nil {
+		service.Spec.Type = v1.ServiceTypeClusterIP
+		service.Spec.Ports = append(service.Spec.Ports, k3sServerPort, etcdPort)
+	}
+
+	// If expose is specified, set the type to the appropriate type
 	if cluster.Spec.Expose != nil {
-		nodePortConfig := cluster.Spec.Expose.NodePort
-		if nodePortConfig != nil {
+		expose := cluster.Spec.Expose
+
+		// ingress
+		if expose.Ingress != nil {
+			service.Spec.Type = v1.ServiceTypeClusterIP
+			service.Spec.Ports = append(service.Spec.Ports, k3sServerPort, etcdPort)
+		}
+
+		// loadbalancer
+		if expose.LoadBalancer != nil {
+			service.Spec.Type = v1.ServiceTypeLoadBalancer
+			addLoadBalancerPorts(service, *expose.LoadBalancer, k3sServerPort, etcdPort)
+		}
+
+		// nodeport
+		if expose.NodePort != nil {
 			service.Spec.Type = v1.ServiceTypeNodePort
-
-			if nodePortConfig.ServerPort != nil {
-				k3sServerPort.NodePort = *nodePortConfig.ServerPort
-			}
-
-			if nodePortConfig.ServicePort != nil {
-				k3sServicePort.NodePort = *nodePortConfig.ServicePort
-			}
-
-			if nodePortConfig.ETCDPort != nil {
-				etcdPort.NodePort = *nodePortConfig.ETCDPort
-			}
+			addNodePortPorts(service, *expose.NodePort, k3sServerPort, etcdPort)
 		}
 	}
 
-	service.Spec.Ports = append(
-		service.Spec.Ports,
-		k3sServicePort,
-		etcdPort,
-		k3sServerPort,
-	)
-
 	return service
 }
 
+// addLoadBalancerPorts adds the load balancer ports to the service
+func addLoadBalancerPorts(service *v1.Service, loadbalancerConfig v1alpha1.LoadBalancerConfig, k3sServerPort, etcdPort v1.ServicePort) {
+	// If the server port is not specified, use the default port
+	if loadbalancerConfig.ServerPort == nil {
+		service.Spec.Ports = append(service.Spec.Ports, k3sServerPort)
+	} else if *loadbalancerConfig.ServerPort > 0 {
+		// If the server port is specified, set the port, otherwise the service will not be exposed
+		k3sServerPort.Port = *loadbalancerConfig.ServerPort
+		service.Spec.Ports = append(service.Spec.Ports, k3sServerPort)
+	}
+
+	// If the etcd port is not specified, use the default port
+	if loadbalancerConfig.ETCDPort == nil {
+		service.Spec.Ports = append(service.Spec.Ports, etcdPort)
+	} else if *loadbalancerConfig.ETCDPort > 0 {
+		// If the etcd port is specified, set the port, otherwise the service will not be exposed
+		etcdPort.Port = *loadbalancerConfig.ETCDPort
+		service.Spec.Ports = append(service.Spec.Ports, etcdPort)
+	}
+}
+
+// addNodePortPorts adds the node port ports to the service
+func addNodePortPorts(service *v1.Service, nodePortConfig v1alpha1.NodePortConfig, k3sServerPort, etcdPort v1.ServicePort) {
+	// If the server port is not specified Kubernetes will set the node port to a random port between 30000-32767
+	if nodePortConfig.ServerPort == nil {
+		service.Spec.Ports = append(service.Spec.Ports, k3sServerPort)
+	} else {
+		serverNodePort := *nodePortConfig.ServerPort
+
+		// If the server port is in the range of 30000-32767, set the node port
+		// otherwise the service will not be exposed
+		if serverNodePort >= 30000 && serverNodePort <= 32767 {
+			k3sServerPort.NodePort = serverNodePort
+			service.Spec.Ports = append(service.Spec.Ports, k3sServerPort)
+		}
+	}
+
+	// If the etcd port is not specified Kubernetes will set the node port to a random port between 30000-32767
+	if nodePortConfig.ETCDPort == nil {
+		service.Spec.Ports = append(service.Spec.Ports, etcdPort)
+	} else {
+		etcdNodePort := *nodePortConfig.ETCDPort
+
+		// If the etcd port is in the range of 30000-32767, set the node port
+		// otherwise the service will not be exposed
+		if etcdNodePort >= 30000 && etcdNodePort <= 32767 {
+			etcdPort.NodePort = etcdNodePort
+			service.Spec.Ports = append(service.Spec.Ports, etcdPort)
+		}
+	}
+}
+
 func (s *Server) StatefulServerService() *v1.Service {
 	return &v1.Service{
 		TypeMeta: metav1.TypeMeta{
@@ -94,15 +142,10 @@ func (s *Server) StatefulServerService() *v1.Service {
 			},
 			Ports: []v1.ServicePort{
 				{
-					Name:     "k3s-server-port",
-					Protocol: v1.ProtocolTCP,
-					Port:     serverPort,
-				},
-				{
-					Name:       "k3s-service-port",
+					Name:       "k3s-server-port",
 					Protocol:   v1.ProtocolTCP,
-					Port:       servicePort,
-					TargetPort: intstr.FromInt(serverPort),
+					Port:       httpsPort,
+					TargetPort: intstr.FromInt(k3sServerPort),
 				},
 				{
 					Name:     "k3s-etcd-port",

pkg/controller/kubeconfig/kubeconfig.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/x509"
 	"fmt"
+	"slices"
 	"time"
 
 	certutil "github.com/rancher/dynamiclistener/cert"
@@ -12,6 +13,7 @@ import (
 	"github.com/rancher/k3k/pkg/controller/certs"
 	"github.com/rancher/k3k/pkg/controller/cluster/server"
 	"github.com/rancher/k3k/pkg/controller/cluster/server/bootstrap"
+	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -101,15 +103,41 @@ func getURLFromService(ctx context.Context, client client.Client, cluster *v1alp
 		return "", err
 	}
 
-	url := fmt.Sprintf("https://%s:%d", k3kService.Spec.ClusterIP, server.ServerPort)
+	ip := k3kService.Spec.ClusterIP
+	port := int32(443)
 
-	if k3kService.Spec.Type == v1.ServiceTypeNodePort {
-		nodePort := k3kService.Spec.Ports[0].NodePort
-		url = fmt.Sprintf("https://%s:%d", hostServerIP, nodePort)
+	switch k3kService.Spec.Type {
+	case v1.ServiceTypeNodePort:
+		ip = hostServerIP
+		port = k3kService.Spec.Ports[0].NodePort
+	case v1.ServiceTypeLoadBalancer:
+		ip = k3kService.Status.LoadBalancer.Ingress[0].IP
+		port = k3kService.Spec.Ports[0].Port
 	}
 
-	expose := cluster.Spec.Expose
-	if expose != nil && expose.Ingress != nil {
+	if !slices.Contains(cluster.Status.TLSSANs, ip) {
+		logrus.Warnf("ip %s not in tlsSANs", ip)
+
+		if len(cluster.Spec.TLSSANs) > 0 {
+			logrus.Warnf("Using the first TLS SAN in the spec as a fallback: %s", cluster.Spec.TLSSANs[0])
+
+			ip = cluster.Spec.TLSSANs[0]
+		} else if len(cluster.Status.TLSSANs) > 0 {
+			logrus.Warnf("No explicit tlsSANs specified. Trying to use the first TLS SAN in the status: %s", cluster.Status.TLSSANs[0])
+
+			ip = cluster.Status.TLSSANs[0]
+		} else {
+			logrus.Warn("ip not found in tlsSANs. This could cause issue with the certificate validation.")
+		}
+	}
+
+	url := "https://" + ip
+	if port != 443 {
+		url = fmt.Sprintf("%s:%d", url, port)
+	}
+
+	// if ingress is specified, use the ingress host
+	if cluster.Spec.Expose != nil && cluster.Spec.Expose.Ingress != nil {
 		var k3kIngress networkingv1.Ingress
 
 		ingressKey := types.NamespacedName{