From 76dbbdd75a05cc0724af635be738cb91659d4425 Mon Sep 17 00:00:00 2001
From: Mikhail Fedosin <mfedosin@redhat.com>
Date: Tue, 3 Mar 2026 17:55:25 +0100
Subject: [PATCH] feat: support default values in tls.NewConfigFromEnv

Add a DefaultMinTLSVersion constant (TLS 1.3) and an optional defaults
parameter to NewConfigFromEnv so callers can supply fallback values for
fields not set by environment variables. The webhook package now passes
DefaultMinTLSVersion through the new parameter, simplifying its TLS
setup logic.

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>
---
 tls/config.go      | 18 ++++++++++++--
 tls/config_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++++++
 webhook/webhook.go | 17 ++++++-------
 3 files changed, 85 insertions(+), 12 deletions(-)

diff --git a/tls/config.go b/tls/config.go
index 4d07cbb5bb..66f920e70b 100644
--- a/tls/config.go
+++ b/tls/config.go
@@ -23,6 +23,10 @@ import (
 	"strings"
 )
 
+// DefaultMinTLSVersion is the minimum TLS version used when no override
+// is provided via environment variable or caller-supplied defaults.
+const DefaultMinTLSVersion = cryptotls.VersionTLS13
+
 // Environment variable name suffixes for TLS configuration.
 // Use with a prefix to namespace them, e.g. "WEBHOOK_" + MinVersionEnvKey
 // reads the WEBHOOK_TLS_MIN_VERSION variable.
@@ -46,9 +50,19 @@ type Config struct {
 // returns a Config. The prefix is prepended to each standard env-var suffix;
 // for example with prefix "WEBHOOK_" the function reads
 // WEBHOOK_TLS_MIN_VERSION, WEBHOOK_TLS_MAX_VERSION, etc.
-// Fields whose corresponding env var is unset are left at their zero value.
-func NewConfigFromEnv(prefix string) (*Config, error) {
+//
+// An optional defaults Config may be supplied; its values are used for any
+// field whose corresponding environment variable is unset. When no defaults
+// are provided, unset fields remain at their zero value.
+func NewConfigFromEnv(prefix string, defaults ...Config) (*Config, error) {
+	if len(defaults) > 1 {
+		return nil, fmt.Errorf("at most one defaults Config may be provided, got %d", len(defaults))
+	}
+
 	var cfg Config
+	if len(defaults) == 1 {
+		cfg = defaults[0]
+	}
 
 	if v := os.Getenv(prefix + MinVersionEnvKey); v != "" {
 		ver, err := parseVersion(v)
diff --git a/tls/config_test.go b/tls/config_test.go
index b97e4044d5..c5f09469f7 100644
--- a/tls/config_test.go
+++ b/tls/config_test.go
@@ -349,6 +349,68 @@ func TestNewConfigFromEnv(t *testing.T) {
 			t.Fatal("expected error for invalid curve")
 		}
 	})
+
+	t.Run("defaults used when env vars are unset", func(t *testing.T) {
+		defaults := Config{
+			MinVersion:   DefaultMinTLSVersion,
+			MaxVersion:   cryptotls.VersionTLS13,
+			CipherSuites: []uint16{cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+			CurvePreferences: []cryptotls.CurveID{
+				cryptotls.X25519,
+			},
+		}
+		cfg, err := NewConfigFromEnv("", defaults)
+		if err != nil {
+			t.Fatal("unexpected error:", err)
+		}
+		if cfg.MinVersion != DefaultMinTLSVersion {
+			t.Errorf("MinVersion = %d, want %d", cfg.MinVersion, DefaultMinTLSVersion)
+		}
+		if cfg.MaxVersion != cryptotls.VersionTLS13 {
+			t.Errorf("MaxVersion = %d, want %d", cfg.MaxVersion, cryptotls.VersionTLS13)
+		}
+		if len(cfg.CipherSuites) != 1 || cfg.CipherSuites[0] != cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 {
+			t.Errorf("CipherSuites = %v, want [%d]", cfg.CipherSuites, cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256)
+		}
+		if len(cfg.CurvePreferences) != 1 || cfg.CurvePreferences[0] != cryptotls.X25519 {
+			t.Errorf("CurvePreferences = %v, want [%d]", cfg.CurvePreferences, cryptotls.X25519)
+		}
+	})
+
+	t.Run("env vars override defaults", func(t *testing.T) {
+		t.Setenv(MinVersionEnvKey, "1.2")
+		t.Setenv(CurvePreferencesEnvKey, "CurveP256")
+
+		defaults := Config{
+			MinVersion: DefaultMinTLSVersion,
+			CurvePreferences: []cryptotls.CurveID{
+				cryptotls.X25519,
+			},
+		}
+		cfg, err := NewConfigFromEnv("", defaults)
+		if err != nil {
+			t.Fatal("unexpected error:", err)
+		}
+		if cfg.MinVersion != cryptotls.VersionTLS12 {
+			t.Errorf("MinVersion = %d, want %d (env should override default)", cfg.MinVersion, cryptotls.VersionTLS12)
+		}
+		if len(cfg.CurvePreferences) != 1 || cfg.CurvePreferences[0] != cryptotls.CurveP256 {
+			t.Errorf("CurvePreferences = %v, want [%d] (env should override default)", cfg.CurvePreferences, cryptotls.CurveP256)
+		}
+	})
+
+	t.Run("multiple defaults returns error", func(t *testing.T) {
+		_, err := NewConfigFromEnv("", Config{}, Config{})
+		if err == nil {
+			t.Fatal("expected error when passing multiple defaults")
+		}
+	})
+
+	t.Run("DefaultMinTLSVersion equals TLS 1.3", func(t *testing.T) {
+		if DefaultMinTLSVersion != cryptotls.VersionTLS13 {
+			t.Errorf("DefaultMinTLSVersion = %d, want %d (TLS 1.3)", DefaultMinTLSVersion, cryptotls.VersionTLS13)
+		}
+	})
 }
 
 func TestConfig_TLSConfig(t *testing.T) {
diff --git a/webhook/webhook.go b/webhook/webhook.go
index 8f3a3701cc..28525b4c41 100644
--- a/webhook/webhook.go
+++ b/webhook/webhook.go
@@ -191,20 +191,17 @@ func New(
 
 	logger := logging.FromContext(ctx)
 
-	tlsCfg, err := knativetls.NewConfigFromEnv("WEBHOOK_")
+	tlsCfg, err := knativetls.NewConfigFromEnv("WEBHOOK_", knativetls.Config{
+		MinVersion: knativetls.DefaultMinTLSVersion,
+	})
 	if err != nil {
 		return nil, fmt.Errorf("reading TLS configuration from environment: %w", err)
 	}
 
-	// Replace the TLS configuration with the one from the environment if not set.
-	// Default to TLS 1.3 as the minimum version when neither the caller nor the
-	// environment specifies one.
-	if opts.TLSMinVersion == 0 {
-		if tlsCfg.MinVersion != 0 {
-			opts.TLSMinVersion = tlsCfg.MinVersion
-		} else {
-			opts.TLSMinVersion = tls.VersionTLS13
-		}
+	// Apply environment / default TLS settings for any fields the caller
+	// has not already configured.
+	if opts.TLSMinVersion == 0 && tlsCfg.MinVersion != 0 {
+		opts.TLSMinVersion = tlsCfg.MinVersion
 	}
 	if opts.TLSMaxVersion == 0 && tlsCfg.MaxVersion != 0 {
 		opts.TLSMaxVersion = tlsCfg.MaxVersion