From 214f338ec3f858f66ffdde85cba728e51300947c Mon Sep 17 00:00:00 2001
From: Kumar Mallikarjuna <kumarmallikarjuna1@gmail.com>
Date: Thu, 6 Jan 2022 14:41:16 +0530
Subject: [PATCH] Fix TLS inconsitency in HA (#2910)
* Fix TLS inconsitency in HA
Signed-off-by: Kumar Mallikarjuna <kumar@nirmata.com>
* Add error checks
Signed-off-by: Kumar Mallikarjuna <kumar@nirmata.com>
* Remove rendundant err definitions
Signed-off-by: Kumar Mallikarjuna <kumar@nirmata.com>
* Handle all Secret errors
Signed-off-by: Kumar Mallikarjuna <kumar@nirmata.com>
---
pkg/tls/certRenewer.go | 153 ++++++++++++++++++++----------
pkg/tls/reader.go | 35 +++++++
pkg/webhookconfig/registration.go | 5 +-
3 files changed, 142 insertions(+), 51 deletions(-)
diff --git a/pkg/tls/certRenewer.go b/pkg/tls/certRenewer.go
index 342d72333f..54158fe095 100644
--- a/pkg/tls/certRenewer.go
+++ b/pkg/tls/certRenewer.go
@@ -14,6 +14,7 @@ import (
client "github.com/kyverno/kyverno/pkg/dclient"
"github.com/pkg/errors"
v1 "k8s.io/api/core/v1"
+ k8errors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/rest"
@@ -21,7 +22,8 @@ import (
const (
// ManagedByLabel is added to Kyverno managed secrets
- ManagedByLabel string = "cert.kyverno.io/managed-by"
+ ManagedByLabel string = "cert.kyverno.io/managed-by"
+ MasterDeploymentUID string = "cert.kyverno.io/master-deployment-uid"
SelfSignedAnnotation string = "self-signed-cert"
RootCAKey string = "rootCA.crt"
@@ -117,32 +119,58 @@ func (c *CertRenewer) WriteCACertToSecret(caPEM *PemPair, props CertificateProps
logger := c.log.WithName("CAcert")
name := generateRootCASecretName(props)
- secretUnstr, err := c.client.GetResource("", "Secret", props.Namespace, name)
- if err != nil {
- secret := &v1.Secret{
- TypeMeta: metav1.TypeMeta{
- Kind: "Secret",
- APIVersion: "v1",
- },
- ObjectMeta: metav1.ObjectMeta{
- Name: name,
- Namespace: props.Namespace,
- Annotations: map[string]string{
- SelfSignedAnnotation: "true",
- },
- Labels: map[string]string{
- ManagedByLabel: "kyverno",
- },
- },
- Data: map[string][]byte{
- RootCAKey: caPEM.Certificate,
- },
- Type: v1.SecretTypeOpaque,
- }
+ depl, err := c.client.GetResource("", "Deployment", props.Namespace, config.KyvernoDeploymentName)
- _, err := c.client.CreateResource("", "Secret", props.Namespace, secret, false)
+ deplHash := ""
+ if err == nil {
+ deplHash = fmt.Sprintf("%v", depl.GetUID())
+ }
+
+ var deplHashSec string = "default"
+ var ok, managedByKyverno bool
+
+ secretUnstr, err := c.client.GetResource("", "Secret", props.Namespace, name)
+ if err == nil {
+ if label, ok := secretUnstr.GetLabels()[ManagedByLabel]; ok {
+ managedByKyverno = label == "kyverno"
+ }
+ deplHashSec, ok = secretUnstr.GetAnnotations()[MasterDeploymentUID]
+ }
+
+ secret := &v1.Secret{
+ TypeMeta: metav1.TypeMeta{
+ Kind: "Secret",
+ APIVersion: "v1",
+ },
+ ObjectMeta: metav1.ObjectMeta{
+ Name: name,
+ Namespace: props.Namespace,
+ Annotations: map[string]string{
+ SelfSignedAnnotation: "true",
+ MasterDeploymentUID: deplHash,
+ },
+ Labels: map[string]string{
+ ManagedByLabel: "kyverno",
+ },
+ },
+ Data: map[string][]byte{
+ RootCAKey: caPEM.Certificate,
+ },
+ Type: v1.SecretTypeOpaque,
+ }
+
+ if err != nil {
+ if k8errors.IsNotFound(err) {
+ _, err = c.client.CreateResource("", "Secret", props.Namespace, secret, false)
+ if err == nil {
+ logger.Info("secret created", "name", name, "namespace", props.Namespace)
+ }
+ }
+ return err
+ } else if managedByKyverno && (!ok || deplHashSec != deplHash) {
+ _, err = c.client.UpdateResource("", "Secret", props.Namespace, secret, false)
if err == nil {
- logger.Info("secret created", "name", name, "namespace", props.Namespace)
+ logger.Info("secret updated", "name", name, "namespace", props.Namespace)
}
return err
}
@@ -154,7 +182,7 @@ func (c *CertRenewer) WriteCACertToSecret(caPEM *PemPair, props CertificateProps
dataMap := map[string]interface{}{
RootCAKey: base64.StdEncoding.EncodeToString(caPEM.Certificate)}
- if err := unstructured.SetNestedMap(secretUnstr.Object, dataMap, "data"); err != nil {
+ if err = unstructured.SetNestedMap(secretUnstr.Object, dataMap, "data"); err != nil {
return err
}
@@ -172,30 +200,59 @@ func (c *CertRenewer) WriteTLSPairToSecret(props CertificateProps, pemPair *PemP
logger := c.log.WithName("WriteTLSPair")
name := generateTLSPairSecretName(props)
- secretUnstr, err := c.client.GetResource("", "Secret", props.Namespace, name)
- if err != nil {
- secret := &v1.Secret{
- TypeMeta: metav1.TypeMeta{
- Kind: "Secret",
- APIVersion: "v1",
- },
- ObjectMeta: metav1.ObjectMeta{
- Name: name,
- Namespace: props.Namespace,
- Labels: map[string]string{
- ManagedByLabel: "kyverno",
- },
- },
- Data: map[string][]byte{
- v1.TLSCertKey: pemPair.Certificate,
- v1.TLSPrivateKeyKey: pemPair.PrivateKey,
- },
- Type: v1.SecretTypeTLS,
- }
- _, err := c.client.CreateResource("", "Secret", props.Namespace, secret, false)
+ depl, err := c.client.GetResource("", "Deployment", props.Namespace, config.KyvernoDeploymentName)
+
+ deplHash := ""
+ if err == nil {
+ deplHash = fmt.Sprintf("%v", depl.GetUID())
+ }
+
+ var deplHashSec string = "default"
+ var ok, managedByKyverno bool
+
+ secretUnstr, err := c.client.GetResource("", "Secret", props.Namespace, name)
+ if err == nil {
+ if label, ok := secretUnstr.GetLabels()[ManagedByLabel]; ok {
+ managedByKyverno = label == "kyverno"
+ }
+ deplHashSec, ok = secretUnstr.GetAnnotations()[MasterDeploymentUID]
+ }
+
+ secretPtr := &v1.Secret{
+ TypeMeta: metav1.TypeMeta{
+ Kind: "Secret",
+ APIVersion: "v1",
+ },
+ ObjectMeta: metav1.ObjectMeta{
+ Name: name,
+ Namespace: props.Namespace,
+ Annotations: map[string]string{
+ MasterDeploymentUID: deplHash,
+ },
+ Labels: map[string]string{
+ ManagedByLabel: "kyverno",
+ },
+ },
+ Data: map[string][]byte{
+ v1.TLSCertKey: pemPair.Certificate,
+ v1.TLSPrivateKeyKey: pemPair.PrivateKey,
+ },
+ Type: v1.SecretTypeTLS,
+ }
+
+ if err != nil {
+ if k8errors.IsNotFound(err) {
+ _, err = c.client.CreateResource("", "Secret", props.Namespace, secretPtr, false)
+ if err == nil {
+ logger.Info("secret created", "name", name, "namespace", props.Namespace)
+ }
+ }
+ return err
+ } else if managedByKyverno && (!ok || deplHashSec != deplHash) {
+ _, err = c.client.UpdateResource("", "Secret", props.Namespace, secretPtr, false)
if err == nil {
- logger.Info("secret created", "name", name, "namespace", props.Namespace)
+ logger.Info("secret updated", "name", name, "namespace", props.Namespace)
}
return err
}
diff --git a/pkg/tls/reader.go b/pkg/tls/reader.go
index 05ddeccc85..e8ca0fedbf 100644
--- a/pkg/tls/reader.go
+++ b/pkg/tls/reader.go
@@ -22,12 +22,30 @@ func ReadRootCASecret(restConfig *rest.Config, client *client.Client) (result []
return nil, errors.Wrap(err, "failed to get TLS Cert Properties")
}
+ depl, err := client.GetResource("", "Deployment", certProps.Namespace, config.KyvernoDeploymentName)
+
+ deplHash := ""
+ if err == nil {
+ deplHash = fmt.Sprintf("%v", depl.GetUID())
+ }
+
+ var deplHashSec string = "default"
+ var ok, managedByKyverno bool
+
sname := generateRootCASecretName(certProps)
stlsca, err := client.GetResource("", "Secret", certProps.Namespace, sname)
if err != nil {
return nil, err
}
+ if label, ok := stlsca.GetLabels()[ManagedByLabel]; ok {
+ managedByKyverno = label == "kyverno"
+ }
+ deplHashSec, ok = stlsca.GetAnnotations()[MasterDeploymentUID]
+ if managedByKyverno && (!ok || deplHashSec != deplHash) {
+ return nil, fmt.Errorf("outdated secret")
+ }
+
tlsca, err := convertToSecret(stlsca)
if err != nil {
return nil, errors.Wrapf(err, "failed to convert secret %s/%s", certProps.Namespace, sname)
@@ -48,11 +66,28 @@ func ReadTLSPair(restConfig *rest.Config, client *client.Client) (*PemPair, erro
return nil, errors.Wrap(err, "failed to get TLS Cert Properties")
}
+ depl, err := client.GetResource("", "Deployment", certProps.Namespace, config.KyvernoDeploymentName)
+
+ deplHash := ""
+ if err == nil {
+ deplHash = fmt.Sprintf("%v", depl.GetUID())
+ }
+
+ var deplHashSec string = "default"
+ var ok, managedByKyverno bool
+
sname := generateTLSPairSecretName(certProps)
unstrSecret, err := client.GetResource("", "Secret", certProps.Namespace, sname)
if err != nil {
return nil, fmt.Errorf("failed to get secret %s/%s: %v", certProps.Namespace, sname, err)
}
+ if label, ok := unstrSecret.GetLabels()[ManagedByLabel]; ok {
+ managedByKyverno = label == "kyverno"
+ }
+ deplHashSec, ok = unstrSecret.GetAnnotations()[MasterDeploymentUID]
+ if managedByKyverno && (!ok || deplHashSec != deplHash) {
+ return nil, fmt.Errorf("outdated secret")
+ }
// 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
diff --git a/pkg/webhookconfig/registration.go b/pkg/webhookconfig/registration.go
index d25ddc997c..a9825e5ea1 100644
--- a/pkg/webhookconfig/registration.go
+++ b/pkg/webhookconfig/registration.go
@@ -248,10 +248,9 @@ func (wrc *Register) cleanupKyvernoResource() bool {
logger := wrc.log.WithName("cleanupKyvernoResource")
deploy, err := wrc.client.GetResource("", "Deployment", deployNamespace, deployName)
if err != nil {
- logger.Error(err, "failed to get deployment, cleanup kyverno resources anyway")
- return true
+ logger.Error(err, "failed to get deployment, not cleaning up kyverno resources")
+ return false
}
-
if deploy.GetDeletionTimestamp() != nil {
logger.Info("Kyverno is terminating, cleanup Kyverno resources")
return true