diff --git a/CHANGELOG.md b/CHANGELOG.md index 47623798e0..a8d3abe7cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,9 @@ Adding a new version? You'll need three changes: - Warning events are recorded when a Gateway Listener has more than one CertificateRef specified or refers to a Secret that has no valid TLS key-pair. [#3147](https://github.com/Kong/kubernetes-ingress-controller/pull/3147) +- Warning events are recorded when an Ingress refers to a TLS secret that does not exist or + has no valid TLS key-pair. + [#3150](https://github.com/Kong/kubernetes-ingress-controller/pull/3150) ### Fixed diff --git a/internal/dataplane/parser/ingressrules.go b/internal/dataplane/parser/ingressrules.go index bcc1e677c9..fe6fe30304 100644 --- a/internal/dataplane/parser/ingressrules.go +++ b/internal/dataplane/parser/ingressrules.go @@ -9,6 +9,8 @@ import ( corev1 "k8s.io/api/core/v1" netv1 "k8s.io/api/networking/v1" netv1beta1 "k8s.io/api/networking/v1beta1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" @@ -32,7 +34,11 @@ func mergeIngressRules(objs ...ingressRules) ingressRules { for _, obj := range objs { for k, v := range obj.SecretNameToSNIs { - result.SecretNameToSNIs[k] = append(result.SecretNameToSNIs[k], v...) + if _, ok := result.SecretNameToSNIs[k]; !ok { + result.SecretNameToSNIs[k] = &SNIs{} + } + result.SecretNameToSNIs[k].hosts = append(result.SecretNameToSNIs[k].hosts, v.hosts...) + result.SecretNameToSNIs[k].parents = append(result.SecretNameToSNIs[k].parents, v.parents...) } for k, v := range obj.ServiceNameToServices { result.ServiceNameToServices[k] = v @@ -84,7 +90,10 @@ func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer, // ensure that the cert is loaded into Kong if _, ok := ir.SecretNameToSNIs[secretKey]; !ok { - ir.SecretNameToSNIs[secretKey] = []string{} + ir.SecretNameToSNIs[secretKey] = &SNIs{ + parents: []client.Object{k8sService}, + hosts: []string{}, + } } service.ClientCertificate = &kong.Certificate{ ID: kong.String(string(secret.UID)), @@ -99,22 +108,18 @@ func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer, return serviceNamesToSkip } -type SecretNameToSNIs map[string][]string +type SecretNameToSNIs map[string]*SNIs -func newSecretNameToSNIs() SecretNameToSNIs { - return SecretNameToSNIs(map[string][]string{}) +type SNIs struct { + parents []client.Object + hosts []string } -func (m SecretNameToSNIs) addFromIngressV1beta1TLS(tlsSections []netv1beta1.IngressTLS, namespace string) { - // Assume that v1beta1 and v1 tlsSections have identical semantics and field-wise content. - var v1 []netv1.IngressTLS - for _, item := range tlsSections { - v1 = append(v1, netv1.IngressTLS{Hosts: item.Hosts, SecretName: item.SecretName}) - } - m.addFromIngressV1TLS(v1, namespace) +func newSecretNameToSNIs() SecretNameToSNIs { + return map[string]*SNIs{} } -func (m SecretNameToSNIs) addFromIngressV1TLS(tlsSections []netv1.IngressTLS, namespace string) { +func (m SecretNameToSNIs) addFromIngressV1TLS(tlsSections []netv1.IngressTLS, parent client.Object) { for _, tls := range tlsSections { if len(tls.Hosts) == 0 { continue @@ -122,30 +127,54 @@ func (m SecretNameToSNIs) addFromIngressV1TLS(tlsSections []netv1.IngressTLS, na if tls.SecretName == "" { continue } - hosts := tls.Hosts - secretName := namespace + "/" + tls.SecretName - hosts = m.filterHosts(hosts) - if m[secretName] != nil { - hosts = append(hosts, m[secretName]...) - } - m[secretName] = hosts + + secretKey := parent.GetNamespace() + "/" + tls.SecretName + m.addUniqueHosts(secretKey, tls.Hosts) + m.addUniqueParents(secretKey, []client.Object{parent}) } } -func (m SecretNameToSNIs) filterHosts(hosts []string) []string { - hostsToAdd := []string{} +func (m SecretNameToSNIs) addUniqueHosts(secretKey string, hosts []string) { + if _, ok := m[secretKey]; !ok { + m[secretKey] = &SNIs{} + } + seenHosts := map[string]bool{} - for _, hosts := range m { - for _, host := range hosts { + for _, snis := range m { + for _, host := range snis.hosts { seenHosts[host] = true } } + + var hostsToAdd []string for _, host := range hosts { if !seenHosts[host] { hostsToAdd = append(hostsToAdd, host) } } - return hostsToAdd + + m[secretKey].hosts = append(m[secretKey].hosts, hostsToAdd...) +} + +// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/3166 +func (m SecretNameToSNIs) addUniqueParents(secretKey string, parents []client.Object) { + if _, ok := m[secretKey]; !ok { + m[secretKey] = &SNIs{} + } + + seenParents := map[types.UID]struct{}{} + for _, parent := range m[secretKey].parents { + seenParents[parent.GetUID()] = struct{}{} + } + + var parentsToAdd []client.Object + for _, parent := range parents { + if _, ok := seenParents[parent.GetUID()]; !ok { + parentsToAdd = append(parentsToAdd, parent) + } + } + + m[secretKey].parents = append(m[secretKey].parents, parentsToAdd...) } func getK8sServicesForBackends( @@ -233,3 +262,11 @@ func servicesAllUseTheSameKongAnnotations( return match } + +func v1beta1toV1TLS(tlsSections []netv1beta1.IngressTLS) []netv1.IngressTLS { + var v1 []netv1.IngressTLS + for _, item := range tlsSections { + v1 = append(v1, netv1.IngressTLS{Hosts: item.Hosts, SecretName: item.SecretName}) + } + return v1 +} diff --git a/internal/dataplane/parser/ingressrules_test.go b/internal/dataplane/parser/ingressrules_test.go index 42372ef16e..e0a3bd9644 100644 --- a/internal/dataplane/parser/ingressrules_test.go +++ b/internal/dataplane/parser/ingressrules_test.go @@ -10,15 +10,23 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + netv1 "k8s.io/api/networking/v1" netv1beta1 "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" "github.com/kong/kubernetes-ingress-controller/v2/internal/store" ) func TestMergeIngressRules(t *testing.T) { + var ( + parent1 = &netv1.Ingress{ObjectMeta: metav1.ObjectMeta{UID: uuid.NewUUID()}} + parent2 = &netv1.Ingress{ObjectMeta: metav1.ObjectMeta{UID: uuid.NewUUID()}} + ) + for _, tt := range []struct { name string inputs []ingressRules @@ -27,7 +35,7 @@ func TestMergeIngressRules(t *testing.T) { { name: "empty list", wantOutput: &ingressRules{ - SecretNameToSNIs: map[string][]string{}, + SecretNameToSNIs: map[string]*SNIs{}, ServiceNameToServices: map[string]kongstate.Service{}, }, }, @@ -37,7 +45,7 @@ func TestMergeIngressRules(t *testing.T) { {}, {}, {}, }, wantOutput: &ingressRules{ - SecretNameToSNIs: map[string][]string{}, + SecretNameToSNIs: map[string]*SNIs{}, ServiceNameToServices: map[string]kongstate.Service{}, }, }, @@ -45,12 +53,12 @@ func TestMergeIngressRules(t *testing.T) { name: "one input", inputs: []ingressRules{ { - SecretNameToSNIs: map[string][]string{"a": {"b", "c"}, "d": {"e", "f"}}, + SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}, "d": {hosts: []string{"e", "f"}}}, ServiceNameToServices: map[string]kongstate.Service{"1": {Namespace: "potato"}}, }, }, wantOutput: &ingressRules{ - SecretNameToSNIs: map[string][]string{"a": {"b", "c"}, "d": {"e", "f"}}, + SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}, "d": {hosts: []string{"e", "f"}}}, ServiceNameToServices: map[string]kongstate.Service{"1": {Namespace: "potato"}}, }, }, @@ -58,18 +66,18 @@ func TestMergeIngressRules(t *testing.T) { name: "three inputs", inputs: []ingressRules{ { - SecretNameToSNIs: map[string][]string{"a": {"b", "c"}, "d": {"e", "f"}}, + SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}, "d": {hosts: []string{"e", "f"}}}, ServiceNameToServices: map[string]kongstate.Service{"1": {Namespace: "potato"}}, }, { - SecretNameToSNIs: map[string][]string{"g": {"h"}}, + SecretNameToSNIs: map[string]*SNIs{"g": {hosts: []string{"h"}}}, }, { ServiceNameToServices: map[string]kongstate.Service{"2": {Namespace: "carrot"}}, }, }, wantOutput: &ingressRules{ - SecretNameToSNIs: map[string][]string{"a": {"b", "c"}, "d": {"e", "f"}, "g": {"h"}}, + SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}, "d": {hosts: []string{"e", "f"}}, "g": {hosts: []string{"h"}}}, ServiceNameToServices: map[string]kongstate.Service{"1": {Namespace: "potato"}, "2": {Namespace: "carrot"}}, }, }, @@ -77,14 +85,44 @@ func TestMergeIngressRules(t *testing.T) { name: "can merge SNI arrays", inputs: []ingressRules{ { - SecretNameToSNIs: map[string][]string{"a": {"b", "c"}}, + SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}}, + }, + { + SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"d", "e"}}}, + }, + }, + wantOutput: &ingressRules{ + SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c", "d", "e"}}}, + ServiceNameToServices: map[string]kongstate.Service{}, + }, + }, + { + name: "can merge SNI arrays with parents", + inputs: []ingressRules{ + { + SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1}, hosts: []string{"b", "c"}}}, }, { - SecretNameToSNIs: map[string][]string{"a": {"d", "e"}}, + SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent2}, hosts: []string{"d", "e"}}}, }, }, wantOutput: &ingressRules{ - SecretNameToSNIs: map[string][]string{"a": {"b", "c", "d", "e"}}, + SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1, parent2}, hosts: []string{"b", "c", "d", "e"}}}, + ServiceNameToServices: map[string]kongstate.Service{}, + }, + }, + { + name: "can merge SNI arrays with repeating parents", + inputs: []ingressRules{ + { + SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1, parent2}, hosts: []string{"b", "c"}}}, + }, + { + SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1, parent2}, hosts: []string{"d", "e"}}}, + }, + }, + wantOutput: &ingressRules{ + SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1, parent2, parent1, parent2}, hosts: []string{"b", "c", "d", "e"}}}, ServiceNameToServices: map[string]kongstate.Service{}, }, }, @@ -99,7 +137,7 @@ func TestMergeIngressRules(t *testing.T) { }, }, wantOutput: &ingressRules{ - SecretNameToSNIs: map[string][]string{}, + SecretNameToSNIs: newSecretNameToSNIs(), ServiceNameToServices: map[string]kongstate.Service{"svc-name": {Namespace: "new"}}, }, }, @@ -112,9 +150,10 @@ func TestMergeIngressRules(t *testing.T) { } func TestAddFromIngressV1beta1TLS(t *testing.T) { + parentIngress := &netv1beta1.Ingress{ObjectMeta: metav1.ObjectMeta{Namespace: "foo"}} + type args struct { tlsSections []netv1beta1.IngressTLS - namespace string } tests := []struct { name string @@ -139,11 +178,10 @@ func TestAddFromIngressV1beta1TLS(t *testing.T) { SecretName: "sooper-secret2", }, }, - namespace: "foo", }, want: SecretNameToSNIs{ - "foo/sooper-secret": {"1.example.com", "2.example.com"}, - "foo/sooper-secret2": {"3.example.com", "4.example.com"}, + "foo/sooper-secret": {hosts: []string{"1.example.com", "2.example.com"}, parents: []client.Object{parentIngress}}, + "foo/sooper-secret2": {hosts: []string{"3.example.com", "4.example.com"}, parents: []client.Object{parentIngress}}, }, }, { @@ -164,18 +202,17 @@ func TestAddFromIngressV1beta1TLS(t *testing.T) { SecretName: "sooper-secret2", }, }, - namespace: "foo", }, want: SecretNameToSNIs{ - "foo/sooper-secret": {"1.example.com"}, - "foo/sooper-secret2": {"3.example.com", "4.example.com"}, + "foo/sooper-secret": {hosts: []string{"1.example.com"}, parents: []client.Object{parentIngress}}, + "foo/sooper-secret2": {hosts: []string{"3.example.com", "4.example.com"}, parents: []client.Object{parentIngress}}, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { m := newSecretNameToSNIs() - m.addFromIngressV1beta1TLS(tt.args.tlsSections, tt.args.namespace) + m.addFromIngressV1TLS(v1beta1toV1TLS(tt.args.tlsSections), parentIngress) assert.Equal(t, m, tt.want) }) } diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index ac3419d8c5..928aeb68fc 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -11,7 +11,7 @@ import ( "github.com/kong/go-kong/kong" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" - netv1beta1 "k8s.io/api/networking/v1beta1" + netv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" knative "knative.dev/networking/pkg/apis/networking/v1alpha1" @@ -120,7 +120,7 @@ func (p *Parser) Build() (*kongstate.KongState, []TranslationFailure) { result.FillPlugins(p.logger, p.storer) // generate Certificates and SNIs - ingressCerts := getCerts(p.logger, p.storer, ingressRules.SecretNameToSNIs) + ingressCerts := p.getCerts(ingressRules.SecretNameToSNIs) gatewayCerts := p.getGatewayCerts() // note that ingress-derived certificates will take precedence over gateway-derived certificates for SNI assignment result.Certificates = mergeCerts(p.logger, ingressCerts, gatewayCerts) @@ -195,11 +195,11 @@ func (p *Parser) popTranslationFailures() []TranslationFailure { return p.failuresCollector.PopTranslationFailures() } -func knativeIngressToNetworkingTLS(tls []knative.IngressTLS) []netv1beta1.IngressTLS { - var result []netv1beta1.IngressTLS +func knativeIngressToNetworkingTLS(tls []knative.IngressTLS) []netv1.IngressTLS { + var result []netv1.IngressTLS for _, t := range tls { - result = append(result, netv1beta1.IngressTLS{ + result = append(result, netv1.IngressTLS{ Hosts: t.Hosts, SecretName: t.SecretName, }) @@ -207,11 +207,11 @@ func knativeIngressToNetworkingTLS(tls []knative.IngressTLS) []netv1beta1.Ingres return result } -func tcpIngressToNetworkingTLS(tls []configurationv1beta1.IngressTLS) []netv1beta1.IngressTLS { - var result []netv1beta1.IngressTLS +func tcpIngressToNetworkingTLS(tls []configurationv1beta1.IngressTLS) []netv1.IngressTLS { + var result []netv1.IngressTLS for _, t := range tls { - result = append(result, netv1beta1.IngressTLS{ + result = append(result, netv1.IngressTLS{ Hosts: t.Hosts, SecretName: t.SecretName, }) @@ -480,25 +480,20 @@ func (p *Parser) getGatewayCerts() []certWrapper { return certs } -func getCerts(log logrus.FieldLogger, s store.Storer, secretsToSNIs map[string][]string) []certWrapper { +func (p *Parser) getCerts(secretsToSNIs SecretNameToSNIs) []certWrapper { certs := []certWrapper{} for secretKey, SNIs := range secretsToSNIs { namespaceName := strings.Split(secretKey, "/") - secret, err := s.GetSecret(namespaceName[0], namespaceName[1]) + secret, err := p.storer.GetSecret(namespaceName[0], namespaceName[1]) if err != nil { - log.WithFields(logrus.Fields{ - "secret_name": namespaceName[1], - "secret_namespace": namespaceName[0], - }).WithError(err).Error("failed to fetch secret") + p.registerTranslationFailure(fmt.Sprintf("failed to fetch the secret (%s)", secretKey), SNIs.parents...) continue } cert, key, err := getCertFromSecret(secret) if err != nil { - log.WithFields(logrus.Fields{ - "secret_name": namespaceName[1], - "secret_namespace": namespaceName[0], - }).WithError(err).Error("failed to construct certificate from secret") + causingObjects := append(SNIs.parents, secret) + p.registerTranslationFailure("failed to construct certificate from secret", causingObjects...) continue } certs = append(certs, certWrapper{ @@ -509,7 +504,7 @@ func getCerts(log logrus.FieldLogger, s store.Storer, secretsToSNIs map[string][ Key: kong.String(key), }, CreationTimestamp: secret.CreationTimestamp, - snis: SNIs, + snis: SNIs.hosts, }) } diff --git a/internal/dataplane/parser/translate_ingress.go b/internal/dataplane/parser/translate_ingress.go index ff720ebbaa..766e844b9c 100644 --- a/internal/dataplane/parser/translate_ingress.go +++ b/internal/dataplane/parser/translate_ingress.go @@ -48,7 +48,7 @@ func (p *Parser) ingressRulesFromIngressV1beta1() ingressRules { allDefaultBackends = append(allDefaultBackends, *ingress) } - result.SecretNameToSNIs.addFromIngressV1beta1TLS(ingressSpec.TLS, ingress.Namespace) + result.SecretNameToSNIs.addFromIngressV1TLS(v1beta1toV1TLS(ingressSpec.TLS), ingress) var objectSuccessfullyParsed bool for i, rule := range ingressSpec.Rules { @@ -204,7 +204,7 @@ func (p *Parser) ingressRulesFromIngressV1() ingressRules { allDefaultBackends = append(allDefaultBackends, *ingress) } - result.SecretNameToSNIs.addFromIngressV1TLS(ingressSpec.TLS, ingress.Namespace) + result.SecretNameToSNIs.addFromIngressV1TLS(ingressSpec.TLS, ingress) var objectSuccessfullyParsed bool diff --git a/internal/dataplane/parser/translate_ingress_test.go b/internal/dataplane/parser/translate_ingress_test.go index eb4c5e3c7d..f315fdc359 100644 --- a/internal/dataplane/parser/translate_ingress_test.go +++ b/internal/dataplane/parser/translate_ingress_test.go @@ -274,7 +274,7 @@ func TestFromIngressV1beta1(t *testing.T) { parsedInfo := p.ingressRulesFromIngressV1beta1() assert.Equal(ingressRules{ ServiceNameToServices: make(map[string]kongstate.Service), - SecretNameToSNIs: make(map[string][]string), + SecretNameToSNIs: make(map[string]*SNIs), }, parsedInfo) }) t.Run("simple ingress rule is parsed", func(t *testing.T) { @@ -324,8 +324,8 @@ func TestFromIngressV1beta1(t *testing.T) { parsedInfo := p.ingressRulesFromIngressV1beta1() assert.Equal(2, len(parsedInfo.SecretNameToSNIs)) - assert.Equal(2, len(parsedInfo.SecretNameToSNIs["bar-namespace/sooper-secret"])) - assert.Equal(2, len(parsedInfo.SecretNameToSNIs["bar-namespace/sooper-secret2"])) + assert.Equal(2, len(parsedInfo.SecretNameToSNIs["bar-namespace/sooper-secret"].hosts)) + assert.Equal(2, len(parsedInfo.SecretNameToSNIs["bar-namespace/sooper-secret2"].hosts)) }) t.Run("ingress rule with ACME like path has strip_path set to false", func(t *testing.T) { store, err := store.NewFakeStore(store.FakeObjects{ @@ -748,7 +748,7 @@ func TestFromIngressV1(t *testing.T) { parsedInfo := p.ingressRulesFromIngressV1() assert.Equal(ingressRules{ ServiceNameToServices: make(map[string]kongstate.Service), - SecretNameToSNIs: make(map[string][]string), + SecretNameToSNIs: make(map[string]*SNIs), }, parsedInfo) }) t.Run("simple ingress rule is parsed", func(t *testing.T) { @@ -801,8 +801,8 @@ func TestFromIngressV1(t *testing.T) { parsedInfo := p.ingressRulesFromIngressV1() assert.Equal(2, len(parsedInfo.SecretNameToSNIs)) - assert.Equal(2, len(parsedInfo.SecretNameToSNIs["bar-namespace/sooper-secret"])) - assert.Equal(2, len(parsedInfo.SecretNameToSNIs["bar-namespace/sooper-secret2"])) + assert.Equal(2, len(parsedInfo.SecretNameToSNIs["bar-namespace/sooper-secret"].hosts)) + assert.Equal(2, len(parsedInfo.SecretNameToSNIs["bar-namespace/sooper-secret2"].hosts)) }) t.Run("ingress rule with ACME like path has strip_path set to false", func(t *testing.T) { store, err := store.NewFakeStore(store.FakeObjects{ diff --git a/internal/dataplane/parser/translate_knative.go b/internal/dataplane/parser/translate_knative.go index ecc59c4423..402880ab54 100644 --- a/internal/dataplane/parser/translate_knative.go +++ b/internal/dataplane/parser/translate_knative.go @@ -53,7 +53,7 @@ func (p *Parser) ingressRulesFromKnativeIngress() ingressRules { } ingressSpec := ingress.Spec - secretToSNIs.addFromIngressV1beta1TLS(knativeIngressToNetworkingTLS(ingress.Spec.TLS), ingress.Namespace) + secretToSNIs.addFromIngressV1TLS(knativeIngressToNetworkingTLS(ingress.Spec.TLS), ingress) var objectSuccessfullyParsed bool for i, rule := range ingressSpec.Rules { diff --git a/internal/dataplane/parser/translate_knative_test.go b/internal/dataplane/parser/translate_knative_test.go index c19ac60c87..2f39b81347 100644 --- a/internal/dataplane/parser/translate_knative_test.go +++ b/internal/dataplane/parser/translate_knative_test.go @@ -8,6 +8,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" knative "knative.dev/networking/pkg/apis/networking/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" @@ -293,9 +294,9 @@ func TestFromKnativeIngress(t *testing.T) { p := mustNewParser(t, store) parsedInfo := p.ingressRulesFromKnativeIngress() - assert.Equal(SecretNameToSNIs(map[string][]string{ - "foo-namespace/bar-secret": {"bar.example.com", "bar1.example.com"}, - "foo-namespace/foo-secret": {"foo.example.com", "foo1.example.com"}, + assert.Equal(SecretNameToSNIs(map[string]*SNIs{ + "foo-namespace/bar-secret": {hosts: []string{"bar.example.com", "bar1.example.com"}, parents: []client.Object{ingressList[3]}}, + "foo-namespace/foo-secret": {hosts: []string{"foo.example.com", "foo1.example.com"}, parents: []client.Object{ingressList[3]}}, }), parsedInfo.SecretNameToSNIs) }) t.Run("split knative Ingress resource chooses the highest split", func(t *testing.T) { diff --git a/internal/dataplane/parser/translate_kong_l4.go b/internal/dataplane/parser/translate_kong_l4.go index 2ffeeca3bb..205088d377 100644 --- a/internal/dataplane/parser/translate_kong_l4.go +++ b/internal/dataplane/parser/translate_kong_l4.go @@ -28,7 +28,7 @@ func (p *Parser) ingressRulesFromTCPIngressV1beta1() ingressRules { for _, ingress := range ingressList { ingressSpec := ingress.Spec - result.SecretNameToSNIs.addFromIngressV1beta1TLS(tcpIngressToNetworkingTLS(ingressSpec.TLS), ingress.Namespace) + result.SecretNameToSNIs.addFromIngressV1TLS(tcpIngressToNetworkingTLS(ingressSpec.TLS), ingress) var objectSuccessfullyParsed bool for i, rule := range ingressSpec.Rules { diff --git a/internal/dataplane/parser/translate_kong_l4_test.go b/internal/dataplane/parser/translate_kong_l4_test.go index 3c880c6289..bff687c9e5 100644 --- a/internal/dataplane/parser/translate_kong_l4_test.go +++ b/internal/dataplane/parser/translate_kong_l4_test.go @@ -108,7 +108,7 @@ func TestFromTCPIngressV1beta1(t *testing.T) { parsedInfo := p.ingressRulesFromTCPIngressV1beta1() assert.Equal(ingressRules{ ServiceNameToServices: make(map[string]kongstate.Service), - SecretNameToSNIs: make(map[string][]string), + SecretNameToSNIs: make(map[string]*SNIs), }, parsedInfo) }) t.Run("empty TCPIngress return empty info", func(t *testing.T) { @@ -123,7 +123,7 @@ func TestFromTCPIngressV1beta1(t *testing.T) { parsedInfo := p.ingressRulesFromTCPIngressV1beta1() assert.Equal(ingressRules{ ServiceNameToServices: make(map[string]kongstate.Service), - SecretNameToSNIs: make(map[string][]string), + SecretNameToSNIs: make(map[string]*SNIs), }, parsedInfo) }) t.Run("simple TCPIngress rule is parsed", func(t *testing.T) { @@ -194,7 +194,7 @@ func TestFromTCPIngressV1beta1(t *testing.T) { parsedInfo := p.ingressRulesFromTCPIngressV1beta1() assert.Equal(2, len(parsedInfo.SecretNameToSNIs)) - assert.Equal(2, len(parsedInfo.SecretNameToSNIs["default/sooper-secret"])) - assert.Equal(2, len(parsedInfo.SecretNameToSNIs["default/sooper-secret2"])) + assert.Equal(2, len(parsedInfo.SecretNameToSNIs["default/sooper-secret"].hosts)) + assert.Equal(2, len(parsedInfo.SecretNameToSNIs["default/sooper-secret2"].hosts)) }) } diff --git a/test/integration/translation_failures_test.go b/test/integration/translation_failures_test.go index 88b3d6b926..2b9b333d93 100644 --- a/test/integration/translation_failures_test.go +++ b/test/integration/translation_failures_test.go @@ -141,12 +141,13 @@ func TestTranslationFailures(t *testing.T) { service, err := env.Cluster().Client().CoreV1().Services(ns).Create(ctx, service, metav1.CreateOptions{}) require.NoError(t, err) - _, err = env.Cluster().Client().NetworkingV1().Ingresses(ns).Create( + ingress, err := env.Cluster().Client().NetworkingV1().Ingresses(ns).Create( ctx, ingressWithPathBackedByService(service), metav1.CreateOptions{}, ) require.NoError(t, err) + cleaner.Add(ingress) return expectedTranslationFailure{ causingObjects: []client.Object{service}, @@ -161,6 +162,7 @@ func TestTranslationFailures(t *testing.T) { ingress := ingressWithPathBackedByService(nonExistingService) ingress, err := env.Cluster().Client().NetworkingV1().Ingresses(ns).Create(ctx, ingress, metav1.CreateOptions{}) require.NoError(t, err) + cleaner.Add(ingress) return expectedTranslationFailure{ causingObjects: []client.Object{ingress}, @@ -173,6 +175,7 @@ func TestTranslationFailures(t *testing.T) { translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) expectedTranslationFailure { service, err := env.Cluster().Client().CoreV1().Services(ns).Create(ctx, validService(), metav1.CreateOptions{}) require.NoError(t, err) + cleaner.Add(service) ingress := ingressWithPathBackedByService(service) const notMatchingPort = 90 @@ -181,6 +184,7 @@ func TestTranslationFailures(t *testing.T) { } ingress, err = env.Cluster().Client().NetworkingV1().Ingresses(ns).Create(ctx, ingress, metav1.CreateOptions{}) require.NoError(t, err) + cleaner.Add(ingress) return expectedTranslationFailure{ causingObjects: []client.Object{ingress, service}, @@ -230,6 +234,59 @@ func TestTranslationFailures(t *testing.T) { } }, }, + { + name: "ingress referring a non-existing TLS secret", + translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) expectedTranslationFailure { + service, err := env.Cluster().Client().CoreV1().Services(ns).Create(ctx, validService(), metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(service) + + ingress := ingressWithPathBackedByService(service) + ingress.Spec.TLS = []netv1.IngressTLS{ + { + Hosts: []string{"example.com"}, + SecretName: "non-existing-secret", + }, + } + ingress, err = env.Cluster().Client().NetworkingV1().Ingresses(ns).Create(ctx, ingress, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(ingress) + + return expectedTranslationFailure{ + causingObjects: []client.Object{ingress}, + reasonContains: "failed to fetch the secret", + } + }, + }, + { + name: "ingress referring a secret with no valid TLS key-pair", + translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) expectedTranslationFailure { + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: testutils.RandomName(testTranslationFailuresObjectsPrefix)}} + secret, err := env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, secret, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(secret) + + service, err := env.Cluster().Client().CoreV1().Services(ns).Create(ctx, validService(), metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(service) + + ingress := ingressWithPathBackedByService(service) + ingress.Spec.TLS = []netv1.IngressTLS{ + { + Hosts: []string{"example.com"}, + SecretName: secret.Name, + }, + } + ingress, err = env.Cluster().Client().NetworkingV1().Ingresses(ns).Create(ctx, ingress, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(ingress) + + return expectedTranslationFailure{ + causingObjects: []client.Object{ingress, secret}, + reasonContains: "failed to construct certificate from secret", + } + }, + }, } for _, tt := range testCases {