Skip to content

Commit

Permalink
Remove ValidateSecret and add tests for GetSecretKind
Browse files Browse the repository at this point in the history
  • Loading branch information
lucacome authored Oct 7, 2020
1 parent 72c7ffb commit dc03a96
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 28 deletions.
22 changes: 1 addition & 21 deletions internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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()
Expand Down
12 changes: 8 additions & 4 deletions internal/k8s/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -173,17 +174,20 @@ 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
}

glog.V(3).Infof("Removing Secret: %v", secret.Name)
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
}

Expand Down
12 changes: 9 additions & 3 deletions internal/k8s/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const IngressMTLSKey = "ca.crt"

const (
// TLS Secret
TLS = iota
TLS = iota + 1
// JWK Secret
JWK
// IgressMTLS Secret
Expand All @@ -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)
}

Expand Down
144 changes: 144 additions & 0 deletions internal/k8s/secret_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit dc03a96

Please sign in to comment.