Merge pull request #856 from enj/enj/i/impersonation_proxy_signer_expiration

Do not rotate impersonation proxy signer CA unless necessary

internal/controller/apicerts/certs_expirer.go

@@ -14,11 +14,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	corev1informers "k8s.io/client-go/informers/core/v1"
 	"k8s.io/client-go/kubernetes"
-	"k8s.io/klog/v2"
 
 	"go.pinniped.dev/internal/constable"
 	pinnipedcontroller "go.pinniped.dev/internal/controller"
 	"go.pinniped.dev/internal/controllerlib"
+	"go.pinniped.dev/internal/plog"
 )
 
 type certsExpirerController struct {
@@ -74,7 +74,13 @@ func (c *certsExpirerController) Sync(ctx controllerlib.Context) error {
 		return fmt.Errorf("failed to get %s/%s secret: %w", c.namespace, c.certsSecretResourceName, err)
 	}
 	if notFound {
-		klog.Info("certsExpirerController Sync found that the secret does not exist yet or was deleted")
+		plog.Info("secret does not exist yet or was deleted",
+			"controller", ctx.Name,
+			"namespace", c.namespace,
+			"name", c.certsSecretResourceName,
+			"key", c.secretKey,
+			"renewBefore", c.renewBefore.String(),
+		)
 		return nil
 	}
 
@@ -85,7 +91,17 @@ func (c *certsExpirerController) Sync(ctx controllerlib.Context) error {
 
 	certAge := time.Since(notBefore)
 	renewDelta := certAge - c.renewBefore
-	klog.Infof("certsExpirerController Sync found a renew delta of %s", renewDelta)
+	plog.Debug("found renew delta",
+		"controller", ctx.Name,
+		"namespace", c.namespace,
+		"name", c.certsSecretResourceName,
+		"key", c.secretKey,
+		"renewBefore", c.renewBefore.String(),
+		"notBefore", notBefore.String(),
+		"notAfter", notAfter.String(),
+		"certAge", certAge.String(),
+		"renewDelta", renewDelta.String(),
+	)
 	if renewDelta >= 0 || time.Now().After(notAfter) {
 		err := c.k8sClient.
 			CoreV1().
@@ -107,9 +123,7 @@ func (c *certsExpirerController) Sync(ctx controllerlib.Context) error {
 }
 
 // getCertBounds returns the NotBefore and NotAfter fields of the TLS
-// certificate in the provided secret, or an error. Not that it expects the
+// certificate in the provided secret, or an error.
-// provided secret to contain the well-known data keys from this package (see
-// certs_manager.go).
 func (c *certsExpirerController) getCertBounds(secret *corev1.Secret) (time.Time, time.Time, error) {
 	certPEM := secret.Data[c.secretKey]
 	if certPEM == nil {

internal/controller/impersonatorconfig/impersonator_config.go

@@ -51,7 +51,7 @@ const (
 	impersonationProxyPort       = 8444
 	defaultHTTPSPort             = 443
 	approximatelyOneHundredYears = 100 * 365 * 24 * time.Hour
-	caCommonName                 = "Pinniped Impersonation Proxy CA"
+	caCommonName                 = "Pinniped Impersonation Proxy Serving CA"
 	caCrtKey                     = "ca.crt"
 	caKeyKey                     = "ca.key"
 	appLabelKey                  = "app"

internal/controller/impersonatorconfig/impersonator_config_test.go

@@ -1055,7 +1055,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			require.NotNil(t, block)
 			caCert, err := x509.ParseCertificate(block.Bytes)
 			require.NoError(t, err)
-			require.Equal(t, "Pinniped Impersonation Proxy CA", caCert.Subject.CommonName)
+			require.Equal(t, "Pinniped Impersonation Proxy Serving CA", caCert.Subject.CommonName)
 			require.WithinDuration(t, time.Now().Add(-5*time.Minute), caCert.NotBefore, 10*time.Second)
 			require.WithinDuration(t, time.Now().Add(100*time.Hour*24*365), caCert.NotAfter, 10*time.Second)
 			return createdCertPEM

internal/controllermanager/prepare_controllers.go

@@ -148,7 +148,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) {
 				controllerlib.WithInformer,
 				controllerlib.WithInitialEvent,
 				c.ServingCertDuration,
-				"Pinniped CA",
+				"Pinniped Aggregation CA",
 				c.NamesConfig.APIService,
 			),
 			singletonWorker,
@@ -285,7 +285,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) {
 				controllerlib.WithInformer,
 				controllerlib.WithInitialEvent,
 				365*24*time.Hour, // 1 year hard coded value
-				"Pinniped Impersonation Proxy CA",
+				"Pinniped Impersonation Proxy Signer CA",
 				"", // optional, means do not give me a serving cert
 			),
 			singletonWorker,
@@ -297,7 +297,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) {
 				client.Kubernetes,
 				informers.installationNamespaceK8s.Core().V1().Secrets(),
 				controllerlib.WithInformer,
-				c.ServingCertRenewBefore,
+				365*24*time.Hour-time.Hour, // 1 year minus 1 hour hard coded value (i.e. wait until the last moment to break the signer)
 				apicerts.CACertificateSecretKey,
 			),
 			singletonWorker,