From eb5033c40f401e925bb25c719eb54da51481ca24 Mon Sep 17 00:00:00 2001 From: Adam Talbot Date: Tue, 2 Jan 2024 15:25:41 +0000 Subject: [PATCH] feat: add validation for metrics tls config Signed-off-by: Adam Talbot --- .../controller/validation/validation.go | 87 +++++++---- .../controller/validation/validation_test.go | 136 ++++++++++++++++++ 2 files changed, 192 insertions(+), 31 deletions(-) diff --git a/internal/apis/config/controller/validation/validation.go b/internal/apis/config/controller/validation/validation.go index fda5fdd3c..82af65c47 100644 --- a/internal/apis/config/controller/validation/validation.go +++ b/internal/apis/config/controller/validation/validation.go @@ -23,39 +23,64 @@ import ( "net/url" "strings" - //utilerrors "k8s.io/apimachinery/pkg/util/errors" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" config "github.com/cert-manager/cert-manager/internal/apis/config/controller" defaults "github.com/cert-manager/cert-manager/internal/apis/config/controller/v1alpha1" ) -func ValidateControllerConfiguration(o *config.ControllerConfiguration) error { - if len(o.IngressShimConfig.DefaultIssuerKind) == 0 { - return errors.New("the --default-issuer-kind flag must not be empty") - } +func ValidateControllerConfiguration(cfg *config.ControllerConfiguration) error { + var allErrors []error - if o.KubernetesAPIBurst <= 0 { - return fmt.Errorf("invalid value for kube-api-burst: %v must be higher than 0", o.KubernetesAPIBurst) - } - - if o.KubernetesAPIQPS <= 0 { - return fmt.Errorf("invalid value for kube-api-qps: %v must be higher than 0", o.KubernetesAPIQPS) - } - - if float32(o.KubernetesAPIBurst) < o.KubernetesAPIQPS { - return fmt.Errorf("invalid value for kube-api-burst: %v must be higher or equal to kube-api-qps: %v", o.KubernetesAPIQPS, o.KubernetesAPIQPS) - } - - for _, server := range o.ACMEHTTP01Config.SolverNameservers { - // ensure all servers have a port number - _, _, err := net.SplitHostPort(server) - if err != nil { - return fmt.Errorf("invalid DNS server (%v): %v", err, server) + if cfg.MetricsTLSConfig.FilesystemConfigProvided() && cfg.MetricsTLSConfig.DynamicConfigProvided() { + allErrors = append(allErrors, fmt.Errorf("invalid configuration: cannot specify both filesystem based and dynamic TLS configuration")) + } else { + if cfg.MetricsTLSConfig.FilesystemConfigProvided() { + if cfg.MetricsTLSConfig.Filesystem.KeyFile == "" { + allErrors = append(allErrors, fmt.Errorf("invalid configuration: tlsConfig.filesystem.keyFile (--metrics-tls-private-key-file) must be specified when using filesystem based TLS config")) + } + if cfg.MetricsTLSConfig.Filesystem.CertFile == "" { + allErrors = append(allErrors, fmt.Errorf("invalid configuration: tlsConfig.filesystem.certFile (--metrics-tls-cert-file) must be specified when using filesystem based TLS config")) + } + } else if cfg.MetricsTLSConfig.DynamicConfigProvided() { + if cfg.MetricsTLSConfig.Dynamic.SecretNamespace == "" { + allErrors = append(allErrors, fmt.Errorf("invalid configuration: tlsConfig.dynamic.secretNamespace (--metrics-dynamic-serving-ca-secret-namespace) must be specified when using dynamic TLS config")) + } + if cfg.MetricsTLSConfig.Dynamic.SecretName == "" { + allErrors = append(allErrors, fmt.Errorf("invalid configuration: tlsConfig.dynamic.secretName (--metrics-dynamic-serving-ca-secret-name) must be specified when using dynamic TLS config")) + } + if len(cfg.MetricsTLSConfig.Dynamic.DNSNames) == 0 { + allErrors = append(allErrors, fmt.Errorf("invalid configuration: tlsConfig.dynamic.dnsNames (--metrics-dynamic-serving-dns-names) must be specified when using dynamic TLS config")) + } } } - for _, server := range o.ACMEDNS01Config.RecursiveNameservers { + if len(cfg.IngressShimConfig.DefaultIssuerKind) == 0 { + allErrors = append(allErrors, errors.New("the --default-issuer-kind flag must not be empty")) + } + + if cfg.KubernetesAPIBurst <= 0 { + allErrors = append(allErrors, fmt.Errorf("invalid value for kube-api-burst: %v must be higher than 0", cfg.KubernetesAPIBurst)) + } + + if cfg.KubernetesAPIQPS <= 0 { + allErrors = append(allErrors, fmt.Errorf("invalid value for kube-api-qps: %v must be higher than 0", cfg.KubernetesAPIQPS)) + } + + if float32(cfg.KubernetesAPIBurst) < cfg.KubernetesAPIQPS { + allErrors = append(allErrors, fmt.Errorf("invalid value for kube-api-burst: %v must be higher or equal to kube-api-qps: %v", cfg.KubernetesAPIQPS, cfg.KubernetesAPIQPS)) + } + + for _, server := range cfg.ACMEHTTP01Config.SolverNameservers { + // ensure all servers have a port number + _, _, err := net.SplitHostPort(server) + if err != nil { + allErrors = append(allErrors, fmt.Errorf("invalid DNS server (%v): %v", err, server)) + } + } + + for _, server := range cfg.ACMEDNS01Config.RecursiveNameservers { // ensure all servers follow one of the following formats: // - : // - https:// @@ -63,31 +88,31 @@ func ValidateControllerConfiguration(o *config.ControllerConfiguration) error { if strings.HasPrefix(server, "https://") { _, err := url.ParseRequestURI(server) if err != nil { - return fmt.Errorf("invalid DNS server (%v): %v", err, server) + allErrors = append(allErrors, fmt.Errorf("invalid DNS server (%v): %v", err, server)) } } else { _, _, err := net.SplitHostPort(server) if err != nil { - return fmt.Errorf("invalid DNS server (%v): %v", err, server) + allErrors = append(allErrors, fmt.Errorf("invalid DNS server (%v): %v", err, server)) } } } - errs := []error{} + controllerErrors := []error{} allControllersSet := sets.NewString(defaults.AllControllers...) - for _, controller := range o.Controllers { + for _, controller := range cfg.Controllers { if controller == "*" { continue } controller = strings.TrimPrefix(controller, "-") if !allControllersSet.Has(controller) { - errs = append(errs, fmt.Errorf("%q is not in the list of known controllers", controller)) + controllerErrors = append(controllerErrors, fmt.Errorf("%q is not in the list of known controllers", controller)) } } - if len(errs) > 0 { - return fmt.Errorf("validation failed for '--controllers': %v", errs) + if len(controllerErrors) > 0 { + allErrors = append(allErrors, fmt.Errorf("validation failed for '--controllers': %v", controllerErrors)) } - return nil + return utilerrors.NewAggregate(allErrors) } diff --git a/internal/apis/config/controller/validation/validation_test.go b/internal/apis/config/controller/validation/validation_test.go index 8be751e4d..aaefb1b14 100644 --- a/internal/apis/config/controller/validation/validation_test.go +++ b/internal/apis/config/controller/validation/validation_test.go @@ -39,6 +39,142 @@ func TestValidateControllerConfiguration(t *testing.T) { }, false, }, + { + "with both filesystem and dynamic tls configured", + &config.ControllerConfiguration{ + IngressShimConfig: config.IngressShimConfig{ + DefaultIssuerKind: "Issuer", + }, + KubernetesAPIBurst: 1, + KubernetesAPIQPS: 1, + MetricsTLSConfig: config.TLSConfig{ + Filesystem: config.FilesystemServingConfig{ + CertFile: "/test.crt", + KeyFile: "/test.key", + }, + Dynamic: config.DynamicServingConfig{ + SecretNamespace: "cert-manager", + SecretName: "test", + DNSNames: []string{"example.com"}, + }, + }, + }, + true, + }, + { + "with valid filesystem tls config", + &config.ControllerConfiguration{ + IngressShimConfig: config.IngressShimConfig{ + DefaultIssuerKind: "Issuer", + }, + KubernetesAPIBurst: 1, + KubernetesAPIQPS: 1, + MetricsTLSConfig: config.TLSConfig{ + Filesystem: config.FilesystemServingConfig{ + CertFile: "/test.crt", + KeyFile: "/test.key", + }, + }, + }, + false, + }, + { + "with valid tls config missing keyfile", + &config.ControllerConfiguration{ + IngressShimConfig: config.IngressShimConfig{ + DefaultIssuerKind: "Issuer", + }, + KubernetesAPIBurst: 1, + KubernetesAPIQPS: 1, + MetricsTLSConfig: config.TLSConfig{ + Filesystem: config.FilesystemServingConfig{ + CertFile: "/test.crt", + }, + }, + }, + true, + }, + { + "with valid tls config missing certfile", + &config.ControllerConfiguration{ + IngressShimConfig: config.IngressShimConfig{ + DefaultIssuerKind: "Issuer", + }, + KubernetesAPIBurst: 1, + KubernetesAPIQPS: 1, + MetricsTLSConfig: config.TLSConfig{ + Filesystem: config.FilesystemServingConfig{ + KeyFile: "/test.key", + }, + }, + }, + true, + }, + { + "with valid dynamic tls config", + &config.ControllerConfiguration{ + IngressShimConfig: config.IngressShimConfig{ + DefaultIssuerKind: "Issuer", + }, + KubernetesAPIBurst: 1, + KubernetesAPIQPS: 1, + MetricsTLSConfig: config.TLSConfig{ + Dynamic: config.DynamicServingConfig{ + SecretNamespace: "cert-manager", + SecretName: "test", + DNSNames: []string{"example.com"}, + }, + }, + }, + false, + }, + { + "with dynamic tls missing secret namespace", + &config.ControllerConfiguration{ + MetricsTLSConfig: config.TLSConfig{ + Dynamic: config.DynamicServingConfig{ + SecretName: "test", + DNSNames: []string{"example.com"}, + }, + }, + }, + true, + }, + { + "with dynamic tls missing secret name", + &config.ControllerConfiguration{ + IngressShimConfig: config.IngressShimConfig{ + DefaultIssuerKind: "Issuer", + }, + KubernetesAPIBurst: 1, + KubernetesAPIQPS: 1, + MetricsTLSConfig: config.TLSConfig{ + Dynamic: config.DynamicServingConfig{ + SecretNamespace: "cert-manager", + DNSNames: []string{"example.com"}, + }, + }, + }, + true, + }, + { + "with dynamic tls missing dns names", + &config.ControllerConfiguration{ + IngressShimConfig: config.IngressShimConfig{ + DefaultIssuerKind: "Issuer", + }, + KubernetesAPIBurst: 1, + KubernetesAPIQPS: 1, + MetricsTLSConfig: config.TLSConfig{ + Dynamic: config.DynamicServingConfig{ + SecretName: "test", + SecretNamespace: "cert-manager", + DNSNames: nil, + }, + }, + }, + true, + }, { "with missing issuer kind", &config.ControllerConfiguration{