Browse Source

Changed logic of Webhook check for certs.

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Gustavo Carvalho 4 years ago
parent
commit
1d8cfc4a12

+ 5 - 5
apis/externalsecrets/v1beta1/externalsecret_types.go

@@ -131,7 +131,7 @@ type ExternalSecretDataRemoteRef struct {
 }
 }
 
 
 type ExternalSecretDataFromRemoteRef struct {
 type ExternalSecretDataFromRemoteRef struct {
-	// Used to select a specific version and property from the secret
+	// Used to extract multiple key/value pairs from one secret
 	// +optional
 	// +optional
 	Extract ExternalSecretDataRemoteRef `json:"extract,omitempty"`
 	Extract ExternalSecretDataRemoteRef `json:"extract,omitempty"`
 	// Used to find secrets based on tags or regular expressions
 	// Used to find secrets based on tags or regular expressions
@@ -140,17 +140,17 @@ type ExternalSecretDataFromRemoteRef struct {
 }
 }
 
 
 type ExternalSecretFind struct {
 type ExternalSecretFind struct {
-	// Key is the key used in the Provider
+	// Finds secrets based on the name.
 	// +optional
 	// +optional
-	Name FindName `json:"name,omitempty"`
+	Name *FindName `json:"name,omitempty"`
 
 
-	// Used to select a specific version of the Provider value, if supported
+	// Find secrets based on tags.
 	// +optional
 	// +optional
 	Tags map[string]string `json:"tags,omitempty"`
 	Tags map[string]string `json:"tags,omitempty"`
 }
 }
 
 
 type FindName struct {
 type FindName struct {
-	// Used to select multiple secrets based on a regular expression of the name
+	// Finds secrets base
 	// +optional
 	// +optional
 	RegExp string `json:"regexp,omitempty"`
 	RegExp string `json:"regexp,omitempty"`
 }
 }

+ 5 - 1
apis/externalsecrets/v1beta1/zz_generated.deepcopy.go

@@ -437,7 +437,11 @@ func (in *ExternalSecretDataRemoteRef) DeepCopy() *ExternalSecretDataRemoteRef {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ExternalSecretFind) DeepCopyInto(out *ExternalSecretFind) {
 func (in *ExternalSecretFind) DeepCopyInto(out *ExternalSecretFind) {
 	*out = *in
 	*out = *in
-	out.Name = in.Name
+	if in.Name != nil {
+		in, out := &in.Name, &out.Name
+		*out = new(FindName)
+		**out = **in
+	}
 	if in.Tags != nil {
 	if in.Tags != nil {
 		in, out := &in.Tags, &out.Tags
 		in, out := &in.Tags, &out.Tags
 		*out = make(map[string]string, len(*in))
 		*out = make(map[string]string, len(*in))

+ 5 - 7
config/crds/bases/external-secrets.io_externalsecrets.yaml

@@ -316,8 +316,8 @@ spec:
                 items:
                 items:
                   properties:
                   properties:
                     extract:
                     extract:
-                      description: Used to select a specific version and property
-                        from the secret
+                      description: Used to extract multiple key/value pairs from one
+                        secret
                       properties:
                       properties:
                         key:
                         key:
                           description: Key is the key used in the Provider, mandatory
                           description: Key is the key used in the Provider, mandatory
@@ -337,18 +337,16 @@ spec:
                       description: Used to find secrets based on tags or regular expressions
                       description: Used to find secrets based on tags or regular expressions
                       properties:
                       properties:
                         name:
                         name:
-                          description: Key is the key used in the Provider
+                          description: Finds secrets based on the name.
                           properties:
                           properties:
                             regexp:
                             regexp:
-                              description: Used to select multiple secrets based on
-                                a regular expression of the name
+                              description: Finds secrets base
                               type: string
                               type: string
                           type: object
                           type: object
                         tags:
                         tags:
                           additionalProperties:
                           additionalProperties:
                             type: string
                             type: string
-                          description: Used to select a specific version of the Provider
-                            value, if supported
+                          description: Find secrets based on tags.
                           type: object
                           type: object
                       type: object
                       type: object
                   type: object
                   type: object

+ 1 - 1
deploy/charts/external-secrets/values.yaml

@@ -89,7 +89,7 @@ priorityClassName: ""
 
 
 webhook:
 webhook:
   replicaCount: 1
   replicaCount: 1
-  certDir: /tmp/k8s-webhook-server/serving-certs
+  certDir: /tmp/certs
   image:
   image:
     repository: ghcr.io/external-secrets/external-secrets-webhook
     repository: ghcr.io/external-secrets/external-secrets-webhook
     pullPolicy: IfNotPresent
     pullPolicy: IfNotPresent

+ 4 - 4
deploy/crds/bundle.yaml

@@ -2013,7 +2013,7 @@ spec:
                   items:
                   items:
                     properties:
                     properties:
                       extract:
                       extract:
-                        description: Used to select a specific version and property from the secret
+                        description: Used to extract multiple key/value pairs from one secret
                         properties:
                         properties:
                           key:
                           key:
                             description: Key is the key used in the Provider, mandatory
                             description: Key is the key used in the Provider, mandatory
@@ -2031,16 +2031,16 @@ spec:
                         description: Used to find secrets based on tags or regular expressions
                         description: Used to find secrets based on tags or regular expressions
                         properties:
                         properties:
                           name:
                           name:
-                            description: Key is the key used in the Provider
+                            description: Finds secrets based on the name.
                             properties:
                             properties:
                               regexp:
                               regexp:
-                                description: Used to select multiple secrets based on a regular expression of the name
+                                description: Finds secrets base
                                 type: string
                                 type: string
                             type: object
                             type: object
                           tags:
                           tags:
                             additionalProperties:
                             additionalProperties:
                               type: string
                               type: string
-                            description: Used to select a specific version of the Provider value, if supported
+                            description: Find secrets based on tags.
                             type: object
                             type: object
                         type: object
                         type: object
                     type: object
                     type: object

+ 36 - 2
pkg/controllers/crds/crds_controller.go

@@ -48,7 +48,7 @@ const (
 	caCertName           = "ca.crt"
 	caCertName           = "ca.crt"
 	caKeyName            = "ca.key"
 	caKeyName            = "ca.key"
 	certValidityDuration = 10 * 365 * 24 * time.Hour
 	certValidityDuration = 10 * 365 * 24 * time.Hour
-	lookaheadInterval    = 90 * 24 * time.Hour
+	LookaheadInterval    = 90 * 24 * time.Hour
 )
 )
 
 
 type WebhookType int
 type WebhookType int
@@ -75,6 +75,12 @@ type Reconciler struct {
 	RestartOnSecretRefresh bool
 	RestartOnSecretRefresh bool
 }
 }
 
 
+type CertInfo struct {
+	CertDir  string
+	CertName string
+	KeyName  string
+	CAName   string
+}
 type WebhookInfo struct {
 type WebhookInfo struct {
 	Name string
 	Name string
 	Type WebhookType
 	Type WebhookType
@@ -258,7 +264,7 @@ func ValidCert(caCert, cert, key []byte, dnsName string, at time.Time) (bool, er
 }
 }
 
 
 func lookaheadTime() time.Time {
 func lookaheadTime() time.Time {
-	return time.Now().Add(lookaheadInterval)
+	return time.Now().Add(LookaheadInterval)
 }
 }
 
 
 func (r *Reconciler) validServerCert(caCert, cert, key []byte) bool {
 func (r *Reconciler) validServerCert(caCert, cert, key []byte) bool {
@@ -442,3 +448,31 @@ func (r *Reconciler) writeSecret(cert, key []byte, caArtifacts *KeyPairArtifacts
 	populateSecret(cert, key, caArtifacts, secret)
 	populateSecret(cert, key, caArtifacts, secret)
 	return r.Update(context.Background(), secret)
 	return r.Update(context.Background(), secret)
 }
 }
+
+func CheckCerts(c CertInfo, dnsName string, at time.Time) error {
+	certFile := c.CertDir + "/" + c.CertName
+	_, err := os.Stat(certFile)
+	if err != nil {
+		return err
+	}
+	ca, err := os.ReadFile(c.CertDir + "/" + c.CAName)
+	if err != nil {
+		return err
+	}
+	cert, err := os.ReadFile(c.CertDir + "/" + c.CertName)
+	if err != nil {
+		return err
+	}
+	key, err := os.ReadFile(c.CertDir + "/" + c.KeyName)
+	if err != nil {
+		return err
+	}
+	ok, err := ValidCert(ca, cert, key, dnsName, at)
+	if err != nil {
+		return err
+	}
+	if !ok {
+		return errors.New("certificate is not valid")
+	}
+	return nil
+}

+ 45 - 4
pkg/controllers/crds/crds_controller_test.go

@@ -19,6 +19,7 @@ import (
 	"crypto/rsa"
 	"crypto/rsa"
 	"crypto/x509"
 	"crypto/x509"
 	"encoding/json"
 	"encoding/json"
+	"os"
 	"testing"
 	"testing"
 	"time"
 	"time"
 
 
@@ -37,6 +38,7 @@ const (
 	failedCreateCaCerts     = "could not create ca certificates:%v"
 	failedCreateCaCerts     = "could not create ca certificates:%v"
 	failedCreateServerCerts = "could not create server certificates:%v"
 	failedCreateServerCerts = "could not create server certificates:%v"
 	invalidCerts            = "generated certificates are invalid:%v,%v"
 	invalidCerts            = "generated certificates are invalid:%v,%v"
+	dnsName                 = "foobar"
 )
 )
 
 
 func newReconciler() Reconciler {
 func newReconciler() Reconciler {
@@ -172,7 +174,7 @@ func TestInjectSvcToConversionWebhook(t *testing.T) {
 }
 }
 
 
 func TestInjectCertToConversionWebhook(t *testing.T) {
 func TestInjectCertToConversionWebhook(t *testing.T) {
-	certPEM := []byte("foobar")
+	certPEM := []byte("certFooBar")
 	crd := newCRD()
 	crd := newCRD()
 	crdunmarshalled := make(map[string]interface{})
 	crdunmarshalled := make(map[string]interface{})
 	crdJSON, err := json.Marshal(crd)
 	crdJSON, err := json.Marshal(crd)
@@ -253,7 +255,7 @@ func TestCreateCertPEM(t *testing.T) {
 }
 }
 func TestValidCert(t *testing.T) {
 func TestValidCert(t *testing.T) {
 	rec := newReconciler()
 	rec := newReconciler()
-	rec.dnsName = "foobar"
+	rec.dnsName = dnsName
 	caArtifacts, err := rec.CreateCACert(time.Now(), time.Now().AddDate(1, 0, 0))
 	caArtifacts, err := rec.CreateCACert(time.Now(), time.Now().AddDate(1, 0, 0))
 	if err != nil {
 	if err != nil {
 		t.Fatalf(failedCreateCaCerts, err)
 		t.Fatalf(failedCreateCaCerts, err)
@@ -262,7 +264,7 @@ func TestValidCert(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Errorf(failedCreateServerCerts, err)
 		t.Errorf(failedCreateServerCerts, err)
 	}
 	}
-	ok, err := ValidCert(caArtifacts.CertPEM, certPEM, keyPEM, "foobar", time.Now())
+	ok, err := ValidCert(caArtifacts.CertPEM, certPEM, keyPEM, dnsName, time.Now())
 	if err != nil {
 	if err != nil {
 		t.Errorf("error validating cert: %v", err)
 		t.Errorf("error validating cert: %v", err)
 	}
 	}
@@ -276,7 +278,7 @@ func TestRefreshCertIfNeeded(t *testing.T) {
 	secret := newSecret()
 	secret := newSecret()
 	c := client.NewClientBuilder().WithObjects(&secret).Build()
 	c := client.NewClientBuilder().WithObjects(&secret).Build()
 	rec.Client = c
 	rec.Client = c
-	rec.dnsName = "foobar"
+	rec.dnsName = dnsName
 	caArtifacts, err := rec.CreateCACert(time.Now().AddDate(-1, 0, 0), time.Now().AddDate(0, -1, 0))
 	caArtifacts, err := rec.CreateCACert(time.Now().AddDate(-1, 0, 0), time.Now().AddDate(0, -1, 0))
 	if err != nil {
 	if err != nil {
 		t.Fatalf(failedCreateCaCerts, err)
 		t.Fatalf(failedCreateCaCerts, err)
@@ -301,3 +303,42 @@ func TestRefreshCertIfNeeded(t *testing.T) {
 		t.Error("expected refresh false. got true")
 		t.Error("expected refresh false. got true")
 	}
 	}
 }
 }
+
+func TestCheckCerts(t *testing.T) {
+	rec := newReconciler()
+	rec.dnsName = dnsName
+	caArtifacts, err := rec.CreateCACert(time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2))
+	if err != nil {
+		t.Fatalf(failedCreateCaCerts, err)
+	}
+	certPEM, keyPEM, err := rec.CreateCertPEM(caArtifacts, time.Now(), time.Now().AddDate(0, 0, 1))
+	if err != nil {
+		t.Errorf(failedCreateServerCerts, err)
+	}
+	os.WriteFile("/tmp/ca", caArtifacts.CertPEM, 0644)
+	os.WriteFile("/tmp/tls", certPEM, 0644)
+	os.WriteFile("/tmp/key", keyPEM, 0644)
+	cert := CertInfo{
+		CertDir:  "/tmp",
+		CertName: "tls",
+		CAName:   "ca",
+		KeyName:  "key",
+	}
+	err = CheckCerts(cert, rec.dnsName, time.Now())
+	if err != nil {
+		t.Errorf("error checking valid cert: %v", err)
+	}
+	err = CheckCerts(cert, rec.dnsName, time.Now().AddDate(-1, 0, 0))
+	if err == nil {
+		t.Error("expected failure due to expired certificate, got success")
+	}
+	err = CheckCerts(cert, "wrong", time.Now())
+	if err == nil {
+		t.Error("expected failure due to dns name got, success")
+	}
+	cert.CAName = "wrong"
+	err = CheckCerts(cert, rec.dnsName, time.Now())
+	if err == nil {
+		t.Error("expected failure due to wrong certificate name, got success")
+	}
+}

+ 1 - 1
pkg/controllers/externalsecret/externalsecret_controller.go

@@ -398,7 +398,7 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient p
 	for _, remoteRef := range externalSecret.Spec.DataFrom {
 	for _, remoteRef := range externalSecret.Spec.DataFrom {
 		var secretMap map[string][]byte
 		var secretMap map[string][]byte
 		var err error
 		var err error
-		if len(remoteRef.Find.Tags) > 0 || len(remoteRef.Find.Name.RegExp) > 0 {
+		if len(remoteRef.Find.Tags) > 0 || remoteRef.Find.Name != nil {
 			secretMap, err = providerClient.GetAllSecrets(ctx, remoteRef.Find)
 			secretMap, err = providerClient.GetAllSecrets(ctx, remoteRef.Find)
 			if err != nil {
 			if err != nil {
 				return nil, fmt.Errorf(errGetSecretKey, remoteRef.Extract.Key, externalSecret.Name, err)
 				return nil, fmt.Errorf(errGetSecretKey, remoteRef.Extract.Key, externalSecret.Name, err)

+ 1 - 1
pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -795,7 +795,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		tc.externalSecret.Spec.DataFrom = []esv1beta1.ExternalSecretDataFromRemoteRef{
 		tc.externalSecret.Spec.DataFrom = []esv1beta1.ExternalSecretDataFromRemoteRef{
 			{
 			{
 				Find: esv1beta1.ExternalSecretFind{
 				Find: esv1beta1.ExternalSecretFind{
-					Name: esv1beta1.FindName{
+					Name: &esv1beta1.FindName{
 						RegExp: "foobar",
 						RegExp: "foobar",
 					},
 					},
 				},
 				},

+ 39 - 36
webhook/main.go

@@ -15,8 +15,11 @@ limitations under the License.
 package main
 package main
 
 
 import (
 import (
+	"context"
 	"flag"
 	"flag"
 	"os"
 	"os"
+	"os/signal"
+	"syscall"
 	"time"
 	"time"
 
 
 	"go.uber.org/zap/zapcore"
 	"go.uber.org/zap/zapcore"
@@ -46,40 +49,6 @@ func init() {
 	_ = esv1alpha1.AddToScheme(scheme)
 	_ = esv1alpha1.AddToScheme(scheme)
 }
 }
 
 
-func checkCerts(certDir, dnsName string) {
-	for {
-		setupLog.Info("Checking certs")
-		certFile := certDir + "/tls.crt"
-		_, err := os.Stat(certFile)
-		if err != nil {
-			setupLog.Error(err, "certs not found")
-			os.Exit(1)
-		}
-		ca, err := os.ReadFile(certDir + "/ca.crt")
-		if err != nil {
-			setupLog.Error(err, "error loading ca cert")
-			os.Exit(1)
-		}
-		cert, err := os.ReadFile(certDir + "/tls.crt")
-		if err != nil {
-			setupLog.Error(err, "error loading server cert")
-			os.Exit(1)
-		}
-		key, err := os.ReadFile(certDir + "/tls.key")
-		if err != nil {
-			setupLog.Error(err, "error loading server key")
-			os.Exit(1)
-		}
-		ok, err := crds.ValidCert(ca, cert, key, dnsName, time.Now())
-		if err != nil || !ok {
-			setupLog.Error(err, "certificates are not valid!")
-			os.Exit(1)
-		}
-		setupLog.Info("Certs valid")
-		time.Sleep(time.Hour * 24)
-	}
-}
-
 func main() {
 func main() {
 	var metricsAddr string
 	var metricsAddr string
 	var enableLeaderElection bool
 	var enableLeaderElection bool
@@ -103,14 +72,48 @@ func main() {
 		setupLog.Error(err, "error unmarshalling loglevel")
 		setupLog.Error(err, "error unmarshalling loglevel")
 		os.Exit(1)
 		os.Exit(1)
 	}
 	}
-	go checkCerts(certDir, dnsName)
+	c := crds.CertInfo{
+		CertDir:  certDir,
+		CertName: "tls.crt",
+		KeyName:  "tls.key",
+		CAName:   "ca.crt",
+	}
+
 	logger := zap.New(zap.Level(lvl))
 	logger := zap.New(zap.Level(lvl))
 	ctrl.SetLogger(logger)
 	ctrl.SetLogger(logger)
 
 
+	setupLog.Info("validating certs")
+	err = crds.CheckCerts(c, dnsName, time.Now().Add(time.Hour))
+	if err != nil {
+		setupLog.Error(err, "error checking certs")
+		os.Exit(1)
+	}
+	ctx, cancel := context.WithCancel(context.Background())
+	go func(c crds.CertInfo, dnsName string) {
+		// notify on sigterm and hour
+		sigs := make(chan os.Signal, 1)
+		signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
+		ticker := time.NewTicker(time.Hour)
+		for {
+			select {
+			case <-sigs:
+				cancel()
+			case <-ticker.C:
+				setupLog.Info("validating certs")
+				err = crds.CheckCerts(c, dnsName, time.Now().Add(crds.LookaheadInterval+time.Minute))
+				if err != nil {
+					cancel()
+				}
+				setupLog.Info("certs are valid")
+			}
+		}
+	}(c, dnsName)
+
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:             scheme,
 		Scheme:             scheme,
 		MetricsBindAddress: metricsAddr,
 		MetricsBindAddress: metricsAddr,
 		Port:               9443,
 		Port:               9443,
+		CertDir:            certDir,
 		LeaderElection:     enableLeaderElection,
 		LeaderElection:     enableLeaderElection,
 		LeaderElectionID:   "webhook-controller",
 		LeaderElectionID:   "webhook-controller",
 		Namespace:          namespace,
 		Namespace:          namespace,
@@ -144,7 +147,7 @@ func main() {
 		os.Exit(1)
 		os.Exit(1)
 	}
 	}
 	setupLog.Info("starting manager")
 	setupLog.Info("starting manager")
-	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
+	if err := mgr.Start(ctx); err != nil {
 		setupLog.Error(err, "problem running manager")
 		setupLog.Error(err, "problem running manager")
 		os.Exit(1)
 		os.Exit(1)
 	}
 	}