diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 9975b40678..ffc33e0d38 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -1609,7 +1609,7 @@ func (lbc *LoadBalancerController) handleRegularSecretDeletion(key string, ings func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, ings []networking.Ingress, virtualServers []*conf_v1.VirtualServer) { secretNsName := secret.Namespace + "/" + secret.Name - err := lbc.ValidateSecret(secret) + kind, err := GetSecretKind(secret) if err != nil { // Secret becomes Invalid glog.Errorf("Couldn't validate secret %v: %v", secretNsName, err) @@ -1626,9 +1626,6 @@ func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, ing message := fmt.Sprintf("Configuration was updated due to updated secret %v", secretNsName) state := conf_v1.StateValid - // we can safely ignore the error because the secret is valid in this function - kind, _ := GetSecretKind(secret) - if kind == JWK { virtualServerExes := lbc.virtualServersToVirtualServerExes(virtualServers) @@ -3302,23 +3299,6 @@ func (lbc *LoadBalancerController) isHealthCheckEnabled(ing *networking.Ingress) return false } -// ValidateSecret validates that the secret follows the TLS Secret format. -// For NGINX Plus, it also checks if the secret follows the JWK Secret format. -func (lbc *LoadBalancerController) ValidateSecret(secret *api_v1.Secret) error { - err1 := ValidateTLSSecret(secret) - if !lbc.isNginxPlus { - return err1 - } - - err2 := ValidateJWKSecret(secret) - - if err1 == nil || err2 == nil { - return nil - } - - return fmt.Errorf("Secret is not a TLS or JWK secret") -} - // getMinionsForHost returns a list of all minion ingress resources for a given master func (lbc *LoadBalancerController) getMinionsForMaster(master *configs.IngressEx) ([]*configs.IngressEx, error) { ings, err := lbc.ingressLister.List() diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index df068c4fdc..42864be250 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -153,7 +153,8 @@ func createSecretHandlers(lbc *LoadBalancerController) cache.ResourceEventHandle return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { secret := obj.(*v1.Secret) - if err := lbc.ValidateSecret(secret); err != nil { + if _, err := GetSecretKind(secret); err != nil { + glog.V(3).Infof("Ignoring unknown secret: %v", secret.Name) return } glog.V(3).Infof("Adding Secret: %v", secret.Name) @@ -173,7 +174,8 @@ func createSecretHandlers(lbc *LoadBalancerController) cache.ResourceEventHandle return } } - if err := lbc.ValidateSecret(secret); err != nil { + if _, err := GetSecretKind(secret); err != nil { + glog.V(3).Infof("Ignoring unknown secret: %v", secret.Name) return } @@ -181,9 +183,11 @@ func createSecretHandlers(lbc *LoadBalancerController) cache.ResourceEventHandle lbc.AddSyncQueue(obj) }, UpdateFunc: func(old, cur interface{}) { - errOld := lbc.ValidateSecret(old.(*v1.Secret)) - errCur := lbc.ValidateSecret(cur.(*v1.Secret)) + curSecret := cur.(*v1.Secret) + _, errOld := GetSecretKind(old.(*v1.Secret)) + _, errCur := GetSecretKind(curSecret) if errOld != nil && errCur != nil { + glog.V(3).Infof("Ignoring unknown secret: %v", curSecret.Name) return } diff --git a/internal/k8s/secret.go b/internal/k8s/secret.go index 89eeea0139..beea15b624 100644 --- a/internal/k8s/secret.go +++ b/internal/k8s/secret.go @@ -14,7 +14,7 @@ const IngressMTLSKey = "ca.crt" const ( // TLS Secret - TLS = iota + TLS = iota + 1 // JWK Secret JWK // IgressMTLS Secret @@ -23,11 +23,17 @@ const ( // ValidateTLSSecret validates the secret. If it is valid, the function returns nil. func ValidateTLSSecret(secret *v1.Secret) error { - if _, exists := secret.Data[v1.TLSCertKey]; !exists { + _, certExists := secret.Data[v1.TLSCertKey] + _, keyExists := secret.Data[v1.TLSPrivateKeyKey] + + if certExists && keyExists { + return nil + } + if !certExists { return fmt.Errorf("Secret doesn't have %v", v1.TLSCertKey) } - if _, exists := secret.Data[v1.TLSPrivateKeyKey]; !exists { + if !keyExists { return fmt.Errorf("Secret doesn't have %v", v1.TLSPrivateKeyKey) } diff --git a/internal/k8s/secret_test.go b/internal/k8s/secret_test.go new file mode 100644 index 0000000000..8ecc263213 --- /dev/null +++ b/internal/k8s/secret_test.go @@ -0,0 +1,144 @@ +package k8s + +import ( + "errors" + "testing" + + v1 "k8s.io/api/core/v1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGetSecretKind(t *testing.T) { + + var tests = []struct { + secret *v1.Secret + expected int + }{ + { + secret: &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "tls-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "tls.key": nil, + "tls.crt": nil, + }, + }, + expected: TLS, + }, + { + secret: &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "ingress-mtls-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "ca.crt": nil, + }, + }, + expected: IngressMTLS, + }, { + secret: &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "jwk-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "jwk": nil, + }, + }, + expected: JWK, + }, + } + + for _, test := range tests { + + secret, err := GetSecretKind(test.secret) + if err != nil { + t.Errorf("GetSecretKind() returned an unexpected error: %v", err) + } + if secret != test.expected { + t.Errorf("GetSecretKind() return %v but expected %v", secret, test.expected) + } + } +} + +func TestGetSecretKindUnkown(t *testing.T) { + s := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "foo-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo.bar": nil, + }, + } + e := errors.New("Unknown Secret") + + secret, err := GetSecretKind(s) + if secret != 0 { + t.Errorf("GetSecretKind() returned an unexpected secret: %v", secret) + } + if err.Error() != e.Error() { + t.Errorf("GetSecretKind() return %v but expected %v", err, e) + } + +} + +func TestValidateTLSSecretFail(t *testing.T) { + + s := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "tls-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "tls.crt": nil, + }, + } + e := errors.New("Secret doesn't have tls.key") + + err := ValidateTLSSecret(s) + if err.Error() != e.Error() { + t.Errorf("ValidateTLSSecret() return %v but expected %v", err, e) + } +} + +func TestValidateIngressMTLSSecretFail(t *testing.T) { + + s := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "mtls-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "ca.cert": nil, + }, + } + e := errors.New("Secret doesn't have ca.crt") + + err := ValidateIngressMTLSSecret(s) + if err.Error() != e.Error() { + t.Errorf("ValidateIngressMTLSSecret() return %v but expected %v", err, e) + } +} + +func TestValidateJWKSecretFail(t *testing.T) { + + s := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "mtls-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "jwka": nil, + }, + } + e := errors.New("Secret doesn't have jwk") + + err := ValidateJWKSecret(s) + if err.Error() != e.Error() { + t.Errorf("ValidateJWKSecret() return %v but expected %v", err, e) + } +}