From 967ad7cb8eabb531eb23620ba54a586644e29b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 10 May 2022 10:58:51 +0200 Subject: [PATCH] refactor: remove the need for self-signed annotation on cert secret (#3850) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- cmd/kyverno/main.go | 6 +- pkg/controllers/certmanager/controller.go | 25 ++---- pkg/tls/certRenewer.go | 94 ++++++++++++----------- pkg/tls/reader.go | 13 ++-- pkg/tls/tls.go | 4 +- 5 files changed, 67 insertions(+), 75 deletions(-) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 02d9a417c2..db730ac0c5 100755 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -330,7 +330,11 @@ func main() { promConfig, ) - certRenewer := tls.NewCertRenewer(kubeClient, clientConfig, tls.CertRenewalInterval, tls.CertValidityDuration, serverIP, log.Log.WithName("CertRenewer")) + certRenewer, err := tls.NewCertRenewer(kubeClient, clientConfig, tls.CertRenewalInterval, tls.CertValidityDuration, serverIP, log.Log.WithName("CertRenewer")) + if err != nil { + setupLog.Error(err, "failed to initialize CertRenewer") + os.Exit(1) + } certManager, err := certmanager.NewController(kubeKyvernoInformer.Core().V1().Secrets(), kubeClient, certRenewer) if err != nil { setupLog.Error(err, "failed to initialize CertManager") diff --git a/pkg/controllers/certmanager/controller.go b/pkg/controllers/certmanager/controller.go index 1387f90b46..20342cfd6f 100644 --- a/pkg/controllers/certmanager/controller.go +++ b/pkg/controllers/certmanager/controller.go @@ -48,31 +48,20 @@ func NewController(secretInformer informerv1.SecretInformer, kubeClient kubernet func (m *controller) addSecretFunc(obj interface{}) { secret := obj.(*v1.Secret) - if secret.GetNamespace() != config.KyvernoNamespace { - return + if secret.GetNamespace() == config.KyvernoNamespace && secret.GetName() == m.renewer.GenerateTLSPairSecretName() { + m.secretQueue <- true } - val, ok := secret.GetAnnotations()[tls.SelfSignedAnnotation] - if !ok || val != "true" { - return - } - m.secretQueue <- true } func (m *controller) updateSecretFunc(oldObj interface{}, newObj interface{}) { old := oldObj.(*v1.Secret) new := newObj.(*v1.Secret) - if new.GetNamespace() != config.KyvernoNamespace { - return + if new.GetNamespace() == config.KyvernoNamespace && new.GetName() == m.renewer.GenerateTLSPairSecretName() { + if !reflect.DeepEqual(old.DeepCopy().Data, new.DeepCopy().Data) { + m.secretQueue <- true + logger.V(4).Info("secret updated, reconciling webhook configurations") + } } - val, ok := new.GetAnnotations()[tls.SelfSignedAnnotation] - if !ok || val != "true" { - return - } - if reflect.DeepEqual(old.DeepCopy().Data, new.DeepCopy().Data) { - return - } - m.secretQueue <- true - logger.V(4).Info("secret updated, reconciling webhook configurations") } func (m *controller) InitTLSPemPair() { diff --git a/pkg/tls/certRenewer.go b/pkg/tls/certRenewer.go index db86e5b759..11a9a26e1f 100644 --- a/pkg/tls/certRenewer.go +++ b/pkg/tls/certRenewer.go @@ -22,10 +22,8 @@ import ( const ( // ManagedByLabel is added to Kyverno managed secrets - ManagedByLabel string = "cert.kyverno.io/managed-by" - MasterDeploymentUID string = "cert.kyverno.io/master-deployment-uid" - - SelfSignedAnnotation string = "self-signed-cert" + ManagedByLabel string = "cert.kyverno.io/managed-by" + MasterDeploymentUID string = "cert.kyverno.io/master-deployment-uid" RootCAKey string = "rootCA.crt" rollingUpdateAnnotation string = "update.kyverno.io/force-rolling-update" ) @@ -38,6 +36,7 @@ type CertRenewer struct { clientConfig *rest.Config certRenewalInterval time.Duration certValidityDuration time.Duration + certProps *CertificateProps // IP address where Kyverno controller runs. Only required if out-of-cluster. serverIP string @@ -46,15 +45,20 @@ type CertRenewer struct { } // NewCertRenewer returns an instance of CertRenewer -func NewCertRenewer(client kubernetes.Interface, clientConfig *rest.Config, certRenewalInterval, certValidityDuration time.Duration, serverIP string, log logr.Logger) *CertRenewer { +func NewCertRenewer(client kubernetes.Interface, clientConfig *rest.Config, certRenewalInterval, certValidityDuration time.Duration, serverIP string, log logr.Logger) (*CertRenewer, error) { + certProps, err := GetTLSCertProps(clientConfig) + if err != nil { + return nil, err + } return &CertRenewer{ client: client, clientConfig: clientConfig, certRenewalInterval: certRenewalInterval, certValidityDuration: certValidityDuration, + certProps: certProps, serverIP: serverIP, log: log, - } + }, nil } func (c *CertRenewer) Client() kubernetes.Interface { @@ -70,11 +74,6 @@ func (c *CertRenewer) ClientConfig() *rest.Config { // Returns struct with key/certificate pair. func (c *CertRenewer) InitTLSPemPair() (*PemPair, error) { logger := c.log.WithName("InitTLSPemPair") - certProps, err := GetTLSCertProps(c.clientConfig) - if err != nil { - return nil, err - } - if valid, err := c.ValidCert(); err == nil && valid { if tlsPair, err := ReadTLSPair(c.clientConfig, c.client); err == nil { logger.Info("using existing TLS key/certificate pair") @@ -85,27 +84,27 @@ func (c *CertRenewer) InitTLSPemPair() (*PemPair, error) { } logger.Info("building key/certificate pair for TLS") - return c.buildTLSPemPairAndWriteToSecrets(certProps, c.serverIP) + return c.buildTLSPemPairAndWriteToSecrets(c.serverIP) } // buildTLSPemPairAndWriteToSecrets Issues TLS certificate for webhook server using self-signed CA cert // Returns signed and approved TLS certificate in PEM format -func (c *CertRenewer) buildTLSPemPairAndWriteToSecrets(props CertificateProps, serverIP string) (*PemPair, error) { +func (c *CertRenewer) buildTLSPemPairAndWriteToSecrets(serverIP string) (*PemPair, error) { caCert, caPEM, err := GenerateCACert(c.certValidityDuration) if err != nil { return nil, err } - if err := c.WriteCACertToSecret(caPEM, props); err != nil { + if err := c.WriteCACertToSecret(caPEM); err != nil { return nil, fmt.Errorf("failed to write CA cert to secret: %v", err) } - tlsPair, err := GenerateCertPem(caCert, props, serverIP, c.certValidityDuration) + tlsPair, err := GenerateCertPem(caCert, c.certProps, serverIP, c.certValidityDuration) if err != nil { return nil, err } - if err = c.WriteTLSPairToSecret(props, tlsPair); err != nil { + if err = c.WriteTLSPairToSecret(tlsPair); err != nil { return nil, fmt.Errorf("unable to save TLS pair to the cluster: %v", err) } @@ -115,18 +114,18 @@ func (c *CertRenewer) buildTLSPemPairAndWriteToSecrets(props CertificateProps, s // ReadTLSPair Reads the pair of TLS certificate and key from the specified secret. // WriteCACertToSecret stores the CA cert in secret -func (c *CertRenewer) WriteCACertToSecret(caPEM *PemPair, props CertificateProps) error { +func (c *CertRenewer) WriteCACertToSecret(caPEM *PemPair) error { logger := c.log.WithName("CAcert") - name := GenerateRootCASecretName(props) + name := c.GenerateRootCASecretName() - depl, err := c.client.AppsV1().Deployments(props.Namespace).Get(context.TODO(), config.KyvernoDeploymentName, metav1.GetOptions{}) + depl, err := c.client.AppsV1().Deployments(c.certProps.Namespace).Get(context.TODO(), config.KyvernoDeploymentName, metav1.GetOptions{}) deplHash := "" if err == nil { deplHash = fmt.Sprintf("%v", depl.GetUID()) } - secret, err := c.client.CoreV1().Secrets(props.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) + secret, err := c.client.CoreV1().Secrets(c.certProps.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) if err != nil { if k8errors.IsNotFound(err) { @@ -137,10 +136,9 @@ func (c *CertRenewer) WriteCACertToSecret(caPEM *PemPair, props CertificateProps }, ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: props.Namespace, + Namespace: c.certProps.Namespace, Annotations: map[string]string{ - SelfSignedAnnotation: "true", - MasterDeploymentUID: deplHash, + MasterDeploymentUID: deplHash, }, Labels: map[string]string{ ManagedByLabel: "kyverno", @@ -151,52 +149,48 @@ func (c *CertRenewer) WriteCACertToSecret(caPEM *PemPair, props CertificateProps }, Type: v1.SecretTypeOpaque, } - _, err = c.client.CoreV1().Secrets(props.Namespace).Create(context.TODO(), secret, metav1.CreateOptions{}) + _, err = c.client.CoreV1().Secrets(c.certProps.Namespace).Create(context.TODO(), secret, metav1.CreateOptions{}) if err == nil { - logger.Info("secret created", "name", name, "namespace", props.Namespace) + logger.Info("secret created", "name", name, "namespace", c.certProps.Namespace) } } return err } else if CanAddAnnotationToSecret(deplHash, secret) { - _, err = c.client.CoreV1().Secrets(props.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) + _, err = c.client.CoreV1().Secrets(c.certProps.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) if err == nil { - logger.Info("secret updated", "name", name, "namespace", props.Namespace) + logger.Info("secret updated", "name", name, "namespace", c.certProps.Namespace) } return err } - if _, ok := secret.GetAnnotations()[SelfSignedAnnotation]; !ok { - secret.SetAnnotations(map[string]string{SelfSignedAnnotation: "true"}) - } - dataMap := map[string][]byte{ RootCAKey: caPEM.Certificate, } secret.Data = dataMap - _, err = c.client.CoreV1().Secrets(props.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) + _, err = c.client.CoreV1().Secrets(c.certProps.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) if err != nil { return err } - logger.Info("secret updated", "name", name, "namespace", props.Namespace) + logger.Info("secret updated", "name", name, "namespace", c.certProps.Namespace) return nil } // WriteTLSPairToSecret Writes the pair of TLS certificate and key to the specified secret. // Updates existing secret or creates new one. -func (c *CertRenewer) WriteTLSPairToSecret(props CertificateProps, pemPair *PemPair) error { +func (c *CertRenewer) WriteTLSPairToSecret(pemPair *PemPair) error { logger := c.log.WithName("WriteTLSPair") - name := GenerateTLSPairSecretName(props) + name := c.GenerateTLSPairSecretName() - depl, err := c.client.AppsV1().Deployments(props.Namespace).Get(context.TODO(), config.KyvernoDeploymentName, metav1.GetOptions{}) + depl, err := c.client.AppsV1().Deployments(c.certProps.Namespace).Get(context.TODO(), config.KyvernoDeploymentName, metav1.GetOptions{}) deplHash := "" if err == nil { deplHash = fmt.Sprintf("%v", depl.GetUID()) } - secret, err := c.client.CoreV1().Secrets(props.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) + secret, err := c.client.CoreV1().Secrets(c.certProps.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) if err != nil { if k8errors.IsNotFound(err) { @@ -207,7 +201,7 @@ func (c *CertRenewer) WriteTLSPairToSecret(props CertificateProps, pemPair *PemP }, ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: props.Namespace, + Namespace: c.certProps.Namespace, Annotations: map[string]string{ MasterDeploymentUID: deplHash, }, @@ -221,16 +215,16 @@ func (c *CertRenewer) WriteTLSPairToSecret(props CertificateProps, pemPair *PemP }, Type: v1.SecretTypeTLS, } - _, err = c.client.CoreV1().Secrets(props.Namespace).Create(context.TODO(), secret, metav1.CreateOptions{}) + _, err = c.client.CoreV1().Secrets(c.certProps.Namespace).Create(context.TODO(), secret, metav1.CreateOptions{}) if err == nil { - logger.Info("secret created", "name", name, "namespace", props.Namespace) + logger.Info("secret created", "name", name, "namespace", c.certProps.Namespace) } } return err } else if CanAddAnnotationToSecret(deplHash, secret) { - _, err = c.client.CoreV1().Secrets(props.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) + _, err = c.client.CoreV1().Secrets(c.certProps.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) if err == nil { - logger.Info("secret updated", "name", name, "namespace", props.Namespace) + logger.Info("secret updated", "name", name, "namespace", c.certProps.Namespace) } return err } @@ -242,12 +236,12 @@ func (c *CertRenewer) WriteTLSPairToSecret(props CertificateProps, pemPair *PemP secret.Data = dataMap - _, err = c.client.CoreV1().Secrets(props.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) + _, err = c.client.CoreV1().Secrets(c.certProps.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) if err != nil { return err } - logger.Info("secret updated", "name", name, "namespace", props.Namespace) + logger.Info("secret updated", "name", name, "namespace", c.certProps.Namespace) return nil } @@ -394,11 +388,19 @@ func IsKyvernoInRollingUpdate(deploy *appsv1.Deployment, logger logr.Logger) boo return false } -func GenerateTLSPairSecretName(props CertificateProps) string { +func (c *CertRenewer) GenerateTLSPairSecretName() string { + return GenerateTLSPairSecretName(c.certProps) +} + +func (c *CertRenewer) GenerateRootCASecretName() string { + return GenerateRootCASecretName(c.certProps) +} + +func GenerateTLSPairSecretName(props *CertificateProps) string { return generateInClusterServiceName(props) + ".kyverno-tls-pair" } -func GenerateRootCASecretName(props CertificateProps) string { +func GenerateRootCASecretName(props *CertificateProps) string { return generateInClusterServiceName(props) + ".kyverno-tls-ca" } diff --git a/pkg/tls/reader.go b/pkg/tls/reader.go index 4956f2e834..77bb2d247c 100644 --- a/pkg/tls/reader.go +++ b/pkg/tls/reader.go @@ -86,8 +86,7 @@ func ReadTLSPair(restConfig *rest.Config, client kubernetes.Interface) (*PemPair // If secret contains annotation 'self-signed-cert', then it's created using helper scripts to setup self-signed certificates. // As the root CA used to sign the certificate is required for webhook configuration, check if the corresponding secret is created - annotations := secret.GetAnnotations() - if _, ok := annotations[SelfSignedAnnotation]; ok { + { sname := GenerateRootCASecretName(certProps) _, err := client.CoreV1().Secrets(certProps.Namespace).Get(context.TODO(), sname, metav1.GetOptions{}) if err != nil { @@ -111,16 +110,14 @@ func ReadTLSPair(restConfig *rest.Config, client kubernetes.Interface) (*PemPair } //GetTLSCertProps provides the TLS Certificate Properties -func GetTLSCertProps(configuration *rest.Config) (certProps CertificateProps, err error) { +func GetTLSCertProps(configuration *rest.Config) (*CertificateProps, error) { apiServerURL, err := url.Parse(configuration.Host) if err != nil { - return certProps, err + return nil, err } - - certProps = CertificateProps{ + return &CertificateProps{ Service: config.KyvernoServiceName, Namespace: config.KyvernoNamespace, APIServerHost: apiServerURL.Hostname(), - } - return certProps, nil + }, nil } diff --git a/pkg/tls/tls.go b/pkg/tls/tls.go index 51b505a533..e3dda52048 100644 --- a/pkg/tls/tls.go +++ b/pkg/tls/tls.go @@ -114,7 +114,7 @@ func GenerateCACert(certValidityDuration time.Duration) (*KeyPair, *PemPair, err // GenerateCertPem takes the results of GenerateCACert and uses it to create the // PEM-encoded public certificate and private key, respectively -func GenerateCertPem(caCert *KeyPair, props CertificateProps, serverIP string, certValidityDuration time.Duration) (*PemPair, error) { +func GenerateCertPem(caCert *KeyPair, props *CertificateProps, serverIP string, certValidityDuration time.Duration) (*PemPair, error) { now := time.Now() begin := now.Add(-1 * time.Hour) end := now.Add(certValidityDuration) @@ -170,7 +170,7 @@ func GenerateCertPem(caCert *KeyPair, props CertificateProps, serverIP string, c } //GenerateInClusterServiceName The generated service name should be the common name for TLS certificate -func generateInClusterServiceName(props CertificateProps) string { +func generateInClusterServiceName(props *CertificateProps) string { return props.Service + "." + props.Namespace + ".svc" }