Refactor kubeconfig URL generation (#938)

* Refactor kubeconfig generation to remove unused port parameter and update related functions

* Refactor kubeconfig generation to streamline error handling and remove unused imports

* Refactor kubeconfig URL generation functions and deprecate old implementation

* restore old behavior

* Add 'k3kcli kubeconfig get' command and update documentation

* Refactor URL generation by removing deprecated getURLFromService function and updating tests to use new implementation

* Fix expected URL for LoadBalancer test case to include hostname

* Refactor kubeconfig test documentation to clarify URL generation behavior for ClusterIP, NodePort, and LoadBalancer service types

* Refactor kubeconfig URL generation functions to improve clarity and maintainability

* Remove deprecated 'k3kcli kubeconfig get' command and update related documentation

* Set logger to discard in NewRootCmd for improved logging control

* Refactor getURLFromService to streamline ingress key retrieval
This commit is contained in:
Enrico Candino
2026-06-26 11:16:54 +02:00
committed by GitHub
parent d008deded7
commit 244011e68d
9 changed files with 163 additions and 124 deletions
+14 -4
View File
@@ -14,7 +14,9 @@ import (
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/util/retry"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -27,7 +29,6 @@ import (
"github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1"
"github.com/rancher/k3k/pkg/controller"
k3kcluster "github.com/rancher/k3k/pkg/controller/cluster"
"github.com/rancher/k3k/pkg/controller/kubeconfig"
)
type CreateConfig struct {
@@ -174,12 +175,21 @@ func createAction(appCtx *AppContext, config *CreateConfig) func(cmd *cobra.Comm
Steps: 25,
}
cfg := kubeconfig.New()
var kubeconfig *clientcmdapi.Config
if err := retry.OnError(availableBackoff, apierrors.IsNotFound, func() error {
kubeconfig, err = cfg.Generate(ctx, client, cluster, host[0], 0)
kubeconfigSecretKey := types.NamespacedName{
Name: controller.SafeConcatNameWithPrefix(cluster.Name, "kubeconfig"),
Namespace: cluster.Namespace,
}
var kubeconfigSecret corev1.Secret
if err := client.Get(ctx, kubeconfigSecretKey, &kubeconfigSecret); err != nil {
return err
}
kubeconfig, err = clientcmd.Load(kubeconfigSecret.Data["kubeconfig.yaml"])
return err
}); err != nil {
return err
+1 -1
View File
@@ -118,7 +118,7 @@ func generate(appCtx *AppContext, cfg *GenerateKubeconfigConfig) func(cmd *cobra
var kubeconfig *clientcmdapi.Config
if err := retry.OnError(controller.Backoff, apierrors.IsNotFound, func() error {
kubeconfig, err = kubeCfg.Generate(ctx, client, &cluster, host[0], 0)
kubeconfig, err = kubeCfg.Generate(ctx, client, &cluster, host[0])
return err
}); err != nil {
return err
+4
View File
@@ -4,6 +4,7 @@ import (
"fmt"
"strings"
"github.com/go-logr/logr"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
@@ -15,6 +16,7 @@ import (
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1"
"github.com/rancher/k3k/pkg/buildinfo"
@@ -41,6 +43,8 @@ func NewRootCmd() *cobra.Command {
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
InitializeConfig(cmd)
ctrl.SetLogger(logr.Discard())
if appCtx.Debug {
logrus.SetLevel(logrus.DebugLevel)
}
+3 -3
View File
@@ -443,7 +443,7 @@ func (c *ClusterReconciler) reconcile(ctx context.Context, cluster *v1beta1.Clus
return err
}
if err := c.ensureKubeconfigSecret(ctx, cluster, serviceIP, 443); err != nil {
if err := c.ensureKubeconfigSecret(ctx, cluster, serviceIP); err != nil {
return err
}
@@ -471,13 +471,13 @@ func (c *ClusterReconciler) ensureBootstrapSecret(ctx context.Context, cluster *
}
// ensureKubeconfigSecret will create or update the Secret containing the kubeconfig data from the k3s server
func (c *ClusterReconciler) ensureKubeconfigSecret(ctx context.Context, cluster *v1beta1.Cluster, serviceIP string, port int) error {
func (c *ClusterReconciler) ensureKubeconfigSecret(ctx context.Context, cluster *v1beta1.Cluster, serviceIP string) error {
log := ctrl.LoggerFrom(ctx)
log.V(1).Info("Ensuring Kubeconfig Secret")
adminKubeconfig := kubeconfig.New()
kubeconfig, err := adminKubeconfig.Generate(ctx, c.Client, cluster, serviceIP, port)
kubeconfig, err := adminKubeconfig.Generate(ctx, c.Client, cluster, serviceIP)
if err != nil {
return err
}
+3 -3
View File
@@ -13,9 +13,9 @@ import (
)
const (
httpsPort = 443
k3sServerPort = 6443
etcdPort = 2379
httpsPort int32 = 443
k3sServerPort int32 = 6443
etcdPort int32 = 2379
)
func IngressName(clusterName string) string {
+2 -2
View File
@@ -32,7 +32,7 @@ func Service(cluster *v1beta1.Cluster) *corev1.Service {
Name: "k3s-server-port",
Protocol: corev1.ProtocolTCP,
Port: httpsPort,
TargetPort: intstr.FromInt(k3sServerPort),
TargetPort: intstr.FromInt32(k3sServerPort),
}
etcdPort := corev1.ServicePort{
@@ -142,7 +142,7 @@ func (s *Server) StatefulServerService() *corev1.Service {
Name: "k3s-server-port",
Protocol: corev1.ProtocolTCP,
Port: httpsPort,
TargetPort: intstr.FromInt(k3sServerPort),
TargetPort: intstr.FromInt32(k3sServerPort),
},
{
Name: "k3s-etcd-port",
+107 -70
View File
@@ -4,10 +4,12 @@ import (
"context"
"crypto/x509"
"fmt"
"net"
"net/url"
"slices"
"strconv"
"time"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/authentication/user"
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -16,6 +18,7 @@ import (
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
ctrl "sigs.k8s.io/controller-runtime"
"github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1"
"github.com/rancher/k3k/pkg/controller"
@@ -39,7 +42,7 @@ func New() *KubeConfig {
}
}
func (k *KubeConfig) Generate(ctx context.Context, client client.Client, cluster *v1beta1.Cluster, hostServerIP string, port int) (*clientcmdapi.Config, error) {
func (k *KubeConfig) Generate(ctx context.Context, client client.Client, cluster *v1beta1.Cluster, hostServerIP string) (*clientcmdapi.Config, error) {
bootstrapData, err := bootstrap.LoadFromSecret(ctx, client, cluster)
if err != nil {
return nil, err
@@ -60,12 +63,12 @@ func (k *KubeConfig) Generate(ctx context.Context, client client.Client, cluster
return nil, err
}
url, err := getURLFromService(ctx, client, cluster, hostServerIP, port)
serverURL, err := getURLFromService(ctx, client, cluster, hostServerIP)
if err != nil {
return nil, err
}
config := NewConfig(url, serverCACert, adminCert, adminKey)
config := NewConfig(serverURL.String(), serverCACert, adminCert, adminKey)
return config, nil
}
@@ -93,84 +96,118 @@ func NewConfig(url string, serverCA, clientCert, clientKey []byte) *clientcmdapi
return config
}
func getURLFromService(ctx context.Context, client client.Client, cluster *v1beta1.Cluster, hostServerIP string, serverPort int) (string, error) {
// get the server service to extract the right IP
// getURLFromService generates the API server URL for the kubeconfig based on the service configuration.
//
// It handles internal vs external access patterns:
// - Internal access (hostServerIP == service.ClusterIP): uses the ClusterIP for direct pod-to-pod communication
// - External access (hostServerIP != service.ClusterIP): uses the appropriate external endpoint based on the service type
//
// Service type handling:
// - ClusterIP: uses service.Spec.ClusterIP (internal-only)
// - NodePort: uses hostServerIP:NodePort for external access, ClusterIP:Port for internal access
// - LoadBalancer: uses the LoadBalancer ingress IP, falling back to its hostname
// - Ingress (if configured): takes precedence over the service-based URL
//
// The hostServerIP parameter determines the access pattern:
// - Controller reconciliation: passes service.Spec.ClusterIP → internal access
// - CLI kubeconfig export: passes the external host → external access
func getURLFromService(ctx context.Context, c client.Client, cluster *v1beta1.Cluster, hostServerIP string) (*url.URL, error) {
log := ctrl.LoggerFrom(ctx)
key := types.NamespacedName{
Name: server.ServiceName(cluster.Name),
Namespace: cluster.Namespace,
}
var k3kService corev1.Service
if err := client.Get(ctx, key, &k3kService); err != nil {
return "", err
}
ip := k3kService.Spec.ClusterIP
port := int32(443)
if len(k3kService.Spec.Ports) == 0 {
logrus.Warn("No ports exposed by the cluster service.")
}
switch k3kService.Spec.Type {
case corev1.ServiceTypeNodePort:
ip = hostServerIP
if len(k3kService.Spec.Ports) > 0 {
port = k3kService.Spec.Ports[0].NodePort
}
case corev1.ServiceTypeLoadBalancer:
if len(k3kService.Status.LoadBalancer.Ingress) > 0 {
ip = k3kService.Status.LoadBalancer.Ingress[0].IP
} else {
logrus.Warn("No ingress found in LoadBalancer service.")
}
if len(k3kService.Spec.Ports) > 0 {
port = k3kService.Spec.Ports[0].Port
}
}
if serverPort != 0 {
port = int32(serverPort)
}
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
// Check if ingress is configured
if cluster.Spec.Expose != nil && cluster.Spec.Expose.Ingress != nil {
var k3kIngress networkingv1.Ingress
ingressKey := types.NamespacedName{
key := types.NamespacedName{
Name: server.IngressName(cluster.Name),
Namespace: cluster.Namespace,
}
if err := client.Get(ctx, ingressKey, &k3kIngress); err != nil {
return "", err
var k3kIngress networkingv1.Ingress
if err := c.Get(ctx, key, &k3kIngress); err != nil {
return nil, err
}
url = fmt.Sprintf("https://%s", k3kIngress.Spec.Rules[0].Host)
if len(k3kIngress.Spec.Rules) > 0 && k3kIngress.Spec.Rules[0].Host != "" {
return url.Parse(fmt.Sprintf("https://%s", k3kIngress.Spec.Rules[0].Host))
}
log.V(1).Info("Ingress has no rule with a host set, falling back to the service URL.")
}
return url, nil
// Fall back to Service-based URL
var k3kService corev1.Service
if err := c.Get(ctx, key, &k3kService); err != nil {
return nil, err
}
// init to hostServerIP and 443 port
ip := hostServerIP
port := int32(443)
// Use service port as default if available
if len(k3kService.Spec.Ports) > 0 {
port = k3kService.Spec.Ports[0].Port
}
// Handle each service type separately
switch k3kService.Spec.Type {
case corev1.ServiceTypeClusterIP:
ip = k3kService.Spec.ClusterIP
case corev1.ServiceTypeNodePort:
// Only use NodePort if hostServerIP is NOT the ClusterIP
// If hostServerIP == ClusterIP, this is an internal connection, use ClusterIP
if hostServerIP != k3kService.Spec.ClusterIP {
if len(k3kService.Spec.Ports) > 0 {
port = k3kService.Spec.Ports[0].NodePort
}
} else {
// Internal connection: use ClusterIP
ip = k3kService.Spec.ClusterIP
}
case corev1.ServiceTypeLoadBalancer:
if len(k3kService.Status.LoadBalancer.Ingress) > 0 {
ingress := k3kService.Status.LoadBalancer.Ingress[0]
switch {
case ingress.IP != "":
ip = ingress.IP
case ingress.Hostname != "":
ip = ingress.Hostname
default:
log.V(1).Info("No usable ingress address found in LoadBalancer service.")
}
}
}
if !slices.Contains(cluster.Status.TLSSANs, ip) {
log.V(1).Info(fmt.Sprintf("IP %s not in tlsSANs.", ip))
if len(cluster.Spec.TLSSANs) > 0 {
log.V(1).Info("Using the first TLS SAN in the spec as a fallback: " + cluster.Spec.TLSSANs[0])
ip = cluster.Spec.TLSSANs[0]
} else if len(cluster.Status.TLSSANs) > 0 {
log.V(1).Info("No explicit tlsSANs specified. Trying to use the first TLS SAN in the status: " + cluster.Status.TLSSANs[0])
ip = cluster.Status.TLSSANs[0]
} else {
log.V(1).Info("IP not found in tlsSANs. This could cause issue with the certificate validation.")
}
}
// Build URL with port only if not the default HTTPS port
var rawURL string
if port != int32(443) {
rawURL = fmt.Sprintf("https://%s", net.JoinHostPort(ip, strconv.Itoa(int(port))))
} else {
rawURL = fmt.Sprintf("https://%s", ip)
}
return url.Parse(rawURL)
}
+27 -39
View File
@@ -1,26 +1,23 @@
package kubeconfig
// This file pins the current behavior of getURLFromService() across the different
// This file pins the behavior of getURLFromService() across the different
// service types (ClusterIP, NodePort, LoadBalancer), Ingress exposure, and the
// TLS SAN fallback logic.
//
// Several test cases intentionally assert known quirks of the current implementation.
// They are documented here (and inline) so a future refactor knows exactly which
// behaviors it changes:
// The behaviors asserted here are:
//
// 1. ClusterIP port handling:
// Always uses Spec.ClusterIP and defaults to port 443; the service's declared
// port is ignored. Only the serverPort override changes the port.
// Uses Spec.ClusterIP with the service's declared port (Spec.Ports[0].Port).
// The port suffix is omitted only when it is the default 443.
//
// 2. NodePort access:
// Always uses hostServerIP:NodePort, with no internal/external detection.
// Internal/external aware. When hostServerIP == ClusterIP the connection is
// treated as internal and uses ClusterIP:Port; otherwise it uses
// hostServerIP:NodePort.
//
// 3. LoadBalancer hostname support:
// Reads only Status.LoadBalancer.Ingress[0].IP. The Hostname field is not
// consulted, so a hostname-only ingress produces the invalid URL "https://".
//
// 4. Port override:
// A non-zero serverPort parameter overrides the computed port.
// Reads Status.LoadBalancer.Ingress[0], preferring IP and falling back to
// Hostname, so a hostname-only ingress produces a valid URL.
import (
"testing"
@@ -45,30 +42,20 @@ func TestURLGeneration_ClusterIP(t *testing.T) {
name string
hostServerIP string
servicePort int32
serverPort int
expectedURL string
}{
{
name: "ClusterIP with default port 443",
hostServerIP: "10.0.0.1",
servicePort: 443,
serverPort: 0,
expectedURL: "https://10.43.0.100",
},
{
// the service port is ignored for ClusterIP, so the URL has no port suffix
name: "ClusterIP ignores custom service port",
// the service's declared port is used, so it appears in the URL
name: "ClusterIP uses custom service port",
hostServerIP: "10.0.0.1",
servicePort: 8443,
serverPort: 0,
expectedURL: "https://10.43.0.100",
},
{
name: "ClusterIP with serverPort override",
hostServerIP: "10.0.0.1",
servicePort: 443,
serverPort: 9443,
expectedURL: "https://10.43.0.100:9443",
expectedURL: "https://10.43.0.100:8443",
},
}
@@ -77,10 +64,10 @@ func TestURLGeneration_ClusterIP(t *testing.T) {
cluster, svc := createClusterIPService("test-cluster", "default", tt.servicePort)
fakeClient := createFakeClient(t, cluster, svc)
url, err := getURLFromService(t.Context(), fakeClient, cluster, tt.hostServerIP, tt.serverPort)
url, err := getURLFromService(t.Context(), fakeClient, cluster, tt.hostServerIP)
require.NoError(t, err)
assert.Equal(t, tt.expectedURL, url)
assert.Equal(t, tt.expectedURL, url.String())
})
}
}
@@ -102,11 +89,13 @@ func TestURLGeneration_NodePort(t *testing.T) {
expectedURL: "https://192.168.1.100:30443",
},
{
// internal access: hostServerIP == ClusterIP, so ClusterIP:Port is used
// instead of the NodePort (port 443 is the default, so it is omitted)
name: "NodePort internal access",
hostServerIP: "10.43.0.100", // same as ClusterIP
nodePort: 30443,
servicePort: 443,
expectedURL: "https://10.43.0.100:30443",
expectedURL: "https://10.43.0.100",
},
}
@@ -115,10 +104,10 @@ func TestURLGeneration_NodePort(t *testing.T) {
cluster, svc := createNodePortService("test-cluster", "default", tt.servicePort, tt.nodePort)
fakeClient := createFakeClient(t, cluster, svc)
url, err := getURLFromService(t.Context(), fakeClient, cluster, tt.hostServerIP, 0)
url, err := getURLFromService(t.Context(), fakeClient, cluster, tt.hostServerIP)
require.NoError(t, err)
assert.Equal(t, tt.expectedURL, url)
assert.Equal(t, tt.expectedURL, url.String())
})
}
}
@@ -142,14 +131,13 @@ func TestURLGeneration_LoadBalancer(t *testing.T) {
expectedURL: "https://203.0.113.10",
},
{
// quirk: the Hostname field is not read, so a hostname-only ingress
// produces the invalid URL "https://" (empty host)
// the Hostname field is used as a fallback when no IP is present
name: "LoadBalancer with hostname ingress",
hostServerIP: "10.0.0.1",
lbIP: "",
lbHostname: "cluster.example.com",
servicePort: 443,
expectedURL: "https://",
expectedURL: "https://cluster.example.com",
},
{
name: "LoadBalancer with custom port",
@@ -166,10 +154,10 @@ func TestURLGeneration_LoadBalancer(t *testing.T) {
cluster, svc := createLoadBalancerService("test-cluster", "default", tt.servicePort, tt.lbIP, tt.lbHostname)
fakeClient := createFakeClient(t, cluster, svc)
url, err := getURLFromService(t.Context(), fakeClient, cluster, tt.hostServerIP, 0)
url, err := getURLFromService(t.Context(), fakeClient, cluster, tt.hostServerIP)
require.NoError(t, err)
assert.Equal(t, tt.expectedURL, url)
assert.Equal(t, tt.expectedURL, url.String())
})
}
}
@@ -193,10 +181,10 @@ func TestURLGeneration_Ingress(t *testing.T) {
cluster, svc, ingress := createIngressService("test-cluster", "default", tt.ingressHost)
fakeClient := createFakeClient(t, cluster, svc, ingress)
url, err := getURLFromService(t.Context(), fakeClient, cluster, "10.0.0.1", 0)
url, err := getURLFromService(t.Context(), fakeClient, cluster, "10.0.0.1")
require.NoError(t, err)
assert.Equal(t, tt.expectedURL, url)
assert.Equal(t, tt.expectedURL, url.String())
})
}
}
@@ -268,10 +256,10 @@ func TestURLGeneration_TLSSANs(t *testing.T) {
fakeClient := createFakeClient(t, cluster, svc)
url, err := getURLFromService(t.Context(), fakeClient, cluster, tt.hostServerIP, 0)
url, err := getURLFromService(t.Context(), fakeClient, cluster, tt.hostServerIP)
require.NoError(t, err)
assert.Equal(t, tt.expectedURL, url)
assert.Equal(t, tt.expectedURL, url.String())
})
}
}
+2 -2
View File
@@ -191,7 +191,7 @@ func NewVirtualK8sClientAndConfig(cluster *v1beta1.Cluster) (*kubernetes.Clients
vKubeconfig := kubeconfig.New()
kubeletAltName := fmt.Sprintf("k3k-%s-kubelet", cluster.Name)
vKubeconfig.AltNames = certs.AddSANs([]string{hostIP, kubeletAltName})
config, err = vKubeconfig.Generate(ctx, k8sClient, cluster, hostIP, 0)
config, err = vKubeconfig.Generate(ctx, k8sClient, cluster, hostIP)
return err
}).
@@ -225,7 +225,7 @@ func NewVirtualK8sClientAndKubeconfig(cluster *v1beta1.Cluster) (*kubernetes.Cli
vKubeconfig := kubeconfig.New()
kubeletAltName := fmt.Sprintf("k3k-%s-kubelet", cluster.Name)
vKubeconfig.AltNames = certs.AddSANs([]string{hostIP, kubeletAltName})
config, err = vKubeconfig.Generate(ctx, k8sClient, cluster, hostIP, 0)
config, err = vKubeconfig.Generate(ctx, k8sClient, cluster, hostIP)
return err
}).