Skip to content

Replace NewConfigFromEnv with DefaultConfigFromEnv #3328

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.

Sign up for GitHub

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

Merged
knative-prow merged 1 commit into from
Mar 4, 2026
Merged

Replace NewConfigFromEnv with DefaultConfigFromEnv #3328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 7 additions & 27 deletions tls/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,14 @@ const (
CurvePreferencesEnvKey = "TLS_CURVE_PREFERENCES"
)

// Config holds parsed TLS configuration values that can be used
// to build a *crypto/tls.Config.
type Config struct {
MinVersion uint16
MaxVersion uint16
CipherSuites []uint16
CurvePreferences []cryptotls.CurveID
}

// NewConfigFromEnv reads TLS configuration from environment variables and
// returns a Config. The prefix is prepended to each standard env-var suffix;
// DefaultConfigFromEnv returns a tls.Config with secure defaults.
// 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) {
var cfg Config
func DefaultConfigFromEnv(prefix string) (*cryptotls.Config, error) {
cfg := &cryptotls.Config{
MinVersion: cryptotls.VersionTLS13,
}

if v := os.Getenv(prefix + MinVersionEnvKey); v != "" {
ver, err := parseVersion(v)
Expand Down Expand Up @@ -82,19 +74,7 @@ func NewConfigFromEnv(prefix string) (*Config, error) {
cfg.CurvePreferences = curves
}

return &cfg, nil
}

// TLSConfig constructs a *crypto/tls.Config from the parsed configuration.
// The caller typically adds additional fields such as GetCertificate.
func (c *Config) TLSConfig() *cryptotls.Config {
//nolint:gosec // Min version is caller-configurable; default is TLS 1.3.
return &cryptotls.Config{
MinVersion: c.MinVersion,
MaxVersion: c.MaxVersion,
CipherSuites: c.CipherSuites,
CurvePreferences: c.CurvePreferences,
}
return cfg, nil
}

// parseVersion converts a TLS version string to the corresponding
Expand Down
65 changes: 16 additions & 49 deletions tls/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,14 @@ func Test_parseCurvePreferences(t *testing.T) {
}
}

