diff --git a/rollout/trafficrouting/nginx/nginx.go b/rollout/trafficrouting/nginx/nginx.go index 2964cd5f39..3d56c55f6a 100644 --- a/rollout/trafficrouting/nginx/nginx.go +++ b/rollout/trafficrouting/nginx/nginx.go @@ -66,8 +66,6 @@ func (r *Reconciler) buildCanaryIngress(stableIngress *networkingv1.Ingress, nam canaryServiceName := r.cfg.Rollout.Spec.Strategy.Canary.CanaryService annotationPrefix := defaults.GetCanaryIngressAnnotationPrefixOrDefault(r.cfg.Rollout) - // Set up canary ingress resource, we do *not* have to duplicate `spec.tls` in a canary, only - // `spec.rules` desiredCanaryIngress := &networkingv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -78,6 +76,14 @@ func (r *Reconciler) buildCanaryIngress(stableIngress *networkingv1.Ingress, nam }, } + // Preserve TLS from stable ingress + if stableIngress.Spec.TLS != nil { + desiredCanaryIngress.Spec.TLS = make([]networkingv1.IngressTLS, len(stableIngress.Spec.TLS)) + for it := 0; it < len(stableIngress.Spec.TLS); it++ { + stableIngress.Spec.TLS[it].DeepCopyInto(&desiredCanaryIngress.Spec.TLS[it]) + } + } + // Preserve ingressClassName from stable ingress if stableIngress.Spec.IngressClassName != nil { desiredCanaryIngress.Spec.IngressClassName = stableIngress.Spec.IngressClassName @@ -136,8 +142,6 @@ func (r *Reconciler) buildLegacyCanaryIngress(stableIngress *extensionsv1beta1.I canaryServiceName := r.cfg.Rollout.Spec.Strategy.Canary.CanaryService annotationPrefix := defaults.GetCanaryIngressAnnotationPrefixOrDefault(r.cfg.Rollout) - // Set up canary ingress resource, we do *not* have to duplicate `spec.tls` in a canary, only - // `spec.rules` desiredCanaryIngress := &extensionsv1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -148,6 +152,14 @@ func (r *Reconciler) buildLegacyCanaryIngress(stableIngress *extensionsv1beta1.I }, } + // Preserve TLS from stable ingress + if stableIngress.Spec.TLS != nil { + desiredCanaryIngress.Spec.TLS = make([]extensionsv1beta1.IngressTLS, len(stableIngress.Spec.TLS)) + for it := 0; it < len(stableIngress.Spec.TLS); it++ { + stableIngress.Spec.TLS[it].DeepCopyInto(&desiredCanaryIngress.Spec.TLS[it]) + } + } + // Preserve ingressClassName from stable ingress if stableIngress.Spec.IngressClassName != nil { desiredCanaryIngress.Spec.IngressClassName = stableIngress.Spec.IngressClassName diff --git a/rollout/trafficrouting/nginx/nginx_test.go b/rollout/trafficrouting/nginx/nginx_test.go index d597b1187e..6930ce1e06 100644 --- a/rollout/trafficrouting/nginx/nginx_test.go +++ b/rollout/trafficrouting/nginx/nginx_test.go @@ -212,6 +212,48 @@ func checkBackendServiceLegacy(t *testing.T, ing *extensionsv1beta1.Ingress, ser assert.Fail(t, msg) } +func checkTLS(t *testing.T, ing *ingressutil.Ingress, hosts [][]string, secretNames []string) { + t.Helper() + switch ing.Mode() { + case ingressutil.IngressModeNetworking: + networkingIngress, err := ing.GetNetworkingIngress() + if err != nil { + t.Error(err) + } + checkIngressTLS(t, networkingIngress, hosts, secretNames) + case ingressutil.IngressModeExtensions: + extensionsIngress, err := ing.GetExtensionsIngress() + if err != nil { + t.Error(err) + } + checkTLSLegacy(t, extensionsIngress, hosts, secretNames) + } +} + +func checkIngressTLS(t *testing.T, ing *networkingv1.Ingress, hosts [][]string, secretNames []string) { + t.Helper() + assert.Equal(t, len(hosts), len(ing.Spec.TLS), "Count of TLS rules differs") + for it := 0; it < len(ing.Spec.TLS); it++ { + assert.Equal(t, secretNames[it], ing.Spec.TLS[it].SecretName, "Secret name differs") + assert.Equal(t, len(hosts[it]), len(ing.Spec.TLS[it].Hosts), "Count of hosts differs") + for ih := 0; ih < len(ing.Spec.TLS[it].Hosts); ih++ { + assert.Equal(t, hosts[it][ih], ing.Spec.TLS[it].Hosts[ih]) + } + } +} + +func checkTLSLegacy(t *testing.T, ing *extensionsv1beta1.Ingress, hosts [][]string, secretNames []string) { + t.Helper() + assert.Equal(t, len(hosts), len(ing.Spec.TLS), "Count of TLS rules differs") + for it := 0; it < len(ing.Spec.TLS); it++ { + assert.Equal(t, secretNames[it], ing.Spec.TLS[it].SecretName, "Secret name differs") + assert.Equal(t, len(hosts[it]), len(ing.Spec.TLS[it].Hosts), "Count of hosts differs") + for ih := 0; ih < len(ing.Spec.TLS[it].Hosts); ih++ { + assert.Equal(t, hosts[it][ih], ing.Spec.TLS[it].Hosts[ih]) + } + } +} + func TestCanaryIngressCreate(t *testing.T) { tests := generateMultiIngressTestData() for _, test := range tests { @@ -347,6 +389,159 @@ func TestCanaryIngressRetainIngressClass(t *testing.T) { } } +func TestCanaryIngressRetainTLS(t *testing.T) { + r := Reconciler{ + cfg: ReconcilerConfig{ + Rollout: fakeRollout(stableService, canaryService, StableIngress, nil), + }, + } + stable := networkingIngress(StableIngress, 80, stableService) + stable.Spec.TLS = []networkingv1.IngressTLS{ + { + Hosts: []string{"fakehost.example.com"}, + SecretName: "tls-secret-name", + }, + { + Hosts: []string{"fakehost.example.com", "*.example.com"}, + SecretName: "tls-secret-name-two", + }, + } + stableIngress := ingressutil.NewIngress(stable) + + desiredCanaryIngress, err := r.canaryIngress(stableIngress, ingressutil.GetCanaryIngressName(r.cfg.Rollout.GetName(), StableIngress), 15) + assert.Nil(t, err, "No error returned when calling canaryIngress") + + checkTLS(t, desiredCanaryIngress, [][]string{ + {"fakehost.example.com"}, + {"fakehost.example.com", "*.example.com"}, + }, []string{ + "tls-secret-name", + "tls-secret-name-two", + }) +} + +func TestCanaryIngressRetainTLSWithMultipleStableIngresses(t *testing.T) { + r := Reconciler{ + cfg: ReconcilerConfig{ + Rollout: fakeRollout(stableService, canaryService, StableIngress, []string{StableIngresses}), + }, + } + + // main + stable := networkingIngress(StableIngress, 80, stableService) + stable.Spec.TLS = []networkingv1.IngressTLS{ + { + Hosts: []string{"fakehost.example.com"}, + SecretName: "tls-secret-name", + }, + } + stableIngress := ingressutil.NewIngress(stable) + + desiredCanaryIngress, err := r.canaryIngress(stableIngress, ingressutil.GetCanaryIngressName(r.cfg.Rollout.GetName(), StableIngress), 15) + assert.Nil(t, err, "No error returned when calling canaryIngress") + + checkTLS(t, desiredCanaryIngress, [][]string{ + {"fakehost.example.com"}, + }, []string{ + "tls-secret-name", + }) + + // additional + stableAdditional := networkingIngress(StableIngresses, 80, stableService) + stableAdditional.Spec.TLS = []networkingv1.IngressTLS{ + { + Hosts: []string{"fakehost-additional.example.com"}, + SecretName: "tls-secret-name-additional", + }, + } + stableAdditionalIngress := ingressutil.NewIngress(stableAdditional) + + desiredAdditionalCanaryIngress, err := r.canaryIngress(stableAdditionalIngress, ingressutil.GetCanaryIngressName(r.cfg.Rollout.GetName(), StableIngresses), 15) + assert.Nil(t, err, "No error returned when calling canaryIngress") + + checkTLS(t, desiredAdditionalCanaryIngress, [][]string{ + {"fakehost-additional.example.com"}, + }, []string{ + "tls-secret-name-additional", + }) + +} + +func TestCanaryIngressRetainLegacyTLS(t *testing.T) { + r := Reconciler{ + cfg: ReconcilerConfig{ + Rollout: fakeRollout(stableService, canaryService, StableIngress, nil), + }, + } + stable := extensionsIngress(StableIngress, 80, stableService) + stable.Spec.TLS = []extensionsv1beta1.IngressTLS{ + { + Hosts: []string{"fakehost.example.com"}, + SecretName: "tls-secret-name", + }, + { + Hosts: []string{"fakehost.example.com", "*.example.com"}, + SecretName: "tls-secret-name-two", + }, + } + stableIngress := ingressutil.NewLegacyIngress(stable) + + desiredCanaryIngress, err := r.canaryIngress(stableIngress, ingressutil.GetCanaryIngressName(r.cfg.Rollout.GetName(), StableIngress), 15) + assert.Nil(t, err, "No error returned when calling canaryIngress") + + checkTLS(t, desiredCanaryIngress, [][]string{ + {"fakehost.example.com"}, + {"fakehost.example.com", "*.example.com"}, + }, []string{ + "tls-secret-name", + "tls-secret-name-two", + }) +} + +func TestCanaryIngressNotAddTLS(t *testing.T) { + r := Reconciler{ + cfg: ReconcilerConfig{ + Rollout: fakeRollout(stableService, canaryService, StableIngress, nil), + }, + } + stable := networkingIngress(StableIngress, 80, stableService) + + stableIngress := ingressutil.NewIngress(stable) + + desiredCanaryIngress, err := r.canaryIngress(stableIngress, ingressutil.GetCanaryIngressName(r.cfg.Rollout.GetName(), StableIngress), 15) + assert.Nil(t, err, "No error returned when calling canaryIngress") + + desired, err := desiredCanaryIngress.GetNetworkingIngress() + + if err != nil { + t.Fatal(err) + } + + assert.Nil(t, desired.Spec.TLS, "No TLS in the the canary ingress should be present") +} + +func TestCanaryIngressNotAddLegacyTLS(t *testing.T) { + r := Reconciler{ + cfg: ReconcilerConfig{ + Rollout: fakeRollout(stableService, canaryService, StableIngress, nil), + }, + } + stable := extensionsIngress(StableIngress, 80, stableService) + + stableIngress := ingressutil.NewLegacyIngress(stable) + + desiredCanaryIngress, err := r.canaryIngress(stableIngress, ingressutil.GetCanaryIngressName(r.cfg.Rollout.GetName(), StableIngress), 15) + assert.Nil(t, err, "No error returned when calling canaryIngress") + + desired, err := desiredCanaryIngress.GetExtensionsIngress() + + if err != nil { + t.Fatal(err) + } + + assert.Nil(t, desired.Spec.TLS, "No TLS in the the canary ingress should be present") +} + func TestCanaryIngressAdditionalAnnotations(t *testing.T) { tests := generateMultiIngressTestData() for _, test := range tests {