From 564c9ac19903e557ff4a678e5e09d006f5153da0 Mon Sep 17 00:00:00 2001
From: Harry <harrybagdi@gmail.com>
Date: Fri, 31 Jan 2020 10:56:12 -0800
Subject: [PATCH] fix(parser) handle same SNI across different Ingress rules

When the same SNI is associated with different Kubernetes Secrets.
When using Kong with a database, this results in the same SNI being
associated with a different certificate and the controller fails to
update the SNI in Kong.

This fix reduces the likelihood of this happening for some cases but
will not fix the problem for all cases.

See
https://github.com/hbagdi/deck/commit/890404c3012a620bf46b220705a54e338e5387b1
for the root cause of this issue.

Fix #510
From #523
---
 internal/ingress/controller/parser/parser.go  | 58 +++++++++++----
 .../ingress/controller/parser/parser_test.go  | 72 +++++++++++++++++++
 2 files changed, 116 insertions(+), 14 deletions(-)

diff --git a/internal/ingress/controller/parser/parser.go b/internal/ingress/controller/parser/parser.go
index 577473d671..ef30a02f80 100644
--- a/internal/ingress/controller/parser/parser.go
+++ b/internal/ingress/controller/parser/parser.go
@@ -378,9 +378,52 @@ func (p *Parser) fillConsumersAndCredentials(state *KongState) error {
 	return nil
 }
 
+func filterHosts(secretNameToSNIs map[string][]string, hosts []string) []string {
+	hostsToAdd := []string{}
+	seenHosts := map[string]bool{}
+	for _, hosts := range secretNameToSNIs {
+		for _, host := range hosts {
+			seenHosts[host] = true
+		}
+	}
+	for _, host := range hosts {
+		if !seenHosts[host] {
+			hostsToAdd = append(hostsToAdd, host)
+		}
+	}
+	return hostsToAdd
+}
+
+func processTLSSections(tlsSections []networking.IngressTLS,
+	namespace string, secretNameToSNIs map[string][]string) {
+	// TODO: optmize: collect all TLS sections and process at the same
+	// time to avoid regenerating the seen map; or use a seen map in the
+	// parser struct itself.
+	for _, tls := range tlsSections {
+		if len(tls.Hosts) == 0 {
+			continue
+		}
+		if tls.SecretName == "" {
+			continue
+		}
+		hosts := tls.Hosts
+		secretName := namespace + "/" + tls.SecretName
+		hosts = filterHosts(secretNameToSNIs, hosts)
+		if secretNameToSNIs[secretName] != nil {
+			hosts = append(hosts, secretNameToSNIs[secretName]...)
+		}
+		secretNameToSNIs[secretName] = hosts
+	}
+}
+
 func (p *Parser) parseIngressRules(
 	ingressList []*networking.Ingress) (*parsedIngressRules, error) {
 
+	sort.SliceStable(ingressList, func(i, j int) bool {
+		return ingressList[i].CreationTimestamp.Before(
+			&ingressList[j].CreationTimestamp)
+	})
+
 	// generate the following:
 	// Services and Routes
 	var allDefaultBackends []networking.Ingress
@@ -396,20 +439,7 @@ func (p *Parser) parseIngressRules(
 
 		}
 
-		for _, tls := range ingressSpec.TLS {
-			if len(tls.Hosts) == 0 {
-				continue
-			}
-			if tls.SecretName == "" {
-				continue
-			}
-			hosts := tls.Hosts
-			secretName := ingress.Namespace + "/" + tls.SecretName
-			if secretNameToSNIs[secretName] != nil {
-				hosts = append(hosts, secretNameToSNIs[secretName]...)
-			}
-			secretNameToSNIs[secretName] = hosts
-		}
+		processTLSSections(ingressSpec.TLS, ingress.Namespace, secretNameToSNIs)
 
 		for i, rule := range ingressSpec.Rules {
 			host := rule.Host
diff --git a/internal/ingress/controller/parser/parser_test.go b/internal/ingress/controller/parser/parser_test.go
index 99bc78353a..6ea457e7f1 100644
--- a/internal/ingress/controller/parser/parser_test.go
+++ b/internal/ingress/controller/parser/parser_test.go
@@ -2635,3 +2635,75 @@ func Test_getCombinations(t *testing.T) {
 		})
 	}
 }
+
+func Test_processTLSSections(t *testing.T) {
+	type args struct {
+		tlsSections []networking.IngressTLS
+		namespace   string
+	}
+	tests := []struct {
+		name string
+		args args
+		want map[string][]string
+	}{
+		{
+			args: args{
+				tlsSections: []networking.IngressTLS{
+					{
+						Hosts: []string{
+							"1.example.com",
+							"2.example.com",
+						},
+						SecretName: "sooper-secret",
+					},
+					{
+						Hosts: []string{
+							"3.example.com",
+							"4.example.com",
+						},
+						SecretName: "sooper-secret2",
+					},
+				},
+				namespace: "foo",
+			},
+			want: map[string][]string{
+				"foo/sooper-secret":  {"1.example.com", "2.example.com"},
+				"foo/sooper-secret2": {"3.example.com", "4.example.com"},
+			},
+		},
+		{
+			args: args{
+				tlsSections: []networking.IngressTLS{
+					{
+						Hosts: []string{
+							"1.example.com",
+						},
+						SecretName: "sooper-secret",
+					},
+					{
+						Hosts: []string{
+							"3.example.com",
+							"1.example.com",
+							"4.example.com",
+						},
+						SecretName: "sooper-secret2",
+					},
+				},
+				namespace: "foo",
+			},
+			want: map[string][]string{
+				"foo/sooper-secret":  {"1.example.com"},
+				"foo/sooper-secret2": {"3.example.com", "4.example.com"},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := map[string][]string{}
+			processTLSSections(tt.args.tlsSections, tt.args.namespace, got)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("processTLSSections() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}