func TestNewConfigFromEnv(t *testing.T) {
t.Run("no env vars set returns zero value", func(t *testing.T) {
cfg, err := NewConfigFromEnv("")
func TestDefaultConfigFromEnv(t *testing.T) {
t.Run("no env vars returns TLS 1.3 default", func(t *testing.T) {
cfg, err := DefaultConfigFromEnv("")
if err != nil {
t.Fatal("unexpected error:", err)
}
if cfg.MinVersion != 0 {
t.Errorf("MinVersion = %d, want 0", cfg.MinVersion)
if cfg.MinVersion != cryptotls.VersionTLS13 {
t.Errorf("MinVersion = %d, want %d", cfg.MinVersion, cryptotls.VersionTLS13)
}
if cfg.MaxVersion != 0 {
t.Errorf("MaxVersion = %d, want 0", cfg.MaxVersion)
Expand All @@ -233,9 +233,9 @@ func TestNewConfigFromEnv(t *testing.T) {
}
})

t.Run("min version from env", func(t *testing.T) {
t.Run("min version from env overrides default", func(t *testing.T) {
t.Setenv(MinVersionEnvKey, "1.2")
cfg, err := NewConfigFromEnv("")
cfg, err := DefaultConfigFromEnv("")
if err != nil {
t.Fatal("unexpected error:", err)
}
Expand All @@ -246,7 +246,7 @@ func TestNewConfigFromEnv(t *testing.T) {

t.Run("max version from env", func(t *testing.T) {
t.Setenv(MaxVersionEnvKey, "1.3")
cfg, err := NewConfigFromEnv("")
cfg, err := DefaultConfigFromEnv("")
if err != nil {
t.Fatal("unexpected error:", err)
}
Expand All @@ -257,7 +257,7 @@ func TestNewConfigFromEnv(t *testing.T) {

t.Run("cipher suites from env", func(t *testing.T) {
t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256")
cfg, err := NewConfigFromEnv("")
cfg, err := DefaultConfigFromEnv("")
if err != nil {
t.Fatal("unexpected error:", err)
}
Expand All @@ -268,7 +268,7 @@ func TestNewConfigFromEnv(t *testing.T) {

t.Run("curve preferences from env", func(t *testing.T) {
t.Setenv(CurvePreferencesEnvKey, "X25519,CurveP256")
cfg, err := NewConfigFromEnv("")
cfg, err := DefaultConfigFromEnv("")
if err != nil {
t.Fatal("unexpected error:", err)
}
Expand All @@ -285,7 +285,7 @@ func TestNewConfigFromEnv(t *testing.T) {

t.Run("prefix is prepended to env key", func(t *testing.T) {
t.Setenv("WEBHOOK_TLS_MIN_VERSION", "1.2")
cfg, err := NewConfigFromEnv("WEBHOOK_")
cfg, err := DefaultConfigFromEnv("WEBHOOK_")
if err != nil {
t.Fatal("unexpected error:", err)
}
Expand All @@ -300,7 +300,7 @@ func TestNewConfigFromEnv(t *testing.T) {
t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384")
t.Setenv(CurvePreferencesEnvKey, "X25519,P-256")

cfg, err := NewConfigFromEnv("")
cfg, err := DefaultConfigFromEnv("")
if err != nil {
t.Fatal("unexpected error:", err)
}
Expand All @@ -320,66 +320,33 @@ func TestNewConfigFromEnv(t *testing.T) {

t.Run("invalid min version", func(t *testing.T) {
t.Setenv(MinVersionEnvKey, "1.0")
_, err := NewConfigFromEnv("")
_, err := DefaultConfigFromEnv("")
if err == nil {
t.Fatal("expected error for invalid min version")
}
})

t.Run("invalid max version", func(t *testing.T) {
t.Setenv(MaxVersionEnvKey, "bad")
_, err := NewConfigFromEnv("")
_, err := DefaultConfigFromEnv("")
if err == nil {
t.Fatal("expected error for invalid max version")
}
})

t.Run("invalid cipher suite", func(t *testing.T) {
t.Setenv(CipherSuitesEnvKey, "NOT_A_REAL_CIPHER")
_, err := NewConfigFromEnv("")
_, err := DefaultConfigFromEnv("")
if err == nil {
t.Fatal("expected error for invalid cipher suite")
}
})

t.Run("invalid curve", func(t *testing.T) {
t.Setenv(CurvePreferencesEnvKey, "NotACurve")
_, err := NewConfigFromEnv("")
_, err := DefaultConfigFromEnv("")
if err == nil {
t.Fatal("expected error for invalid curve")
}
})
}

func TestConfig_TLSConfig(t *testing.T) {
t.Setenv(MinVersionEnvKey, "1.2")
t.Setenv(MaxVersionEnvKey, "1.3")
t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256")
t.Setenv(CurvePreferencesEnvKey, "X25519,CurveP256")

cfg, err := NewConfigFromEnv("")
if err != nil {
t.Fatal("unexpected error:", err)
}

tc := cfg.TLSConfig()

if tc.MinVersion != cryptotls.VersionTLS12 {
t.Errorf("MinVersion = %d, want %d", tc.MinVersion, cryptotls.VersionTLS12)
}
if tc.MaxVersion != cryptotls.VersionTLS13 {
t.Errorf("MaxVersion = %d, want %d", tc.MaxVersion, cryptotls.VersionTLS13)
}
if len(tc.CipherSuites) != 1 || tc.CipherSuites[0] != cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 {
t.Errorf("CipherSuites = %v, want [%d]", tc.CipherSuites, cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256)
}
if len(tc.CurvePreferences) != 2 {
t.Fatalf("CurvePreferences has %d entries, want 2", len(tc.CurvePreferences))
}
if tc.CurvePreferences[0] != cryptotls.X25519 {
t.Errorf("CurvePreferences[0] = %d, want %d", tc.CurvePreferences[0], cryptotls.X25519)
}
if tc.CurvePreferences[1] != cryptotls.CurveP256 {
t.Errorf("CurvePreferences[1] = %d, want %d", tc.CurvePreferences[1], cryptotls.CurveP256)
}
}
2 changes: 1 addition & 1 deletion webhook/env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func SecretNameFromEnv(defaultSecretName string) string {
return secret
}

// Deprecated: Use knative.dev/pkg/tls.NewConfigFromEnv instead.
// Deprecated: Use knative.dev/pkg/tls.DefaultConfigFromEnv instead.
// TLS configuration is now read automatically inside webhook.New via the shared tls package.
func TLSMinVersionFromEnv(defaultTLSMinVersion uint16) uint16 {
switch tlsMinVersion := os.Getenv(tlsMinVersionEnvKey); tlsMinVersion {
Expand Down
96 changes: 41 additions & 55 deletions webhook/webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,36 +191,29 @@ func New(

logger := logging.FromContext(ctx)

tlsCfg, err := knativetls.NewConfigFromEnv("WEBHOOK_")
tlsCfg, err := knativetls.DefaultConfigFromEnv("WEBHOOK_")
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
}
if opts.TLSMinVersion != 0 {
tlsCfg.MinVersion = opts.TLSMinVersion
}
if opts.TLSMaxVersion == 0 && tlsCfg.MaxVersion != 0 {
opts.TLSMaxVersion = tlsCfg.MaxVersion
if opts.TLSMaxVersion != 0 {
tlsCfg.MaxVersion = opts.TLSMaxVersion
}
if opts.TLSCipherSuites == nil && len(tlsCfg.CipherSuites) > 0 {
opts.TLSCipherSuites = tlsCfg.CipherSuites
if opts.TLSCipherSuites != nil {
tlsCfg.CipherSuites = opts.TLSCipherSuites
}
if opts.TLSCurvePreferences == nil && len(tlsCfg.CurvePreferences) > 0 {
opts.TLSCurvePreferences = tlsCfg.CurvePreferences
if opts.TLSCurvePreferences != nil {
tlsCfg.CurvePreferences = opts.TLSCurvePreferences
}

if opts.TLSMinVersion != 0 && opts.TLSMinVersion != tls.VersionTLS12 && opts.TLSMinVersion != tls.VersionTLS13 {
return nil, fmt.Errorf("unsupported TLS minimum version %d: must be TLS 1.2 or TLS 1.3", opts.TLSMinVersion)
if tlsCfg.MinVersion != tls.VersionTLS12 && tlsCfg.MinVersion != tls.VersionTLS13 {
return nil, fmt.Errorf("unsupported TLS minimum version %d: must be TLS 1.2 or TLS 1.3", tlsCfg.MinVersion)
}
if opts.TLSMaxVersion != 0 && opts.TLSMinVersion > opts.TLSMaxVersion {
return nil, fmt.Errorf("TLS minimum version (%#x) is greater than maximum version (%#x)", opts.TLSMinVersion, opts.TLSMaxVersion)
if tlsCfg.MaxVersion != 0 && tlsCfg.MinVersion > tlsCfg.MaxVersion {
return nil, fmt.Errorf("TLS minimum version (%#x) is greater than maximum version (%#x)", tlsCfg.MinVersion, tlsCfg.MaxVersion)
}

syncCtx, cancel := context.WithCancel(context.Background())
Expand All @@ -240,42 +233,35 @@ func New(
// a new secret informer from it.
secretInformer := kubeinformerfactory.Get(ctx).Core().V1().Secrets()

//nolint:gosec // operator configures TLS min version (default is 1.3)
webhook.tlsConfig = &tls.Config{
MinVersion: opts.TLSMinVersion,
MaxVersion: opts.TLSMaxVersion,
CipherSuites: opts.TLSCipherSuites,
CurvePreferences: opts.TLSCurvePreferences,

// If we return (nil, error) the client sees - 'tls: internal error"
// If we return (nil, nil) the client sees - 'tls: no certificates configured'
//
// We'll return (nil, nil) when we don't find a certificate
GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
secret, err := secretInformer.Lister().Secrets(system.Namespace()).Get(opts.SecretName)
if err != nil {
logger.Errorw("failed to fetch secret", zap.Error(err))
return nil, nil
}
webOpts := GetOptions(ctx)
sKey, sCert := getSecretDataKeyNamesOrDefault(webOpts.ServerPrivateKeyName, webOpts.ServerCertificateName)
serverKey, ok := secret.Data[sKey]
if !ok {
logger.Warn("server key missing")
return nil, nil
}
serverCert, ok := secret.Data[sCert]
if !ok {
logger.Warn("server cert missing")
return nil, nil
}
cert, err := tls.X509KeyPair(serverCert, serverKey)
if err != nil {
return nil, err
}
return &cert, nil
},
// If we return (nil, error) the client sees - 'tls: internal error'
// If we return (nil, nil) the client sees - 'tls: no certificates configured'
//
// We'll return (nil, nil) when we don't find a certificate
tlsCfg.GetCertificate = func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
secret, err := secretInformer.Lister().Secrets(system.Namespace()).Get(opts.SecretName)
if err != nil {
logger.Errorw("failed to fetch secret", zap.Error(err))
return nil, nil
}
webOpts := GetOptions(ctx)
sKey, sCert := getSecretDataKeyNamesOrDefault(webOpts.ServerPrivateKeyName, webOpts.ServerCertificateName)
serverKey, ok := secret.Data[sKey]
if !ok {
logger.Warn("server key missing")
return nil, nil
}
serverCert, ok := secret.Data[sCert]
if !ok {
logger.Warn("server cert missing")
return nil, nil
}
cert, err := tls.X509KeyPair(serverCert, serverKey)
if err != nil {
return nil, err
}
return &cert, nil
}
webhook.tlsConfig = tlsCfg
}

webhook.mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
Expand Down
Loading