Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add concise error messages for ingress hostname does not supported #543

Merged
merged 6 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
33 changes: 29 additions & 4 deletions api/v1alpha1/tenantcontrolplane_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
23 changes: 17 additions & 6 deletions internal/kubeadm/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,29 +44,40 @@ 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{
Certificate: contents[0],
PrivateKey: contents[1],
}

return certificatePrivateKeyPair, err
return certificatePrivateKeyPair, nil
}

func getKubeadmCert(baseName string) (*certs.KubeadmCert, error) {
Expand Down
4 changes: 2 additions & 2 deletions internal/resources/api_server_certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
Loading