Do not rotate impersonation proxy signer CA unless necessary

This change fixes a copy paste error that led to the impersonation
proxy signer CA being rotated based on the configuration of the
rotation of the aggregated API serving certificate.  This would lead
to occasional "Unauthorized" flakes in our CI environments that
rotate the serving certificate at a frequent interval.

Updated the certs_expirer controller logs to be more detailed.

Updated CA common names to be more specific (this does not update
any previously generated CAs).

Signed-off-by: Monis Khan <mok@vmware.com>
This commit is contained in:
Monis Khan 2021-10-06 11:46:54 -04:00
parent 946419fc18
commit 4bf715758f
No known key found for this signature in database
GPG Key ID: 52C90ADA01B269B8
4 changed files with 25 additions and 11 deletions

View File

@ -14,11 +14,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
corev1informers "k8s.io/client-go/informers/core/v1" corev1informers "k8s.io/client-go/informers/core/v1"
"k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
"go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/constable"
pinnipedcontroller "go.pinniped.dev/internal/controller" pinnipedcontroller "go.pinniped.dev/internal/controller"
"go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/plog"
) )
type certsExpirerController struct { 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) return fmt.Errorf("failed to get %s/%s secret: %w", c.namespace, c.certsSecretResourceName, err)
} }
if notFound { 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 return nil
} }
@ -85,7 +91,17 @@ func (c *certsExpirerController) Sync(ctx controllerlib.Context) error {
certAge := time.Since(notBefore) certAge := time.Since(notBefore)
renewDelta := certAge - c.renewBefore 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) { if renewDelta >= 0 || time.Now().After(notAfter) {
err := c.k8sClient. err := c.k8sClient.
CoreV1(). CoreV1().
@ -107,9 +123,7 @@ func (c *certsExpirerController) Sync(ctx controllerlib.Context) error {
} }
// getCertBounds returns the NotBefore and NotAfter fields of the TLS // 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) { func (c *certsExpirerController) getCertBounds(secret *corev1.Secret) (time.Time, time.Time, error) {
certPEM := secret.Data[c.secretKey] certPEM := secret.Data[c.secretKey]
if certPEM == nil { if certPEM == nil {

View File

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

View File

@ -1055,7 +1055,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
require.NotNil(t, block) require.NotNil(t, block)
caCert, err := x509.ParseCertificate(block.Bytes) caCert, err := x509.ParseCertificate(block.Bytes)
require.NoError(t, err) 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(-5*time.Minute), caCert.NotBefore, 10*time.Second)
require.WithinDuration(t, time.Now().Add(100*time.Hour*24*365), caCert.NotAfter, 10*time.Second) require.WithinDuration(t, time.Now().Add(100*time.Hour*24*365), caCert.NotAfter, 10*time.Second)
return createdCertPEM return createdCertPEM

View File

@ -148,7 +148,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) {
controllerlib.WithInformer, controllerlib.WithInformer,
controllerlib.WithInitialEvent, controllerlib.WithInitialEvent,
c.ServingCertDuration, c.ServingCertDuration,
"Pinniped CA", "Pinniped Aggregation CA",
c.NamesConfig.APIService, c.NamesConfig.APIService,
), ),
singletonWorker, singletonWorker,
@ -285,7 +285,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) {
controllerlib.WithInformer, controllerlib.WithInformer,
controllerlib.WithInitialEvent, controllerlib.WithInitialEvent,
365*24*time.Hour, // 1 year hard coded value 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 "", // optional, means do not give me a serving cert
), ),
singletonWorker, singletonWorker,
@ -297,7 +297,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) {
client.Kubernetes, client.Kubernetes,
informers.installationNamespaceK8s.Core().V1().Secrets(), informers.installationNamespaceK8s.Core().V1().Secrets(),
controllerlib.WithInformer, 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, apicerts.CACertificateSecretKey,
), ),
singletonWorker, singletonWorker,