Copy parent HTTPOption to endpoint probe, if present. (#713)

* Copy parent HTTPOption to endpoint probe, if present. Fixes #712 * Copy TLS information or probe via HTTP-only * Copy generated hostnames into TLS spec for HTTPS endpoints probe
knative-extensions · Jan 24, 2022 · 8253890 · 8253890
1 parent a248e38
commit 8253890
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 6 deletions.
diff --git a/pkg/reconciler/contour/resources/httpproxy.go b/pkg/reconciler/contour/resources/httpproxy.go
@@ -97,11 +97,10 @@ func MakeHTTPProxies(ctx context.Context, ing *v1alpha1.Ingress, serviceToProtoc
 	ing = ing.DeepCopy()
 	ingress.InsertProbe(ing)
 
-	hostToTLS := make(map[string]*v1alpha1.IngressTLS, len(ing.Spec.TLS))
+	hostToTLS := make(map[string]v1alpha1.IngressTLS, len(ing.Spec.TLS))
 	for _, tls := range ing.Spec.TLS {
 		for _, host := range tls.Hosts {
-			t := tls
-			hostToTLS[host] = &t
+			hostToTLS[host] = tls
 		}
 	}
 

diff --git a/pkg/reconciler/contour/resources/kingress.go b/pkg/reconciler/contour/resources/kingress.go
@@ -47,8 +47,6 @@ func MakeEndpointProbeIngress(ctx context.Context, ing *v1alpha1.Ingress, previo
 			OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)},
 		},
 		Spec: v1alpha1.IngressSpec{
-			// TODO: Probing against HTTP should be enough as it ensures Envoy's EDS?
-			// Need to verify it by scale-N test with HTTPS.
 			HTTPOption: v1alpha1.HTTPOptionEnabled,
 		},
 	}
@@ -106,15 +104,19 @@ func MakeEndpointProbeIngress(ctx context.Context, ing *v1alpha1.Ingress, previo
 	l := order.List()
 	logging.FromContext(ctx).Debugf("Endpoints probe will cover services: %v", l)
 
+	probeHosts := make([]string, 0, len(l))
+
 	for _, name := range l {
 		si := sns[name]
 		if si.HasPath {
 			// TODO(https://github.com/knative-sandbox/net-certmanager/issues/44): Remove this.
 			continue
 		}
 		for _, vis := range si.Visibilities() {
+			host := fmt.Sprintf("%s.gen-%d.%s.%s.net-contour.invalid", name, ing.Generation, ing.Name, ing.Namespace)
+			probeHosts = append(probeHosts, host)
 			childIng.Spec.Rules = append(childIng.Spec.Rules, v1alpha1.IngressRule{
-				Hosts:      []string{fmt.Sprintf("%s.gen-%d.%s.%s.net-contour.invalid", name, ing.Generation, ing.Name, ing.Namespace)},
+				Hosts:      []string{host},
 				Visibility: vis,
 				HTTP: &v1alpha1.HTTPIngressRuleValue{
 					Paths: []v1alpha1.HTTPIngressPath{{
@@ -133,5 +135,16 @@ func MakeEndpointProbeIngress(ctx context.Context, ing *v1alpha1.Ingress, previo
 		}
 	}
 
+	hasCert := len(ing.Spec.TLS) > 0 || config.FromContext(ctx).Contour.DefaultTLSSecret != nil
+
+	if ing.Spec.HTTPOption == v1alpha1.HTTPOptionRedirected && hasCert {
+		// Set the probe to operate over HTTPS IFF we have certificates AND are TLS-required
+		childIng.Spec.HTTPOption = v1alpha1.HTTPOptionRedirected
+		childIng.Spec.TLS = append(childIng.Spec.TLS, ing.Spec.TLS...)
+		for i := range childIng.Spec.TLS {
+			childIng.Spec.TLS[i].Hosts = probeHosts
+		}
+	}
+
 	return childIng
 }
diff --git a/pkg/reconciler/contour/resources/kingress_test.go b/pkg/reconciler/contour/resources/kingress_test.go
@@ -263,6 +263,73 @@ func TestMakeEndpointProbeIngress(t *testing.T) {
 				}},
 			},
 		},
+	}, {
+		name: "https-only",
+		ing: &v1alpha1.Ingress{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "foo",
+				Name:      "bar",
+			},
+			Spec: v1alpha1.IngressSpec{
+				HTTPOption: v1alpha1.HTTPOptionRedirected,
+				Rules: []v1alpha1.IngressRule{{
+					Hosts: []string{"example.com"},
+					HTTP: &v1alpha1.HTTPIngressRuleValue{
+						Paths: []v1alpha1.HTTPIngressPath{{
+							Splits: []v1alpha1.IngressBackendSplit{{
+								IngressBackend: v1alpha1.IngressBackend{
+									ServiceName: "goo",
+									ServicePort: intstr.FromInt(123),
+								},
+								Percent: 100,
+							}},
+						}},
+					},
+				}},
+				TLS: []v1alpha1.IngressTLS{{
+					Hosts:      []string{"example.com"},
+					SecretName: "example",
+				}},
+			},
+		},
+		want: &v1alpha1.Ingress{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "foo",
+				Name:      "bar--ep",
+				Annotations: map[string]string{
+					EndpointsProbeKey: "true",
+				},
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion:         "networking.internal.knative.dev/v1alpha1",
+					Kind:               "Ingress",
+					Name:               "bar",
+					Controller:         ptr.Bool(true),
+					BlockOwnerDeletion: ptr.Bool(true),
+				}},
+			},
+			Spec: v1alpha1.IngressSpec{
+				HTTPOption: v1alpha1.HTTPOptionRedirected,
+				Rules: []v1alpha1.IngressRule{{
+					Hosts: []string{"goo.gen-0.bar.foo.net-contour.invalid"},
+					HTTP: &v1alpha1.HTTPIngressRuleValue{
+						Paths: []v1alpha1.HTTPIngressPath{{
+							Splits: []v1alpha1.IngressBackendSplit{{
+								IngressBackend: v1alpha1.IngressBackend{
+									ServiceNamespace: "foo",
+									ServiceName:      "goo",
+									ServicePort:      intstr.FromInt(123),
+								},
+								Percent: 100,
+							}},
+						}},
+					},
+				}},
+				TLS: []v1alpha1.IngressTLS{{
+					Hosts:      []string{"goo.gen-0.bar.foo.net-contour.invalid"},
+					SecretName: "example",
+				}},
+			},
+		},
 	}, {
 		name: "multiple paths with header conditions",
 		ing: &v1alpha1.Ingress{