From 477989a214a82bd11b8fdc35fc609a2e048e7df6 Mon Sep 17 00:00:00 2001 From: reoring Date: Tue, 20 Aug 2024 17:01:28 +0900 Subject: [PATCH] chore: add concise error messages for non supported ingress hostname (#543) * chore: improve error handling and logging for certificate operations - Enhance error reporting in GenerateCertificatePrivateKeyPair function - Add detailed error checks for CA certificate and private key parsing - Implement check for expected number of certificate files - Improve error logging in APIServerCertificate resource This commit preserves more details about certificate-related issues, aiding in debugging and troubleshooting. * feat: support loadbalancer hostname resolution Add functionality to resolve loadbalancer hostname to IP address in DeclaredControlPlaneAddress method. This enhances the existing IP address handling by allowing the use of hostnames for loadbalancers. - Add hostname check in addition to IP check - Implement hostname resolution using net.LookupIP - Return the first resolved IP address if available * fix: Remove hostname support for LoadBalancer ingress - Extract LoadBalancer address logic to separate function - Remove hostname resolution for LoadBalancer ingress - Add explanatory comments on reasons for not supporting hostnames * fix: replace fmt and vet with golint - Remove fmt and vet targets - Update build target to use golint instead of fmt and vet - Remove fmt and vet dependencies from run target * fix: lint errors --- Makefile | 4 +-- api/v1alpha1/tenantcontrolplane_funcs.go | 33 +++++++++++++++++--- internal/kubeadm/certificates.go | 23 ++++++++++---- internal/resources/api_server_certificate.go | 4 +-- 4 files changed, 50 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 126bb60..b1ccb3f 100644 --- a/Makefile +++ b/Makefile @@ -170,10 +170,10 @@ BUILD_DATE ?= $$(git log -1 --format="%at" | xargs -I{} date -d @{} +%Y-%m- get_version: @echo -n v$(VERSION) -build: generate fmt vet ## Build manager binary. +build: generate golint ## Build manager binary. go build -o bin/manager main.go -run: manifests generate fmt vet ## Run a controller from your host. +run: manifests generate ## Run a controller from your host. go run ./main.go docker-build: ## Build docker image with the manager. diff --git a/api/v1alpha1/tenantcontrolplane_funcs.go b/api/v1alpha1/tenantcontrolplane_funcs.go index b2276f9..bd30a96 100644 --- a/api/v1alpha1/tenantcontrolplane_funcs.go +++ b/api/v1alpha1/tenantcontrolplane_funcs.go @@ -61,10 +61,35 @@ func (in *TenantControlPlane) DeclaredControlPlaneAddress(ctx context.Context, c return "", kamajierrors.NonExposedLoadBalancerError{} } - for _, lb := range loadBalancerStatus.Ingress { - if ip := lb.IP; len(ip) > 0 { - return ip, nil - } + return getLoadBalancerAddress(loadBalancerStatus.Ingress) + } + + return "", kamajierrors.MissingValidIPError{} +} + +// getLoadBalancerAddress extracts the IP address from LoadBalancer ingress. +// It also checks and rejects hostname usage for LoadBalancer ingress. +// +// Reasons for not supporting hostnames: +// - DNS resolution can differ across environments, leading to inconsistent behavior. +// - It may cause connectivity problems between Kubernetes components. +// - The DNS resolution could change over time, potentially breaking cluster-to-API-server connections. +// +// Recommended solutions: +// - Use a static IP address to ensure stable and predictable communication within the cluster. +// - If a hostname is necessary, consider setting up a Virtual IP (VIP) for the given hostname. +// - Alternatively, use an external load balancer that can provide a stable IP address. +// +// Note: Implementing L7 routing with the API Server requires a deep understanding of the implications. +// Users should be aware of the complexities involved, including potential issues with TLS passthrough +// for client-based certificate authentication in Ingress expositions. +func getLoadBalancerAddress(ingress []corev1.LoadBalancerIngress) (string, error) { + for _, lb := range ingress { + if ip := lb.IP; len(ip) > 0 { + return ip, nil + } + if hostname := lb.Hostname; len(hostname) > 0 { + return "", fmt.Errorf("hostname not supported for LoadBalancer ingress: use static IP instead") } } diff --git a/internal/kubeadm/certificates.go b/internal/kubeadm/certificates.go index fbb362e..bdb151b 100644 --- a/internal/kubeadm/certificates.go +++ b/internal/kubeadm/certificates.go @@ -44,21 +44,32 @@ func GenerateCACertificatePrivateKeyPair(baseName string, config *Configuration) func GenerateCertificatePrivateKeyPair(baseName string, config *Configuration, ca CertificatePrivateKeyPair) (*CertificatePrivateKeyPair, error) { defer deleteCertificateDirectory(config.InitConfiguration.CertificatesDir) - certificate, _ := cryptoKamaji.ParseCertificateBytes(ca.Certificate) - signer, _ := cryptoKamaji.ParsePrivateKeyBytes(ca.PrivateKey) + certificate, err := cryptoKamaji.ParseCertificateBytes(ca.Certificate) + if err != nil { + return nil, fmt.Errorf("failed to parse CA certificate: %w", err) + } + + signer, err := cryptoKamaji.ParsePrivateKeyBytes(ca.PrivateKey) + if err != nil { + return nil, fmt.Errorf("failed to parse CA private key: %w", err) + } kubeadmCert, err := getKubeadmCert(baseName) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get kubeadm cert: %w", err) } if err = initPhaseFromCA(kubeadmCert, config, certificate, signer); err != nil { - return nil, err + return nil, fmt.Errorf("failed to initialize phase from CA: %w", err) } contents, err := readCertificateFiles(baseName, config.InitConfiguration.CertificatesDir, "crt", "key") if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read certificate files: %w", err) + } + + if len(contents) != 2 { + return nil, fmt.Errorf("unexpected number of certificate files: expected 2, got %d", len(contents)) } certificatePrivateKeyPair := &CertificatePrivateKeyPair{ @@ -66,7 +77,7 @@ func GenerateCertificatePrivateKeyPair(baseName string, config *Configuration, c PrivateKey: contents[1], } - return certificatePrivateKeyPair, err + return certificatePrivateKeyPair, nil } func getKubeadmCert(baseName string) (*certs.KubeadmCert, error) { diff --git a/internal/resources/api_server_certificate.go b/internal/resources/api_server_certificate.go index a056dad..076f50f 100644 --- a/internal/resources/api_server_certificate.go +++ b/internal/resources/api_server_certificate.go @@ -128,9 +128,9 @@ func (r *APIServerCertificate) mutate(ctx context.Context, tenantControlPlane *k config, err := getStoredKubeadmConfiguration(ctx, r.Client, r.TmpDirectory, tenantControlPlane) if err != nil { - logger.Error(err, "cannot retrieve kubeadm configuration") + logger.Error(err, "cannot generate certificate and private key in api server certificate", "details", err.Error()) - return err + return fmt.Errorf("failed to generate certificate and private key: %w", err) } ca := kubeadm.CertificatePrivateKeyPair{