🐛 confusing ManagerOptions#CertDir default

pkg/metrics/server/defaults_test.go

@@ -0,0 +1,42 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package server
+
+import (
+	"testing"
+)
+
+func TestOptionsSetDefaults(t *testing.T) {
+	o := &Options{}
+	o.setDefaults()
+
+	if o.CertDir != "" {
+		t.Errorf("expected CertDir to be empty by default, got %q", o.CertDir)
+	}
+
+	if o.CertName != "tls.crt" {
+		t.Errorf("expected CertName to be tls.crt, got %q", o.CertName)
+	}
+
+	if o.KeyName != "tls.key" {
+		t.Errorf("expected KeyName to be tls.key, got %q", o.KeyName)
+	}
+
+	if o.BindAddress != DefaultBindAddress {
+		t.Errorf("expected BindAddress to be %q, got %q", DefaultBindAddress, o.BindAddress)
+	}
+}

pkg/metrics/server/server.go

@@ -82,9 +82,12 @@ type Options struct {
 	// endpoint by setting this field to filters.WithAuthenticationAndAuthorization.
 	FilterProvider func(c *rest.Config, httpClient *http.Client) (Filter, error)
 
-	// CertDir is the directory that contains the server key and certificate. Defaults to
-	// <temp-dir>/k8s-metrics-server/serving-certs.
-	//
+	// CertDir is the directory that contains the server key and certificate.
+	// If not set, the webhook server will look up the server key and certificate in
+	// the following preference order:
+	// 1. {TempDir}/k8s-metrics-server/serving-certs (for backward compatibility)
+	// 2. The current working directory
+	// The server key and certificate must be named tls.key and tls.crt, respectively.
 	// Note: This option is only used when TLSOpts does not set GetCertificate.
 	// Note: If certificate or key doesn't exist a self-signed certificate will be used.
 	CertDir string
@@ -166,9 +169,8 @@ func (o *Options) setDefaults() {
 		o.BindAddress = DefaultBindAddress
 	}
 
-	if len(o.CertDir) == 0 {
-		o.CertDir = filepath.Join(os.TempDir(), "k8s-metrics-server", "serving-certs")
-	}
+	// If CertDir is empty, we don't default it to the temp dir anymore.
+	// We will check for the certs in the current directory if they are not provided.
 
 	if len(o.CertName) == 0 {
 		o.CertName = "tls.crt"
@@ -284,6 +286,25 @@ func (s *defaultServer) createListener(ctx context.Context, log logr.Logger) (ne
 	}
 
 	if cfg.GetCertificate == nil {
+		// If CertDir is not specified, check the following in order:
+		// 1. The legacy default directory {TempDir}/k8s-metrics-server/serving-certs
+		// 2. The current working directory
+		if len(s.options.CertDir) == 0 {
+			tempDir := filepath.Join(os.TempDir(), "k8s-metrics-server", "serving-certs")
+			certPath := filepath.Join(tempDir, s.options.CertName)
+			keyPath := filepath.Join(tempDir, s.options.KeyName)
+
+			// Simple check for existence.
+			// If both exist, we'll use the temp dir and log a warning.
+			// Otherwise we'll fallback to the CWD (and then eventually self-signed).
+			_, certErr := os.Stat(certPath)
+			_, keyErr := os.Stat(keyPath)
+			if certErr == nil && keyErr == nil {
+				log.Info("WARNING: usage of the default certificate directory is deprecated and will be removed in future versions. Please properly configure the Certificate Directory", "directory", tempDir)
+				s.options.CertDir = tempDir
+			}
+		}
+
 		certPath := filepath.Join(s.options.CertDir, s.options.CertName)
 		keyPath := filepath.Join(s.options.CertDir, s.options.KeyName)
 

pkg/webhook/defaults_test.go

@@ -0,0 +1,42 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package webhook
+
+import (
+	"testing"
+)
+
+func TestOptionsSetDefaults(t *testing.T) {
+	o := &Options{}
+	o.setDefaults()
+
+	if o.CertDir != "" {
+		t.Errorf("expected CertDir to be empty by default, got %q", o.CertDir)
+	}
+
+	if o.CertName != "tls.crt" {
+		t.Errorf("expected CertName to be tls.crt, got %q", o.CertName)
+	}
+
+	if o.KeyName != "tls.key" {
+		t.Errorf("expected KeyName to be tls.key, got %q", o.KeyName)
+	}
+
+	if o.Port != DefaultPort {
+		t.Errorf("expected Port to be %d, got %d", DefaultPort, o.Port)
+	}
+}

pkg/webhook/server.go

@@ -77,8 +77,12 @@ type Options struct {
 	// It will be defaulted to 9443 if unspecified.
 	Port int
 
-	// CertDir is the directory that contains the server key and certificate. Defaults to
-	// <temp-dir>/k8s-webhook-server/serving-certs.
+	// CertDir is the directory that contains the server key and certificate.
+	// If not set, the webhook server will look up the server key and certificate in
+	// the following preference order:
+	// 1. {TempDir}/k8s-webhook-server/serving-certs (for backward compatibility)
+	// 2. The current working directory
+	// The server key and certificate must be named tls.key and tls.crt, respectively.
 	CertDir string
 
 	// CertName is the server certificate name. Defaults to tls.crt.
@@ -140,9 +144,8 @@ func (o *Options) setDefaults() {
 		o.Port = DefaultPort
 	}
 
-	if len(o.CertDir) == 0 {
-		o.CertDir = filepath.Join(os.TempDir(), "k8s-webhook-server", "serving-certs")
-	}
+	// If CertDir is empty, we don't default it to the temp dir anymore.
+	// We will check for the certs in the current directory if they are not provided.
 
 	if len(o.CertName) == 0 {
 		o.CertName = "tls.crt"
@@ -199,6 +202,34 @@ func (s *DefaultServer) Start(ctx context.Context) error {
 	}
 
 	if cfg.GetCertificate == nil {
+		// If CertDir is not specified, check the following in order:
+		// 1. The legacy default directory {TempDir}/k8s-webhook-server/serving-certs
+		// 2. The current working directory
+		if len(s.Options.CertDir) == 0 {
+			tempDir := filepath.Join(os.TempDir(), "k8s-webhook-server", "serving-certs")
+			certPath := filepath.Join(tempDir, s.Options.CertName)
+			keyPath := filepath.Join(tempDir, s.Options.KeyName)
+
+			// Simple check for existence.
+			// If both exist, we'll use the temp dir and log a warning.
+			// Otherwise we'll fallback to the CWD.
+			_, certErr := os.Stat(certPath)
+			_, keyErr := os.Stat(keyPath)
+			if certErr == nil && keyErr == nil {
+				log.Info("WARNING: usage of the default certificate directory is deprecated and will be removed in future versions. Please properly configure the Certificate Directory", "directory", tempDir)
+				s.Options.CertDir = tempDir
+			} else {
+				// We're falling back to CWD, which is the new default.
+				// However, if the certs aren't there either, we should warn the user
+				// so they know *why* we're failing (since they might expect the old default).
+				_, cwdCertErr := os.Stat(s.Options.CertName)
+				_, cwdKeyErr := os.Stat(s.Options.KeyName)
+				if os.IsNotExist(cwdCertErr) || os.IsNotExist(cwdKeyErr) {
+					log.Info("WARNING: The legacy default certificate directory does not exist or is missing certs, so we are checking the current directory. However, we could not find the required certificates in the current directory either. The webhook server will likely fail to start.", "certName", s.Options.CertName, "keyName", s.Options.KeyName)
+				}
+			}
+		}
+
 		certPath := filepath.Join(s.Options.CertDir, s.Options.CertName)
 		keyPath := filepath.Join(s.Options.CertDir, s.Options.KeyName)