diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 57433f6..e7a6315 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -566,11 +566,27 @@ func getPeerCertName(etcdClusterName string) string { return peerCertName } -func createCMCertificateConfig(ec *ecv1alpha1.EtcdCluster) *certInterface.Config { +// parseValidityDuration parses a duration string and returns the parsed duration. +// If the customizedDuration is empty, it returns the defaultDuration. +// Returns an error if the duration string cannot be parsed. +func parseValidityDuration(customizedDuration string, defaultDuration time.Duration) (time.Duration, error) { + if customizedDuration == "" { + return defaultDuration, nil + } + duration, err := time.ParseDuration(customizedDuration) + if err != nil { + return 0, fmt.Errorf("failed to parse ValidityDuration: %w", err) + } + return duration, nil +} + +func createCMCertificateConfig(ec *ecv1alpha1.EtcdCluster) (*certInterface.Config, error) { cmConfig := ec.Spec.TLS.ProviderCfg.CertManagerCfg - duration, err := time.ParseDuration(cmConfig.ValidityDuration) + + // Set default duration to 90 days for cert-manager if not provided + duration, err := parseValidityDuration(cmConfig.ValidityDuration, certInterface.DefaultCertManagerValidity) if err != nil { - log.Printf("Failed to parse ValidityDuration: %s", err) + return nil, err } var getAltNames certInterface.AltNames @@ -580,7 +596,12 @@ func createCMCertificateConfig(ec *ecv1alpha1.EtcdCluster) *certInterface.Config IPs: make([]net.IP, len(cmConfig.AltNames.DNSNames)), } } else { - defaultDNSNames := []string{fmt.Sprintf("%s.svc.cluster.local", cmConfig.CommonName)} + // Use wildcard DNS for the cluster's headless service to cover all pods + // This allows the certificate to work for pod-0, pod-1, etc. + defaultDNSNames := []string{ + fmt.Sprintf("*.%s.%s.svc.cluster.local", ec.Name, ec.Namespace), + fmt.Sprintf("%s.%s.svc.cluster.local", ec.Name, ec.Namespace), + } getAltNames = certInterface.AltNames{ DNSNames: defaultDNSNames, } @@ -596,14 +617,16 @@ func createCMCertificateConfig(ec *ecv1alpha1.EtcdCluster) *certInterface.Config "issuerKind": cmConfig.IssuerKind, }, } - return config + return config, nil } -func createAutoCertificateConfig(ec *ecv1alpha1.EtcdCluster) *certInterface.Config { +func createAutoCertificateConfig(ec *ecv1alpha1.EtcdCluster) (*certInterface.Config, error) { autoConfig := ec.Spec.TLS.ProviderCfg.AutoCfg - duration, err := time.ParseDuration(autoConfig.ValidityDuration) + + // Set default duration to 365 days for auto provider if not provided + duration, err := parseValidityDuration(autoConfig.ValidityDuration, certInterface.DefaultAutoValidity) if err != nil { - log.Printf("Failed to parse ValidityDuration: %s", err) + return nil, err } var altNames certInterface.AltNames @@ -613,7 +636,12 @@ func createAutoCertificateConfig(ec *ecv1alpha1.EtcdCluster) *certInterface.Conf IPs: make([]net.IP, len(autoConfig.AltNames.DNSNames)), } } else { - defaultDNSNames := []string{fmt.Sprintf("%s.svc.cluster.local", autoConfig.CommonName)} + // Use wildcard DNS for the cluster's headless service to cover all pods + // This allows the certificate to work for pod-0, pod-1, etc. + defaultDNSNames := []string{ + fmt.Sprintf("*.%s.%s.svc.cluster.local", ec.Name, ec.Namespace), + fmt.Sprintf("%s.%s.svc.cluster.local", ec.Name, ec.Namespace), + } altNames = certInterface.AltNames{ DNSNames: defaultDNSNames, } @@ -625,7 +653,7 @@ func createAutoCertificateConfig(ec *ecv1alpha1.EtcdCluster) *certInterface.Conf ValidityDuration: duration, AltNames: altNames, } - return config + return config, nil } func createCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client.Client, certName string) error { @@ -647,17 +675,23 @@ func createCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client secretKey := client.ObjectKey{Name: certName, Namespace: ec.Namespace} switch { case ec.Spec.TLS.ProviderCfg.AutoCfg != nil: - autoConfig := createAutoCertificateConfig(ec) + autoConfig, err := createAutoCertificateConfig(ec) + if err != nil { + return fmt.Errorf("error creating auto certificate config: %w", err) + } createCertErr := cert.EnsureCertificateSecret(ctx, secretKey, autoConfig) if createCertErr != nil { - log.Printf("Error creating certificate: %s", createCertErr) + return fmt.Errorf("error creating auto certificate: %w", createCertErr) } return nil case ec.Spec.TLS.ProviderCfg.CertManagerCfg != nil: - cmConfig := createCMCertificateConfig(ec) + cmConfig, err := createCMCertificateConfig(ec) + if err != nil { + return fmt.Errorf("error creating cert-manager certificate config: %w", err) + } createCertErr := cert.EnsureCertificateSecret(ctx, secretKey, cmConfig) if createCertErr != nil { - log.Printf("Error creating certificate: %s", createCertErr) + return fmt.Errorf("error creating cert-manager certificate: %w", createCertErr) } return nil default: diff --git a/pkg/certificate/auto/provider.go b/pkg/certificate/auto/provider.go index 38a75e3..41a67c0 100644 --- a/pkg/certificate/auto/provider.go +++ b/pkg/certificate/auto/provider.go @@ -28,7 +28,9 @@ import ( ) const ( - DefaultValidity = 365 * 24 * time.Hour + // oneYearInHours is used to convert validity duration to years for transport.SelfCert. + // transport.SelfCert expects the validity parameter to be in years (uint). + oneYearInHours = 365 * 24 * time.Hour ) type Provider struct { @@ -218,9 +220,14 @@ func checkKeyPair(cert *x509.Certificate, privateKey crypto.PrivateKey) error { // https://github.com/etcd-io/etcd/blob/b87bc1c3a275d7d4904f4d201b963a2de2264f0d/client/pkg/transport/listener.go#L275 func (ac *Provider) createNewSecret(ctx context.Context, secretKey client.ObjectKey, cfg *interfaces.Config) error { - validity := DefaultValidity + validity := interfaces.DefaultAutoValidity if cfg.ValidityDuration != 0 { - validity = cfg.ValidityDuration * time.Hour + validity = cfg.ValidityDuration + } + + // Validate validity duration: minimum is 1 year as required by etcd + if validity < oneYearInHours { + return fmt.Errorf("validity duration must be at least 1 year (365 days), got: %v", validity) } tmpDir, err := os.MkdirTemp("", fmt.Sprintf("etcd-auto-cert-%s-*", secretKey.Name)) @@ -252,7 +259,15 @@ func (ac *Provider) createNewSecret(ctx context.Context, secretKey client.Object log.Printf("calling SelfCert with hosts: %v", hosts) - tlsInfo, selfCertErr := transport.SelfCert(zap.NewNop(), tmpDir, hosts, uint(validity/DefaultValidity)) + // Convert validity duration to years for transport.SelfCert + // transport.SelfCert expects the validity parameter to be in years (uint) + validityYears := uint(validity / oneYearInHours) + if validityYears == 0 { + // This should not happen due to the earlier check, but adding as a safeguard + return fmt.Errorf("validity duration converts to 0 years, must be at least 1 year") + } + + tlsInfo, selfCertErr := transport.SelfCert(zap.NewNop(), tmpDir, hosts, validityYears) if selfCertErr != nil { return fmt.Errorf("certificate creation via transport.SelfCert failed: %w", selfCertErr) } diff --git a/pkg/certificate/interfaces/interface.go b/pkg/certificate/interfaces/interface.go index fe5ae84..35dfd60 100644 --- a/pkg/certificate/interfaces/interface.go +++ b/pkg/certificate/interfaces/interface.go @@ -46,6 +46,12 @@ const ( // with a delay of RetryInterval between consecutive retries MaxRetries = 36 RetryInterval = 5 * time.Second + + // DefaultAutoValidity is the default validity duration for auto-generated certificates (365 days) + DefaultAutoValidity = 365 * 24 * time.Hour + + // DefaultCertManagerValidity is the default validity duration for cert-manager certificates (90 days) + DefaultCertManagerValidity = 90 * 24 * time.Hour ) // AltNames contains the domain names and IP addresses that will be added diff --git a/test/e2e/auto_provider_test.go b/test/e2e/auto_provider_test.go index 056232a..f044b73 100644 --- a/test/e2e/auto_provider_test.go +++ b/test/e2e/auto_provider_test.go @@ -29,7 +29,7 @@ import ( const ( autoCertificateName = "sample-cert" autoCertificateNamespace = "default" - autoCertificateValidity = auto.DefaultValidity + autoCertificateValidity = interfaces.DefaultAutoValidity ) func TestAutoProvider(t *testing.T) { diff --git a/test/e2e/cert_manager_test.go b/test/e2e/cert_manager_test.go index 217cacd..41560a0 100644 --- a/test/e2e/cert_manager_test.go +++ b/test/e2e/cert_manager_test.go @@ -30,7 +30,7 @@ const ( cmIssuerType = "ClusterIssuer" cmCertificateName = "sample-cert" cmCertificateNamespace = "default" - cmCertificateValidity = 24 * time.Hour + cmCertificateValidity = interfaces.DefaultCertManagerValidity ) var cmIssuer = &certv1.ClusterIssuer{