diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index dc6740400b..18136d0bf6 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -18,6 +18,7 @@ import ( "github.com/nginxinc/kubernetes-ingress/internal/configs/version1" "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" "github.com/nginxinc/kubernetes-ingress/internal/k8s" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" "github.com/nginxinc/kubernetes-ingress/internal/metrics" "github.com/nginxinc/kubernetes-ingress/internal/metrics/collectors" "github.com/nginxinc/kubernetes-ingress/internal/nginx" @@ -757,7 +758,7 @@ func getAndValidateSecret(kubeClient *kubernetes.Clientset, secretNsName string) if err != nil { return nil, fmt.Errorf("could not get %v: %v", secretNsName, err) } - err = k8s.ValidateTLSSecret(secret) + err = secrets.ValidateTLSSecret(secret) if err != nil { return nil, fmt.Errorf("%v is invalid: %v", secretNsName, err) } diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 21463029aa..45f30193b0 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -11,6 +11,7 @@ import ( "sort" "strings" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" "github.com/nginxinc/nginx-prometheus-exporter/collector" "github.com/spiffe/go-spiffe/workload" @@ -1283,9 +1284,9 @@ func (cnf *Configurator) AddInternalRouteConfig() error { // AddOrUpdateSecret adds or updates a secret. func (cnf *Configurator) AddOrUpdateSecret(secret *api_v1.Secret) string { switch secret.Type { - case SecretTypeCA: + case secrets.SecretTypeCA: return cnf.addOrUpdateCASecret(secret) - case SecretTypeJWK: + case secrets.SecretTypeJWK: return cnf.addOrUpdateJWKSecret(secret) default: return cnf.addOrUpdateTLSSecret(secret) diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index 959d9607e4..2b718b1ba4 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/golang/glog" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" api_v1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" @@ -30,7 +31,7 @@ type IngressEx struct { AppProtectPolicy *unstructured.Unstructured AppProtectLogConf *unstructured.Unstructured AppProtectLogDst string - SecretRefs map[string]*SecretReference + SecretRefs map[string]*secrets.SecretReference } // JWTKey represents a secret that holds JSON Web Key. @@ -248,12 +249,12 @@ func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion } } -func generateJWTConfig(secretRefs map[string]*SecretReference, cfgParams *ConfigParams, redirectLocationName string) (*version1.JWTAuth, *version1.JWTRedirectLocation) { +func generateJWTConfig(secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams, redirectLocationName string) (*version1.JWTAuth, *version1.JWTRedirectLocation) { secret := secretRefs[cfgParams.JWTKey] if secret.Error != nil { // TO-DO: add a warning - } else if secret.Type != SecretTypeJWK { + } else if secret.Type != secrets.SecretTypeJWK { // TO-DO: add a warning } @@ -278,7 +279,7 @@ func generateJWTConfig(secretRefs map[string]*SecretReference, cfgParams *Config return jwtAuth, redirectLocation } -func addSSLConfig(server *version1.Server, host string, ingressTLS []networking.IngressTLS, secretRefs map[string]*SecretReference, isWildcardEnabled bool) { +func addSSLConfig(server *version1.Server, host string, ingressTLS []networking.IngressTLS, secretRefs map[string]*secrets.SecretReference, isWildcardEnabled bool) { var tlsEnabled bool var tlsSecret string diff --git a/internal/configs/ingress_test.go b/internal/configs/ingress_test.go index 9cbdb81a25..816c1e03f1 100644 --- a/internal/configs/ingress_test.go +++ b/internal/configs/ingress_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" v1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,7 +36,7 @@ func TestGenerateNginxCfgForJWT(t *testing.T) { cafeIngressEx.Ingress.Annotations["nginx.com/jwt-realm"] = "Cafe App" cafeIngressEx.Ingress.Annotations["nginx.com/jwt-token"] = "$cookie_auth_token" cafeIngressEx.Ingress.Annotations["nginx.com/jwt-login-url"] = "https://login.example.com" - cafeIngressEx.SecretRefs["cafe-jwk"] = &SecretReference{ + cafeIngressEx.SecretRefs["cafe-jwk"] = &secrets.SecretReference{ Type: "nginx.org/jwk", Path: "/etc/nginx/secrets/default-cafe-jwk", } @@ -289,7 +290,7 @@ func createCafeIngressEx() IngressEx { ValidHosts: map[string]bool{ "cafe.example.com": true, }, - SecretRefs: map[string]*SecretReference{ + SecretRefs: map[string]*secrets.SecretReference{ "cafe-secret": { Type: v1.SecretTypeTLS, Path: "/etc/nginx/secrets/default-cafe-secret", @@ -341,8 +342,8 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) { mergeableIngresses.Master.Ingress.Annotations["nginx.com/jwt-realm"] = "Cafe" mergeableIngresses.Master.Ingress.Annotations["nginx.com/jwt-token"] = "$cookie_auth_token" mergeableIngresses.Master.Ingress.Annotations["nginx.com/jwt-login-url"] = "https://login.example.com" - mergeableIngresses.Master.SecretRefs["cafe-jwk"] = &SecretReference{ - Type: SecretTypeJWK, + mergeableIngresses.Master.SecretRefs["cafe-jwk"] = &secrets.SecretReference{ + Type: secrets.SecretTypeJWK, Path: "/etc/nginx/secrets/default-cafe-jwk", } @@ -350,8 +351,8 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) { mergeableIngresses.Minions[0].Ingress.Annotations["nginx.com/jwt-realm"] = "Coffee" mergeableIngresses.Minions[0].Ingress.Annotations["nginx.com/jwt-token"] = "$cookie_auth_token_coffee" mergeableIngresses.Minions[0].Ingress.Annotations["nginx.com/jwt-login-url"] = "https://login.cofee.example.com" - mergeableIngresses.Minions[0].SecretRefs["coffee-jwk"] = &SecretReference{ - Type: SecretTypeJWK, + mergeableIngresses.Minions[0].SecretRefs["coffee-jwk"] = &secrets.SecretReference{ + Type: secrets.SecretTypeJWK, Path: "/etc/nginx/secrets/default-coffee-jwk", } @@ -500,7 +501,7 @@ func createMergeableCafeIngress() *MergeableIngresses { ValidHosts: map[string]bool{ "cafe.example.com": true, }, - SecretRefs: map[string]*SecretReference{ + SecretRefs: map[string]*secrets.SecretReference{ "cafe-secret": { Type: v1.SecretTypeTLS, Path: "/etc/nginx/secrets/default-cafe-secret", @@ -520,7 +521,7 @@ func createMergeableCafeIngress() *MergeableIngresses { ValidMinionPaths: map[string]bool{ "/coffee": true, }, - SecretRefs: map[string]*SecretReference{}, + SecretRefs: map[string]*secrets.SecretReference{}, }, { Ingress: &teaMinion, @@ -533,7 +534,7 @@ func createMergeableCafeIngress() *MergeableIngresses { ValidMinionPaths: map[string]bool{ "/tea": true, }, - SecretRefs: map[string]*SecretReference{}, + SecretRefs: map[string]*secrets.SecretReference{}, }}, } @@ -851,7 +852,7 @@ func TestAddSSLConfig(t *testing.T) { tests := []struct { host string tls []networking.IngressTLS - secretRefs map[string]*SecretReference + secretRefs map[string]*secrets.SecretReference isWildcardEnabled bool expected version1.Server msg string @@ -864,7 +865,7 @@ func TestAddSSLConfig(t *testing.T) { SecretName: "cafe-secret", }, }, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "cafe-secret": { Type: v1.SecretTypeTLS, Path: "/etc/nginx/secrets/default-cafe-secret", @@ -882,7 +883,7 @@ func TestAddSSLConfig(t *testing.T) { SecretName: "cafe-secret", }, }, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "cafe-secret": { Type: v1.SecretTypeTLS, Path: "/etc/nginx/secrets/default-cafe-secret", @@ -904,7 +905,7 @@ func TestAddSSLConfig(t *testing.T) { SecretName: "cafe-secret", }, }, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "cafe-secret": { Error: errors.New("invalid secret"), }, @@ -926,9 +927,9 @@ func TestAddSSLConfig(t *testing.T) { SecretName: "cafe-secret", }, }, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "cafe-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Path: "/etc/nginx/secrets/default-cafe-secret", }, }, @@ -989,7 +990,7 @@ func TestAddSSLConfig(t *testing.T) { func TestGenerateJWTConfig(t *testing.T) { tests := []struct { - secretRefs map[string]*SecretReference + secretRefs map[string]*secrets.SecretReference cfgParams *ConfigParams redirectLocationName string expectedJWTAuth *version1.JWTAuth @@ -997,9 +998,9 @@ func TestGenerateJWTConfig(t *testing.T) { msg string }{ { - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "cafe-jwk": { - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, Path: "/etc/nginx/secrets/default-cafe-jwk", }, }, @@ -1018,9 +1019,9 @@ func TestGenerateJWTConfig(t *testing.T) { msg: "normal case", }, { - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "cafe-jwk": { - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, Path: "/etc/nginx/secrets/default-cafe-jwk", }, }, @@ -1044,7 +1045,7 @@ func TestGenerateJWTConfig(t *testing.T) { msg: "normal case with login url", }, { - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "cafe-jwk": { Path: "/etc/nginx/secrets/default-cafe-jwk", Error: errors.New("invalid secret"), @@ -1065,9 +1066,9 @@ func TestGenerateJWTConfig(t *testing.T) { msg: "invalid secret", }, { - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "cafe-jwk": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Path: "/etc/nginx/secrets/default-cafe-jwk", }, }, diff --git a/internal/configs/secret.go b/internal/configs/secret.go deleted file mode 100644 index 9c332791fb..0000000000 --- a/internal/configs/secret.go +++ /dev/null @@ -1,20 +0,0 @@ -package configs - -import api_v1 "k8s.io/api/core/v1" - -// The constants below are defined because we can't use the same constants from the internal/k8s package -// because that will lead to import cycles -// TO-DO: Consider refactoring the internal/k8s package, so that we only define those constants once - -// SecretTypeCA contains a certificate authority for TLS certificate verification. -const SecretTypeCA api_v1.SecretType = "nginx.org/ca" - -// SecretTypeJWK contains a JWK (JSON Web Key) for validating JWTs (JSON Web Tokens). -const SecretTypeJWK api_v1.SecretType = "nginx.org/jwk" - -// SecretReference holds a reference to a secret stored on the file system. -type SecretReference struct { - Type api_v1.SecretType - Path string - Error error -} diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index b87226f067..e3bc944433 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/golang/glog" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" "github.com/nginxinc/kubernetes-ingress/internal/nginx" conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1" api_v1 "k8s.io/api/core/v1" @@ -59,7 +60,7 @@ type VirtualServerEx struct { ExternalNameSvcs map[string]bool Policies map[string]*conf_v1alpha1.Policy PodsByIP map[string]PodInfo - SecretRefs map[string]*SecretReference + SecretRefs map[string]*secrets.SecretReference } func (vsx *VirtualServerEx) String() string { @@ -612,7 +613,7 @@ type policyOwnerDetails struct { type policyOptions struct { tls bool - secretRefs map[string]*SecretReference + secretRefs map[string]*secrets.SecretReference } type validationResults struct { @@ -673,7 +674,7 @@ func (p *policiesCfg) addJWTAuthConfig( jwtAuth *conf_v1alpha1.JWTAuth, polKey string, polNamespace string, - secretRefs map[string]*SecretReference, + secretRefs map[string]*secrets.SecretReference, ) *validationResults { res := newValidationResults() if p.JWTAuth != nil { @@ -687,7 +688,7 @@ func (p *policiesCfg) addJWTAuthConfig( res.addWarningf("JWT policy %q references an invalid Secret: %v", polKey, secret.Error) res.isError = true return res - } else if secret.Type != SecretTypeJWK { + } else if secret.Type != secrets.SecretTypeJWK { res.addWarningf("JWT policy %q references a Secret of an incorrect type %q", polKey, secret.Type) res.isError = true return res @@ -707,7 +708,7 @@ func (p *policiesCfg) addIngressMTLSConfig( polNamespace string, context string, tls bool, - secretRefs map[string]*SecretReference, + secretRefs map[string]*secrets.SecretReference, ) *validationResults { res := newValidationResults() if !tls { @@ -731,7 +732,7 @@ func (p *policiesCfg) addIngressMTLSConfig( res.addWarningf("IngressMTLS policy %q references an invalid Secret: %v", polKey, secret.Error) res.isError = true return res - } else if secret.Type != SecretTypeCA { + } else if secret.Type != secrets.SecretTypeCA { res.addWarningf("IngressMTLS policy %q references a Secret of an incorrect type %q", polKey, secret.Type) res.isError = true return res @@ -758,7 +759,7 @@ func (p *policiesCfg) addEgressMTLSConfig( egressMTLS *conf_v1alpha1.EgressMTLS, polKey string, polNamespace string, - secretRefs map[string]*SecretReference, + secretRefs map[string]*secrets.SecretReference, ) *validationResults { res := newValidationResults() if p.EgressMTLS != nil { @@ -798,7 +799,7 @@ func (p *policiesCfg) addEgressMTLSConfig( res.addWarningf("EgressMTLS policy %q references an invalid Secret: %v", polKey, trustedSecret.Error) res.isError = true return res - } else if trustedSecret.Type != SecretTypeCA { + } else if trustedSecret.Type != secrets.SecretTypeCA { res.addWarningf("EgressMTLS policy %q references a Secret of an incorrect type %q", polKey, trustedSecret.Type) res.isError = true return res @@ -1780,7 +1781,7 @@ func getNameForSourceForMatchesRouteMapFromCondition(condition conf_v1.Condition return condition.Variable } -func generateSSLConfig(tls *conf_v1.TLS, namespace string, secretRefs map[string]*SecretReference, cfgParams *ConfigParams) *version2.SSL { +func generateSSLConfig(tls *conf_v1.TLS, namespace string, secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams) *version2.SSL { if tls == nil { return nil } diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 3e98aa017d..22397a604e 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -8,6 +8,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" "github.com/nginxinc/kubernetes-ingress/internal/nginx" conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1" @@ -1810,7 +1811,7 @@ func TestGeneratePolicies(t *testing.T) { ingressMTLSCertPath := "/etc/nginx/secrets/default-ingress-mtls-secret" policyOpts := policyOptions{ tls: true, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/ingress-mtls-secret": { Type: "nginx.org/ca", Path: ingressMTLSCertPath, @@ -1824,7 +1825,7 @@ func TestGeneratePolicies(t *testing.T) { Path: "/etc/nginx/secrets/default-egress-trusted-ca-secret", }, "default/jwt-secret": { - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, Path: "/etc/nginx/secrets/default-jwt-secret", }, }, @@ -2294,9 +2295,9 @@ func TestGeneratePoliciesFails(t *testing.T) { }, }, policyOpts: policyOptions{ - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/jwt-secret": { - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, Error: errors.New("secret is invalid"), }, }, @@ -2351,13 +2352,13 @@ func TestGeneratePoliciesFails(t *testing.T) { }, }, policyOpts: policyOptions{ - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/jwt-secret": { - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, Path: "/etc/nginx/secrets/default-jwt-secret", }, "default/jwt-secret2": { - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, Path: "/etc/nginx/secrets/default-jwt-secret2", }, }, @@ -2397,7 +2398,7 @@ func TestGeneratePoliciesFails(t *testing.T) { }, policyOpts: policyOptions{ tls: true, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/ingress-mtls-secret": { Error: errors.New("secret is invalid"), }, @@ -2449,9 +2450,9 @@ func TestGeneratePoliciesFails(t *testing.T) { }, policyOpts: policyOptions{ tls: true, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/ingress-mtls-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Path: "/etc/nginx/secrets/default-ingress-mtls-secret", }, }, @@ -2493,9 +2494,9 @@ func TestGeneratePoliciesFails(t *testing.T) { }, policyOpts: policyOptions{ tls: true, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/ingress-mtls-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Path: "/etc/nginx/secrets/default-ingress-mtls-secret", }, }, @@ -2535,9 +2536,9 @@ func TestGeneratePoliciesFails(t *testing.T) { }, policyOpts: policyOptions{ tls: false, - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/ingress-mtls-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Path: "/etc/nginx/secrets/default-ingress-mtls-secret", }, }, @@ -2591,7 +2592,7 @@ func TestGeneratePoliciesFails(t *testing.T) { }, }, policyOpts: policyOptions{ - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/egress-mtls-secret": { Type: api_v1.SecretTypeTLS, Path: "/etc/nginx/secrets/default-egress-mtls-secret", @@ -2640,9 +2641,9 @@ func TestGeneratePoliciesFails(t *testing.T) { }, }, policyOpts: policyOptions{ - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/egress-trusted-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Error: errors.New("secret is invalid"), }, }, @@ -2682,7 +2683,7 @@ func TestGeneratePoliciesFails(t *testing.T) { }, }, policyOpts: policyOptions{ - secretRefs: map[string]*SecretReference{ + secretRefs: map[string]*secrets.SecretReference{ "default/egress-mtls-secret": { Type: api_v1.SecretTypeTLS, Error: errors.New("secret is invalid"), @@ -3326,14 +3327,14 @@ func TestGenerateLocationForRedirect(t *testing.T) { func TestGenerateSSLConfig(t *testing.T) { tests := []struct { inputTLS *conf_v1.TLS - inputSecretRefs map[string]*SecretReference + inputSecretRefs map[string]*secrets.SecretReference inputCfgParams *ConfigParams expected *version2.SSL msg string }{ { inputTLS: nil, - inputSecretRefs: map[string]*SecretReference{}, + inputSecretRefs: map[string]*secrets.SecretReference{}, inputCfgParams: &ConfigParams{}, expected: nil, msg: "no TLS field", @@ -3342,7 +3343,7 @@ func TestGenerateSSLConfig(t *testing.T) { inputTLS: &conf_v1.TLS{ Secret: "", }, - inputSecretRefs: map[string]*SecretReference{}, + inputSecretRefs: map[string]*secrets.SecretReference{}, inputCfgParams: &ConfigParams{}, expected: nil, msg: "TLS field with empty secret", @@ -3352,7 +3353,7 @@ func TestGenerateSSLConfig(t *testing.T) { Secret: "secret", }, inputCfgParams: &ConfigParams{}, - inputSecretRefs: map[string]*SecretReference{ + inputSecretRefs: map[string]*secrets.SecretReference{ "default/secret": { Error: errors.New("secret doesn't exist"), }, @@ -3369,7 +3370,7 @@ func TestGenerateSSLConfig(t *testing.T) { inputTLS: &conf_v1.TLS{ Secret: "secret", }, - inputSecretRefs: map[string]*SecretReference{ + inputSecretRefs: map[string]*secrets.SecretReference{ "default/secret": { Type: api_v1.SecretTypeTLS, Path: "secret.pem", diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 4c035f8e69..ad554f19fd 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -24,6 +24,7 @@ import ( "time" "github.com/golang/glog" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" "github.com/spiffe/go-spiffe/workload" "k8s.io/apimachinery/pkg/fields" @@ -154,7 +155,7 @@ type LoadBalancerController struct { isNginxReady bool isLatencyMetricsEnabled bool configuration *Configuration - secretStore SecretStore + secretStore secrets.SecretStore } var keyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc @@ -290,7 +291,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc lbc.configuration = NewConfiguration(lbc.HasCorrectIngressClass, input.IsNginxPlus, input.VirtualServerValidator) - lbc.secretStore = NewLocalSecretStore(lbc.configurator) + lbc.secretStore = secrets.NewLocalSecretStore(lbc.configurator) lbc.updateIngressMetrics() return lbc @@ -1496,7 +1497,7 @@ func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, res func (lbc *LoadBalancerController) handleSpecialSecretUpdate(secret *api_v1.Secret) { var specialSecretsToUpdate []string secretNsName := secret.Namespace + "/" + secret.Name - err := ValidateTLSSecret(secret) + err := secrets.ValidateTLSSecret(secret) if err != nil { glog.Errorf("Couldn't validate the special Secret %v: %v", secretNsName, err) lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "Rejected", "the special Secret %v was rejected, using the previous version: %v", secretNsName, err) @@ -1728,22 +1729,18 @@ func (lbc *LoadBalancerController) createIngressEx(ing *networking.Ingress, vali ValidMinionPaths: validMinionPaths, } - ingEx.SecretRefs = make(map[string]*configs.SecretReference) + ingEx.SecretRefs = make(map[string]*secrets.SecretReference) for _, tls := range ing.Spec.TLS { secretName := tls.SecretName secretKey := ing.Namespace + "/" + secretName - secretType, secretPath, err := lbc.secretStore.GetSecret(secretKey) - if err != nil { - glog.Warningf("Error trying to get the secret %v for Ingress %v: %v", secretName, ing.Name, err) + secretRef := lbc.secretStore.GetSecretReference(secretKey) + if secretRef.Error != nil { + glog.Warningf("Error trying to get the secret %v for Ingress %v: %v", secretName, ing.Name, secretRef.Error) } - ingEx.SecretRefs[secretName] = &configs.SecretReference{ - Type: secretType, - Path: secretPath, - Error: err, - } + ingEx.SecretRefs[secretName] = secretRef } if lbc.isNginxPlus { @@ -1751,16 +1748,12 @@ func (lbc *LoadBalancerController) createIngressEx(ing *networking.Ingress, vali secretName := jwtKey secretKey := ing.Namespace + "/" + secretName - secretType, secretPath, err := lbc.secretStore.GetSecret(secretKey) - if err != nil { - glog.Warningf("Error trying to get the secret %v for Ingress %v/%v: %v", secretName, ing.Namespace, ing.Name, err) + secretRef := lbc.secretStore.GetSecretReference(secretKey) + if secretRef.Error != nil { + glog.Warningf("Error trying to get the secret %v for Ingress %v/%v: %v", secretName, ing.Namespace, ing.Name, secretRef.Error) } - ingEx.SecretRefs[secretName] = &configs.SecretReference{ - Type: secretType, - Path: secretPath, - Error: err, - } + ingEx.SecretRefs[secretName] = secretRef } if lbc.appProtectEnabled { if apPolicyAntn, exists := ingEx.Ingress.Annotations[configs.AppProtectPolicyAnnotation]; exists { @@ -1944,22 +1937,18 @@ func (lbc *LoadBalancerController) getAppProtectPolicy(ing *networking.Ingress) func (lbc *LoadBalancerController) createVirtualServerEx(virtualServer *conf_v1.VirtualServer, virtualServerRoutes []*conf_v1.VirtualServerRoute) *configs.VirtualServerEx { virtualServerEx := configs.VirtualServerEx{ VirtualServer: virtualServer, - SecretRefs: make(map[string]*configs.SecretReference), + SecretRefs: make(map[string]*secrets.SecretReference), } if virtualServer.Spec.TLS != nil && virtualServer.Spec.TLS.Secret != "" { secretKey := virtualServer.Namespace + "/" + virtualServer.Spec.TLS.Secret - secretType, secretPath, err := lbc.secretStore.GetSecret(secretKey) - if err != nil { - glog.Warningf("Error trying to get the secret %v for VirtualServer %v: %v", secretKey, virtualServer.Name, err) + secretRef := lbc.secretStore.GetSecretReference(secretKey) + if secretRef.Error != nil { + glog.Warningf("Error trying to get the secret %v for VirtualServer %v: %v", secretKey, virtualServer.Name, secretRef.Error) } - virtualServerEx.SecretRefs[secretKey] = &configs.SecretReference{ - Type: secretType, - Path: secretPath, - Error: err, - } + virtualServerEx.SecretRefs[secretKey] = secretRef } policies, policyErrors := lbc.getPolicies(virtualServer.Spec.Policies, virtualServer.Namespace) @@ -2164,83 +2153,65 @@ func (lbc *LoadBalancerController) getPolicies(policies []conf_v1.PolicyReferenc return result, errors } -func (lbc *LoadBalancerController) addJWTSecretRefs(secretRefs map[string]*configs.SecretReference, policies []*conf_v1alpha1.Policy) error { +func (lbc *LoadBalancerController) addJWTSecretRefs(secretRefs map[string]*secrets.SecretReference, policies []*conf_v1alpha1.Policy) error { for _, pol := range policies { if pol.Spec.JWTAuth == nil { continue } secretKey := fmt.Sprintf("%v/%v", pol.Namespace, pol.Spec.JWTAuth.Secret) - secretType, secretPath, err := lbc.secretStore.GetSecret(secretKey) + secretRef := lbc.secretStore.GetSecretReference(secretKey) - secretRefs[secretKey] = &configs.SecretReference{ - Type: secretType, - Path: secretPath, - Error: err, - } + secretRefs[secretKey] = secretRef - if err != nil { - return err + if secretRef.Error != nil { + return secretRef.Error } } return nil } -func (lbc *LoadBalancerController) addIngressMTLSSecretRefs(secretRefs map[string]*configs.SecretReference, policies []*conf_v1alpha1.Policy) error { +func (lbc *LoadBalancerController) addIngressMTLSSecretRefs(secretRefs map[string]*secrets.SecretReference, policies []*conf_v1alpha1.Policy) error { for _, pol := range policies { if pol.Spec.IngressMTLS == nil { continue } secretKey := fmt.Sprintf("%v/%v", pol.Namespace, pol.Spec.IngressMTLS.ClientCertSecret) - secretType, secretPath, err := lbc.secretStore.GetSecret(secretKey) + secretRef := lbc.secretStore.GetSecretReference(secretKey) - secretRefs[secretKey] = &configs.SecretReference{ - Type: secretType, - Path: secretPath, - Error: err, - } + secretRefs[secretKey] = secretRef - return err + return secretRef.Error } return nil } -func (lbc *LoadBalancerController) addEgressMTLSSecretRefs(secretRefs map[string]*configs.SecretReference, policies []*conf_v1alpha1.Policy) error { +func (lbc *LoadBalancerController) addEgressMTLSSecretRefs(secretRefs map[string]*secrets.SecretReference, policies []*conf_v1alpha1.Policy) error { for _, pol := range policies { if pol.Spec.EgressMTLS == nil { continue } if pol.Spec.EgressMTLS.TLSSecret != "" { secretKey := fmt.Sprintf("%v/%v", pol.Namespace, pol.Spec.EgressMTLS.TLSSecret) + secretRef := lbc.secretStore.GetSecretReference(secretKey) - secretType, secretPath, err := lbc.secretStore.GetSecret(secretKey) - - secretRefs[secretKey] = &configs.SecretReference{ - Type: secretType, - Path: secretPath, - Error: err, - } + secretRefs[secretKey] = secretRef - if err != nil { - return err + if secretRef.Error != nil { + return secretRef.Error } } if pol.Spec.EgressMTLS.TrustedCertSecret != "" { secretKey := fmt.Sprintf("%v/%v", pol.Namespace, pol.Spec.EgressMTLS.TrustedCertSecret) + secretRef := lbc.secretStore.GetSecretReference(secretKey) - secretType, secretPath, err := lbc.secretStore.GetSecret(secretKey) + secretRefs[secretKey] = secretRef - secretRefs[secretKey] = &configs.SecretReference{ - Type: secretType, - Path: secretPath, - Error: err, - } - - if err != nil { - return err + if secretRef.Error != nil { + return secretRef.Error } } } diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index fec4613f02..46941be1df 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -12,6 +12,7 @@ import ( "github.com/nginxinc/kubernetes-ingress/internal/configs" "github.com/nginxinc/kubernetes-ingress/internal/configs/version1" "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" "github.com/nginxinc/kubernetes-ingress/internal/metrics/collectors" "github.com/nginxinc/kubernetes-ingress/internal/nginx" conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" @@ -1229,7 +1230,7 @@ func TestAddJWTSecrets(t *testing.T) { tests := []struct { policies []*conf_v1alpha1.Policy - expectedSecretRefs map[string]*configs.SecretReference + expectedSecretRefs map[string]*secrets.SecretReference wantErr bool msg string }{ @@ -1248,9 +1249,9 @@ func TestAddJWTSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/valid-jwk-secret": { - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, Path: "/etc/nginx/secrets/default-valid-jwk-secret", }, }, @@ -1259,7 +1260,7 @@ func TestAddJWTSecrets(t *testing.T) { }, { policies: []*conf_v1alpha1.Policy{}, - expectedSecretRefs: map[string]*configs.SecretReference{}, + expectedSecretRefs: map[string]*secrets.SecretReference{}, wantErr: false, msg: "test getting valid secret with no policy", }, @@ -1277,7 +1278,7 @@ func TestAddJWTSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{}, + expectedSecretRefs: map[string]*secrets.SecretReference{}, wantErr: false, msg: "test getting invalid secret with wrong policy", }, @@ -1296,9 +1297,9 @@ func TestAddJWTSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/invalid-jwk-secret": { - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, Error: invalidErr, }, }, @@ -1308,14 +1309,14 @@ func TestAddJWTSecrets(t *testing.T) { } lbc := LoadBalancerController{ - secretStore: newFakeSecretsStore(map[string]*StoredSecret{ + secretStore: secrets.NewFakeSecretsStore(map[string]*secrets.StoredSecret{ "default/valid-jwk-secret": { Secret: &v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "valid-jwk-secret", Namespace: "default", }, - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, }, Path: "/etc/nginx/secrets/default-valid-jwk-secret", }, @@ -1325,7 +1326,7 @@ func TestAddJWTSecrets(t *testing.T) { Name: "invalid-jwk-secret", Namespace: "default", }, - Type: SecretTypeJWK, + Type: secrets.SecretTypeJWK, }, ValidationErr: invalidErr, }, @@ -1333,7 +1334,7 @@ func TestAddJWTSecrets(t *testing.T) { } for _, test := range tests { - result := make(map[string]*configs.SecretReference) + result := make(map[string]*secrets.SecretReference) err := lbc.addJWTSecretRefs(result, test.policies) if (err != nil) != test.wantErr { @@ -1351,7 +1352,7 @@ func TestAddIngressMTLSSecret(t *testing.T) { tests := []struct { policies []*conf_v1alpha1.Policy - expectedSecretRefs map[string]*configs.SecretReference + expectedSecretRefs map[string]*secrets.SecretReference wantErr bool msg string }{ @@ -1369,9 +1370,9 @@ func TestAddIngressMTLSSecret(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/valid-ingress-mtls-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Path: "/etc/nginx/secrets/default-valid-ingress-mtls-secret", }, }, @@ -1380,7 +1381,7 @@ func TestAddIngressMTLSSecret(t *testing.T) { }, { policies: []*conf_v1alpha1.Policy{}, - expectedSecretRefs: map[string]*configs.SecretReference{}, + expectedSecretRefs: map[string]*secrets.SecretReference{}, wantErr: false, msg: "test getting valid secret with no policy", }, @@ -1398,7 +1399,7 @@ func TestAddIngressMTLSSecret(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{}, + expectedSecretRefs: map[string]*secrets.SecretReference{}, wantErr: false, msg: "test getting valid secret with wrong policy", }, @@ -1416,9 +1417,9 @@ func TestAddIngressMTLSSecret(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/invalid-ingress-mtls-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Error: invalidErr, }, }, @@ -1428,14 +1429,14 @@ func TestAddIngressMTLSSecret(t *testing.T) { } lbc := LoadBalancerController{ - secretStore: newFakeSecretsStore(map[string]*StoredSecret{ + secretStore: secrets.NewFakeSecretsStore(map[string]*secrets.StoredSecret{ "default/valid-ingress-mtls-secret": { Secret: &v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "valid-ingress-mtls-secret", Namespace: "default", }, - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, }, Path: "/etc/nginx/secrets/default-valid-ingress-mtls-secret", }, @@ -1445,7 +1446,7 @@ func TestAddIngressMTLSSecret(t *testing.T) { Name: "invalid-ingress-mtls-secret", Namespace: "default", }, - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, }, ValidationErr: invalidErr, }, @@ -1453,7 +1454,7 @@ func TestAddIngressMTLSSecret(t *testing.T) { } for _, test := range tests { - result := make(map[string]*configs.SecretReference) + result := make(map[string]*secrets.SecretReference) err := lbc.addIngressMTLSSecretRefs(result, test.policies) if (err != nil) != test.wantErr { @@ -1471,7 +1472,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { tests := []struct { policies []*conf_v1alpha1.Policy - expectedSecretRefs map[string]*configs.SecretReference + expectedSecretRefs map[string]*secrets.SecretReference wantErr bool msg string }{ @@ -1489,7 +1490,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/valid-egress-mtls-secret": { Type: api_v1.SecretTypeTLS, Path: "/etc/nginx/secrets/default-valid-egress-mtls-secret", @@ -1512,9 +1513,9 @@ func TestAddEgressMTLSSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/valid-egress-trusted-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Path: "/etc/nginx/secrets/default-valid-egress-trusted-secret", }, }, @@ -1536,13 +1537,13 @@ func TestAddEgressMTLSSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/valid-egress-mtls-secret": { Type: api_v1.SecretTypeTLS, Path: "/etc/nginx/secrets/default-valid-egress-mtls-secret", }, "default/valid-egress-trusted-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Path: "/etc/nginx/secrets/default-valid-egress-trusted-secret", }, }, @@ -1551,7 +1552,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { }, { policies: []*conf_v1alpha1.Policy{}, - expectedSecretRefs: map[string]*configs.SecretReference{}, + expectedSecretRefs: map[string]*secrets.SecretReference{}, wantErr: false, msg: "test getting valid secret with no policy", }, @@ -1569,7 +1570,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{}, + expectedSecretRefs: map[string]*secrets.SecretReference{}, wantErr: false, msg: "test getting valid secret with wrong policy", }, @@ -1587,7 +1588,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/invalid-egress-mtls-secret": { Type: api_v1.SecretTypeTLS, Error: invalidErr, @@ -1610,9 +1611,9 @@ func TestAddEgressMTLSSecrets(t *testing.T) { }, }, }, - expectedSecretRefs: map[string]*configs.SecretReference{ + expectedSecretRefs: map[string]*secrets.SecretReference{ "default/invalid-egress-trusted-secret": { - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, Error: invalidErr, }, }, @@ -1622,7 +1623,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { } lbc := LoadBalancerController{ - secretStore: newFakeSecretsStore(map[string]*StoredSecret{ + secretStore: secrets.NewFakeSecretsStore(map[string]*secrets.StoredSecret{ "default/valid-egress-mtls-secret": { Secret: &v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ @@ -1639,7 +1640,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { Name: "valid-egress-trusted-secret", Namespace: "default", }, - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, }, Path: "/etc/nginx/secrets/default-valid-egress-trusted-secret", }, @@ -1659,7 +1660,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { Name: "invalid-egress-trusted-secret", Namespace: "default", }, - Type: SecretTypeCA, + Type: secrets.SecretTypeCA, }, ValidationErr: invalidErr, }, @@ -1667,7 +1668,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { } for _, test := range tests { - result := make(map[string]*configs.SecretReference) + result := make(map[string]*secrets.SecretReference) err := lbc.addEgressMTLSSecretRefs(result, test.policies) if (err != nil) != test.wantErr { diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index 609062d4eb..cff5f86b8a 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -5,6 +5,7 @@ import ( "sort" "github.com/golang/glog" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" v1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" "k8s.io/client-go/tools/cache" @@ -133,7 +134,7 @@ func createSecretHandlers(lbc *LoadBalancerController) cache.ResourceEventHandle return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { secret := obj.(*v1.Secret) - if !IsSupportedSecretType(secret.Type) { + if !secrets.IsSupportedSecretType(secret.Type) { glog.V(3).Infof("Ignoring Secret %v of unsupported type %v", secret.Name, secret.Type) return } @@ -154,7 +155,7 @@ func createSecretHandlers(lbc *LoadBalancerController) cache.ResourceEventHandle return } } - if !IsSupportedSecretType(secret.Type) { + if !secrets.IsSupportedSecretType(secret.Type) { glog.V(3).Infof("Ignoring Secret %v of unsupported type %v", secret.Name, secret.Type) return } @@ -165,7 +166,7 @@ func createSecretHandlers(lbc *LoadBalancerController) cache.ResourceEventHandle UpdateFunc: func(old, cur interface{}) { // A secret cannot change its type. That's why we only need to check the type of the current secret. curSecret := cur.(*v1.Secret) - if !IsSupportedSecretType(curSecret.Type) { + if !secrets.IsSupportedSecretType(curSecret.Type) { glog.V(3).Infof("Ignoring Secret %v of unsupported type %v", curSecret.Name, curSecret.Type) return } diff --git a/internal/k8s/secret_store.go b/internal/k8s/secrets/store.go similarity index 58% rename from internal/k8s/secret_store.go rename to internal/k8s/secrets/store.go index 078fb73b4d..13cadb085e 100644 --- a/internal/k8s/secret_store.go +++ b/internal/k8s/secrets/store.go @@ -1,9 +1,10 @@ -package k8s +package secrets import ( "fmt" api_v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // StoredSecret holds a secret, its validation status and the path on the file system. @@ -13,6 +14,13 @@ type StoredSecret struct { ValidationErr error } +// SecretReference holds a reference to a secret stored on the file system. +type SecretReference struct { + Type api_v1.SecretType + Path string + Error error +} + // SecretFileManager manages secrets on the file system. type SecretFileManager interface { AddOrUpdateSecret(secret *api_v1.Secret) string @@ -23,7 +31,7 @@ type SecretFileManager interface { type SecretStore interface { AddOrUpdateSecret(secret *api_v1.Secret) DeleteSecret(key string) - GetSecret(key string) (api_v1.SecretType, string, error) + GetSecretReference(key string) *SecretReference } // LocalSecretStore implements SecretStore interface. @@ -84,18 +92,64 @@ func (s *LocalSecretStore) DeleteSecret(key string) { s.manager.DeleteSecret(key) } -// GetSecret gets the secretType and the path of a requested secret via its namespace/name key. -// If the secret doesn't exist, is of an unsupported type, or invalid, GetSecret will return an error. +// GetSecretReference returns a SecretReference. +// If the secret doesn't exist, is of an unsupported type, or invalid, the Error field will include an error. // If the secret is valid but isn't present on the file system, the secret will be written to the file system. -func (s *LocalSecretStore) GetSecret(key string) (api_v1.SecretType, string, error) { +func (s *LocalSecretStore) GetSecretReference(key string) *SecretReference { storedSecret, exists := s.secrets[key] if !exists { - return "", "", fmt.Errorf("secret doesn't exist or of an unsupported type") + return &SecretReference{ + Error: fmt.Errorf("secret doesn't exist or of an unsupported type"), + } } if storedSecret.ValidationErr == nil && storedSecret.Path == "" { storedSecret.Path = s.manager.AddOrUpdateSecret(storedSecret.Secret) } - return storedSecret.Secret.Type, storedSecret.Path, storedSecret.ValidationErr + return &SecretReference{ + Type: storedSecret.Secret.Type, + Path: storedSecret.Path, + Error: storedSecret.ValidationErr, + } +} + +func getResourceKey(meta *metav1.ObjectMeta) string { + return fmt.Sprintf("%s/%s", meta.Namespace, meta.Name) +} + +// FakeSecretStore is a fake implementation of SecretStore. +type FakeSecretStore struct { + secrets map[string]*StoredSecret +} + +// NewFakeSecretsStore creates a new FakeSecretStore. +func NewFakeSecretsStore(secrets map[string]*StoredSecret) SecretStore { + return &FakeSecretStore{ + secrets: secrets, + } +} + +// AddOrUpdateSecret is a fake implementation of AddOrUpdateSecret. +func (s *FakeSecretStore) AddOrUpdateSecret(secret *api_v1.Secret) { +} + +// DeleteSecret is a fake implementation of DeleteSecret. +func (s *FakeSecretStore) DeleteSecret(key string) { +} + +// GetSecretReference is a fake implementation of GetSecretReference. +func (s *FakeSecretStore) GetSecretReference(key string) *SecretReference { + storedSecret, exists := s.secrets[key] + if !exists { + return &SecretReference{ + Error: fmt.Errorf("secret doesn't exist"), + } + } + + return &SecretReference{ + Type: storedSecret.Secret.Type, + Path: storedSecret.Path, + Error: storedSecret.ValidationErr, + } } diff --git a/internal/k8s/secret_store_test.go b/internal/k8s/secrets/store_test.go similarity index 55% rename from internal/k8s/secret_store_test.go rename to internal/k8s/secrets/store_test.go index e5a2051094..c867bf6236 100644 --- a/internal/k8s/secret_store_test.go +++ b/internal/k8s/secrets/store_test.go @@ -1,7 +1,7 @@ -package k8s +package secrets import ( - "fmt" + "errors" "testing" "github.com/google/go-cmp/cmp" @@ -53,6 +53,14 @@ var ( } ) +func errorComparer(e1, e2 error) bool { + if e1 == nil || e2 == nil { + return e1 == e2 + } + + return e1.Error() == e2.Error() +} + func TestAddOrUpdateSecret(t *testing.T) { manager := &fakeSecretFileManager{} @@ -70,26 +78,23 @@ func TestAddOrUpdateSecret(t *testing.T) { // Get the secret - expectedSecretType := api_v1.SecretTypeTLS - expectedSecretPath := "testpath" + expectedSecretRef := &SecretReference{ + Type: api_v1.SecretTypeTLS, + Path: "testpath", + Error: nil, + } expectedManager = &fakeSecretFileManager{ AddedOrUpdatedSecret: validSecret, } manager.Reset() - secretType, secretPath, err := store.GetSecret("default/tls-secret") + secretRef := store.GetSecretReference("default/tls-secret") - if secretType != expectedSecretType { - t.Errorf("GetSecret() returned %v but expected %v", secretType, expectedSecretType) - } - if secretPath != expectedSecretPath { - t.Errorf("GetSecret() returned %v but expected %v", secretPath, expectedSecretPath) - } - if err != nil { - t.Errorf("GetSecret() returned unexpected error %v", err) + if diff := cmp.Diff(expectedSecretRef, secretRef, cmp.Comparer(errorComparer)); diff != "" { + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } if diff := cmp.Diff(expectedManager, manager); diff != "" { - t.Errorf("GetSecret() returned unexpected result (-want +got):\n%s", diff) + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } // Make the secret invalid @@ -107,25 +112,21 @@ func TestAddOrUpdateSecret(t *testing.T) { // Get the secret - expectedSecretType = api_v1.SecretTypeTLS - expectedSecretPath = "" + expectedSecretRef = &SecretReference{ + Type: api_v1.SecretTypeTLS, + Path: "", + Error: errors.New("Failed to validate TLS cert and key: asn1: syntax error: sequence truncated"), + } expectedManager = &fakeSecretFileManager{} - expectedErrorMsg := "Failed to validate TLS cert and key: asn1: syntax error: sequence truncated" manager.Reset() - secretType, secretPath, err = store.GetSecret("default/tls-secret") + secretRef = store.GetSecretReference("default/tls-secret") - if secretType != expectedSecretType { - t.Errorf("GetSecret() returned %v but expected %v", secretType, expectedSecretType) - } - if secretPath != expectedSecretPath { - t.Errorf("GetSecret() returned %v but expected %v", secretPath, expectedSecretPath) - } - if err == nil || err.Error() != expectedErrorMsg { - t.Errorf("GetSecret() returned error %v but expected %s", err, expectedErrorMsg) + if diff := cmp.Diff(expectedSecretRef, secretRef, cmp.Comparer(errorComparer)); diff != "" { + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } if diff := cmp.Diff(expectedManager, manager); diff != "" { - t.Errorf("GetSecret() returned unexpected result (-want +got):\n%s", diff) + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } // Restore the valid secret @@ -141,26 +142,23 @@ func TestAddOrUpdateSecret(t *testing.T) { // Get the secret - expectedSecretType = api_v1.SecretTypeTLS - expectedSecretPath = "testpath" + expectedSecretRef = &SecretReference{ + Type: api_v1.SecretTypeTLS, + Path: "testpath", + Error: nil, + } expectedManager = &fakeSecretFileManager{ AddedOrUpdatedSecret: validSecret, } manager.Reset() - secretType, secretPath, err = store.GetSecret("default/tls-secret") + secretRef = store.GetSecretReference("default/tls-secret") - if secretType != expectedSecretType { - t.Errorf("GetSecret() returned %v but expected %v", secretType, expectedSecretType) - } - if secretPath != expectedSecretPath { - t.Errorf("GetSecret() returned %v but expected %v", secretPath, expectedSecretPath) - } - if err != nil { - t.Errorf("GetSecret() returned unexpected error %v", err) + if diff := cmp.Diff(expectedSecretRef, secretRef, cmp.Comparer(errorComparer)); diff != "" { + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } if diff := cmp.Diff(expectedManager, manager); diff != "" { - t.Errorf("GetSecret() returned unexpected result (-want +got):\n%s", diff) + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } // Update the secret @@ -179,24 +177,21 @@ func TestAddOrUpdateSecret(t *testing.T) { // Get the secret - expectedSecretType = api_v1.SecretTypeTLS - expectedSecretPath = "testpath" + expectedSecretRef = &SecretReference{ + Type: api_v1.SecretTypeTLS, + Path: "testpath", + Error: nil, + } expectedManager = &fakeSecretFileManager{} manager.Reset() - secretType, secretPath, err = store.GetSecret("default/tls-secret") + secretRef = store.GetSecretReference("default/tls-secret") - if secretType != expectedSecretType { - t.Errorf("GetSecret() returned %v but expected %v", secretType, expectedSecretType) - } - if secretPath != expectedSecretPath { - t.Errorf("GetSecret() returned %v but expected %v", secretPath, expectedSecretPath) - } - if err != nil { - t.Errorf("GetSecret() returned unexpected error %v", err) + if diff := cmp.Diff(expectedSecretRef, secretRef, cmp.Comparer(errorComparer)); diff != "" { + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } if diff := cmp.Diff(expectedManager, manager); diff != "" { - t.Errorf("GetSecret() returned unexpected result (-want +got):\n%s", diff) + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } } @@ -229,26 +224,23 @@ func TestDeleteSecretValidSecret(t *testing.T) { // Get the secret - expectedSecretType := api_v1.SecretTypeTLS - expectedSecretPath := "testpath" + expectedSecretRef := &SecretReference{ + Type: api_v1.SecretTypeTLS, + Path: "testpath", + Error: nil, + } expectedManager = &fakeSecretFileManager{ AddedOrUpdatedSecret: validSecret, } manager.Reset() - secretType, secretPath, err := store.GetSecret("default/tls-secret") + secretRef := store.GetSecretReference("default/tls-secret") - if secretType != expectedSecretType { - t.Errorf("GetSecret() returned %v but expected %v", secretType, expectedSecretType) - } - if secretPath != expectedSecretPath { - t.Errorf("GetSecret() returned %v but expected %v", secretPath, expectedSecretPath) - } - if err != nil { - t.Errorf("GetSecret() returned unexpected error %v", err) + if diff := cmp.Diff(expectedSecretRef, secretRef, cmp.Comparer(errorComparer)); diff != "" { + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } if diff := cmp.Diff(expectedManager, manager); diff != "" { - t.Errorf("GetSecret() returned unexpected result (-want +got):\n%s", diff) + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } // Delete the secret @@ -266,25 +258,21 @@ func TestDeleteSecretValidSecret(t *testing.T) { // Get the secret - expectedSecretType = "" - expectedSecretPath = "" + expectedSecretRef = &SecretReference{ + Type: "", + Path: "", + Error: errors.New("secret doesn't exist or of an unsupported type"), + } expectedManager = &fakeSecretFileManager{} - expectedErrorMsg := "secret doesn't exist or of an unsupported type" manager.Reset() - secretType, secretPath, err = store.GetSecret("default/tls-secret") + secretRef = store.GetSecretReference("default/tls-secret") - if secretType != expectedSecretType { - t.Errorf("GetSecret() returned %v but expected %v", secretType, expectedSecretType) - } - if secretPath != expectedSecretPath { - t.Errorf("GetSecret() returned %v but expected %v", secretPath, expectedSecretPath) - } - if err == nil || err.Error() != expectedErrorMsg { - t.Errorf("GetSecret() returned error %v but expected %s", err, expectedErrorMsg) + if diff := cmp.Diff(expectedSecretRef, secretRef, cmp.Comparer(errorComparer)); diff != "" { + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } if diff := cmp.Diff(expectedManager, manager); diff != "" { - t.Errorf("GetSecret() returned unexpected result (-want +got):\n%s", diff) + t.Errorf("GetSecretReference() returned unexpected result (-want +got):\n%s", diff) } } @@ -313,28 +301,3 @@ func TestDeleteSecretInvalidSecret(t *testing.T) { t.Errorf("DeleteSecret() returned unexpected result (-want +got):\n%s", diff) } } - -type fakeSecretStore struct { - secrets map[string]*StoredSecret -} - -func newFakeSecretsStore(secrets map[string]*StoredSecret) *fakeSecretStore { - return &fakeSecretStore{ - secrets: secrets, - } -} - -func (s *fakeSecretStore) AddOrUpdateSecret(secret *api_v1.Secret) { -} - -func (s *fakeSecretStore) DeleteSecret(key string) { -} - -func (s *fakeSecretStore) GetSecret(key string) (api_v1.SecretType, string, error) { - storedSecret, exists := s.secrets[key] - if !exists { - return "", "", fmt.Errorf("secret doesn't exist") - } - - return storedSecret.Secret.Type, storedSecret.Path, storedSecret.ValidationErr -} diff --git a/internal/k8s/secret.go b/internal/k8s/secrets/validation.go similarity index 74% rename from internal/k8s/secret.go rename to internal/k8s/secrets/validation.go index b58a07fcbd..fea22d6156 100644 --- a/internal/k8s/secret.go +++ b/internal/k8s/secrets/validation.go @@ -1,4 +1,4 @@ -package k8s +package secrets import ( "crypto/tls" @@ -6,7 +6,7 @@ import ( "encoding/pem" "fmt" - v1 "k8s.io/api/core/v1" + api_v1 "k8s.io/api/core/v1" ) // JWTKeyKey is the key of the data field of a Secret where the JWK must be stored. @@ -16,20 +16,20 @@ const JWTKeyKey = "jwk" const CAKey = "ca.crt" // SecretTypeCA contains a certificate authority for TLS certificate verification. -const SecretTypeCA v1.SecretType = "nginx.org/ca" +const SecretTypeCA api_v1.SecretType = "nginx.org/ca" // SecretTypeJWK contains a JWK (JSON Web Key) for validating JWTs (JSON Web Tokens). -const SecretTypeJWK v1.SecretType = "nginx.org/jwk" +const SecretTypeJWK api_v1.SecretType = "nginx.org/jwk" // ValidateTLSSecret validates the secret. If it is valid, the function returns nil. -func ValidateTLSSecret(secret *v1.Secret) error { - if secret.Type != v1.SecretTypeTLS { - return fmt.Errorf("TLS Secret must be of the type %v", v1.SecretTypeTLS) +func ValidateTLSSecret(secret *api_v1.Secret) error { + if secret.Type != api_v1.SecretTypeTLS { + return fmt.Errorf("TLS Secret must be of the type %v", api_v1.SecretTypeTLS) } - // Kubernetes ensures that 'tls.crt' and 'tls.key' are present for secrets of v1.SecretTypeTLS type + // Kubernetes ensures that 'tls.crt' and 'tls.key' are present for secrets of api_v1.SecretTypeTLS type - _, err := tls.X509KeyPair(secret.Data[v1.TLSCertKey], secret.Data[v1.TLSPrivateKeyKey]) + _, err := tls.X509KeyPair(secret.Data[api_v1.TLSCertKey], secret.Data[api_v1.TLSPrivateKeyKey]) if err != nil { return fmt.Errorf("Failed to validate TLS cert and key: %v", err) } @@ -38,7 +38,7 @@ func ValidateTLSSecret(secret *v1.Secret) error { } // ValidateJWKSecret validates the secret. If it is valid, the function returns nil. -func ValidateJWKSecret(secret *v1.Secret) error { +func ValidateJWKSecret(secret *api_v1.Secret) error { if secret.Type != SecretTypeJWK { return fmt.Errorf("JWK secret must be of the type %v", SecretTypeJWK) } @@ -54,7 +54,7 @@ func ValidateJWKSecret(secret *v1.Secret) error { } // ValidateCASecret validates the secret. If it is valid, the function returns nil. -func ValidateCASecret(secret *v1.Secret) error { +func ValidateCASecret(secret *api_v1.Secret) error { if secret.Type != SecretTypeCA { return fmt.Errorf("CA secret must be of the type %v", SecretTypeCA) } @@ -80,14 +80,14 @@ func ValidateCASecret(secret *v1.Secret) error { } // IsSupportedSecretType checks if the secret type is supported. -func IsSupportedSecretType(secretType v1.SecretType) bool { - return secretType == v1.SecretTypeTLS || secretType == SecretTypeCA || secretType == SecretTypeJWK +func IsSupportedSecretType(secretType api_v1.SecretType) bool { + return secretType == api_v1.SecretTypeTLS || secretType == SecretTypeCA || secretType == SecretTypeJWK } // ValidateSecret validates the secret. If it is valid, the function returns nil. -func ValidateSecret(secret *v1.Secret) error { +func ValidateSecret(secret *api_v1.Secret) error { switch secret.Type { - case v1.SecretTypeTLS: + case api_v1.SecretTypeTLS: return ValidateTLSSecret(secret) case SecretTypeJWK: return ValidateJWKSecret(secret) diff --git a/internal/k8s/secret_test.go b/internal/k8s/secrets/validation_test.go similarity index 99% rename from internal/k8s/secret_test.go rename to internal/k8s/secrets/validation_test.go index c3509ce3e4..3e4a3440a9 100644 --- a/internal/k8s/secret_test.go +++ b/internal/k8s/secrets/validation_test.go @@ -1,4 +1,4 @@ -package k8s +package secrets import ( "testing"