Repository: knative-sandbox/net-kourier
Pull request #351 (2020-11-11 10:52:56)

Avoid setting the envoy snapshot twice per reconcile
This **seems** unnecessary and maybe was needed because we used older envoy versions. Let's see what happens if we don't do this.

/assign @jmprusi @davidor 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/net-kourier/pull/351#" title="Author self-approved">markusthoemmes</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fnet-kourier).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/net-kourier/blob/master/OWNERS)~~ [markusthoemmes]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/net-kourier/pull/351?src=pr&el=h1) Report
> Merging [#351](https://codecov.io/gh/knative-sandbox/net-kourier/pull/351?src=pr&el=desc) (31afb70) into [master](https://codecov.io/gh/knative-sandbox/net-kourier/commit/bd0c7ba84172ca48aefc161c01d66c0bcc2d196c?el=desc) (bd0c7ba) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/knative-sandbox/net-kourier/pull/351/graphs/tree.svg?width=650&height=150&src=pr&token=KmlRo23zX4)](https://codecov.io/gh/knative-sandbox/net-kourier/pull/351?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #351   +/-   ##
=======================================
  Coverage   78.79%   78.79%           
=======================================
  Files          14       14           
  Lines         533      533           
=======================================
  Hits          420      420           
  Misses         84       84           
  Partials       29       29           
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/net-kourier/pull/351?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative-sandbox/net-kourier/pull/351?src=pr&el=footer). Last update [bd0c7ba...31afb70](https://codecov.io/gh/knative-sandbox/net-kourier/pull/351?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

markusthoemmes: Running here for additional stability testing: https://github.com/knative/serving/pull/10091

Patch:
diff --git a/pkg/generator/cluster_cache.go b/pkg/generator/cluster_cache.go
index 3b3fa96d..7c4c2aff 100644
--- a/pkg/generator/cluster_cache.go
+++ b/pkg/generator/cluster_cache.go
@@ -46,13 +46,13 @@ import (
 )
 
 const (
-	clusterExpiration      = 15 * time.Second
-	defaultExpiration      = gocache.NoExpiration
+	defaultExpiration      = 15 * time.Second
 	defaultCleanupInterval = 1 * time.Minute
 )
 
 type ClustersCache struct {
-	clusters *gocache.Cache
+	clusterExpiration time.Duration
+	clusters          *gocache.Cache
 }
 
 func newClustersCache() *ClustersCache {
@@ -60,19 +60,19 @@ func newClustersCache() *ClustersCache {
 }
 
 func newClustersCacheWithExpAndCleanupIntervals(expiration time.Duration, cleanupInterval time.Duration) *ClustersCache {
-	goCache := gocache.New(expiration, cleanupInterval)
-	return &ClustersCache{clusters: goCache}
+	goCache := gocache.New(gocache.NoExpiration, cleanupInterval)
+	return &ClustersCache{clusters: goCache, clusterExpiration: expiration}
 }
 
 func (cc *ClustersCache) set(cluster *v2.Cluster, ingressName string, ingressNamespace string) {
 	key := key(cluster.Name, ingressName, ingressNamespace)
-	cc.clusters.Set(key, cluster, defaultExpiration)
+	cc.clusters.Set(key, cluster, gocache.NoExpiration)
 }
 
 func (cc *ClustersCache) setExpiration(clusterName string, ingressName string, ingressNamespace string) {
 	key := key(clusterName, ingressName, ingressNamespace)
 	if cluster, ok := cc.clusters.Get(key); ok {
-		cc.clusters.Set(key, cluster, clusterExpiration)
+		cc.clusters.Set(key, cluster, cc.clusterExpiration)
 	}
 }
 
diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go
index 72b2a5d6..f7fbc15a 100644
--- a/pkg/reconciler/ingress/ingress.go
+++ b/pkg/reconciler/ingress/ingress.go
@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/envoyproxy/go-control-plane/pkg/cache"
 	"knative.dev/net-kourier/pkg/config"
 	envoy "knative.dev/net-kourier/pkg/envoy/server"
 	"knative.dev/net-kourier/pkg/generator"
@@ -120,30 +119,11 @@ func (r *Reconciler) updateIngress(ctx context.Context, ingress *v1alpha1.Ingres
 func (r *Reconciler) updateEnvoyConfig(ctx context.Context) error {
 	logger := logging.FromContext(ctx)
 
-	currentSnapshot, err := r.xdsServer.GetSnapshot(config.EnvoyNodeID)
-	if err != nil {
-		return err
-	}
-
 	logger.Debugf("Preparing Envoy Snapshot")
 	newSnapshot, err := r.caches.ToEnvoySnapshot()
 	if err != nil {
 		return err
 	}
 
-	// Let's warm the Clusters first, by sending the previous snapshot with the new cluster list, that includes
-	// both new and old clusters.
-	currentSnapshot.Resources[cache.Cluster].Items = newSnapshot.GetResources(cache.ClusterType)
-
-	//Validate that the snapshot is consistent.
-	if err := currentSnapshot.Consistent(); err != nil {
-		return err
-	}
-
-	if err := r.xdsServer.SetSnapshot(&currentSnapshot, nodeID); err != nil {
-		return err
-	}
-
-	// Now, send the full new snapshot.
 	return r.xdsServer.SetSnapshot(&newSnapshot, nodeID)
 }


Test patch:
diff --git a/pkg/generator/caches_test.go b/pkg/generator/caches_test.go
index efbc77b1..022fdfea 100644
--- a/pkg/generator/caches_test.go
+++ b/pkg/generator/caches_test.go
@@ -38,9 +38,7 @@ func TestDeleteIngressInfo(t *testing.T) {
 	ctx := context.Background()
 
 	caches, err := NewCaches(ctx, &kubeClient, false)
-	if err != nil {
-		t.Fail()
-	}
+	assert.NilError(t, err)
 
 	// Add info for an ingress
 	firstIngressName := "ingress_1"
@@ -69,7 +67,7 @@ func TestDeleteIngressInfo(t *testing.T) {
 	)
 
 	// Delete the first ingress
-	_ = caches.DeleteIngressInfo(ctx, firstIngressName, firstIngressNamespace)
+	caches.DeleteIngressInfo(ctx, firstIngressName, firstIngressNamespace)
 
 	// Check that the listeners only have the virtual hosts of the second
 	// ingress.
@@ -96,9 +94,7 @@ func TestDeleteIngressInfoWhenDoesNotExist(t *testing.T) {
 	ctx := context.Background()
 
 	caches, err := NewCaches(ctx, &kubeClient, false)
-	if err != nil {
-		t.Fail()
-	}
+	assert.NilError(t, err)
 
 	// Add info for an ingress
 	firstIngressName := "ingress_1"
@@ -114,23 +110,17 @@ func TestDeleteIngressInfoWhenDoesNotExist(t *testing.T) {
 	)
 
 	snapshotBeforeDelete, err := caches.ToEnvoySnapshot()
-	if err != nil {
-		t.FailNow()
-	}
+	assert.NilError(t, err)
 
 	clustersBeforeDelete := snapshotBeforeDelete.GetResources(cache.ClusterType)
 	routesBeforeDelete := snapshotBeforeDelete.GetResources(cache.RouteType)
 	listenersBeforeDelete := snapshotBeforeDelete.GetResources(cache.ListenerType)
 
 	err = caches.DeleteIngressInfo(ctx, "non_existing_name", "non_existing_namespace")
-	if err != nil {
-		t.FailNow()
-	}
+	assert.NilError(t, err)
 
 	snapshotAfterDelete, err := caches.ToEnvoySnapshot()
-	if err != nil {
-		t.FailNow()
-	}
+	assert.NilError(t, err)
 
 	clustersAfterDelete := snapshotAfterDelete.GetResources(cache.ClusterType)
 	routesAfterDelete := snapshotAfterDelete.GetResources(cache.RouteType)
@@ -173,8 +163,8 @@ func createTestDataForIngress(
 		internalVirtualHosts: []*route.VirtualHost{{Name: internalVHostName, Domains: []string{internalVHostName}}},
 	}
 
-	_ = caches.addTranslatedIngress(translatedIngress)
-	_ = caches.setListeners(ctx)
+	caches.addTranslatedIngress(translatedIngress)
+	caches.setListeners(ctx)
 }
 
 func TestValidateIngress(t *testing.T) {
@@ -182,9 +172,7 @@ func TestValidateIngress(t *testing.T) {
 	ctx := context.Background()
 
 	caches, err := NewCaches(ctx, &kubeClient, false)
-	if err != nil {
-		t.Fail()
-	}
+	assert.NilError(t, err)
 
 	createTestDataForIngress(
 		ctx,
diff --git a/pkg/generator/cluster_cache_test.go b/pkg/generator/cluster_cache_test.go
index 9a7252ee..ddf018d1 100644
--- a/pkg/generator/cluster_cache_test.go
+++ b/pkg/generator/cluster_cache_test.go
@@ -23,6 +23,8 @@ import (
 
 	envoy_api_v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+	"k8s.io/apimachinery/pkg/util/wait"
 )
 
 var testCluster1 = envoy_api_v2.Cluster{
@@ -39,7 +41,7 @@ func TestSetCluster(t *testing.T) {
 
 	list := cache.list()
 
-	assert.Equal(t, 1, len(list))
+	assert.Assert(t, is.Len(list, 1))
 	assert.Equal(t, testCluster1.Name, list[0].(*envoy_api_v2.Cluster).Name)
 }
 
@@ -54,30 +56,35 @@ func TestSetSeveralClusters(t *testing.T) {
 		names = append(names, cluster.(*envoy_api_v2.Cluster).Name)
 	}
 
-	assert.Equal(t, 2, len(list))
-
 	// Could be returned in any order
 	expectedNames := []string{testCluster1.Name, testCluster2.Name}
 	sort.Strings(expectedNames)
 	sort.Strings(names)
+
 	assert.DeepEqual(t, expectedNames, names)
 }
 
 func TestClustersExpire(t *testing.T) {
-	cleanupInterval := time.Second
-	cache := newClustersCacheWithExpAndCleanupIntervals(time.Second, cleanupInterval)
-	cache.setExpiration(testCluster1.Name, "some_ingress_name", "some_ingress_namespace")
-	time.Sleep(cleanupInterval + time.Second)
+	interval := 10 * time.Millisecond
+	cache := newClustersCacheWithExpAndCleanupIntervals(interval, interval)
+	cache.set(&testCluster1, "some_ingress_name", "some_ingress_namespace")
+	assert.Assert(t, is.Len(cache.list(), 1))
 
-	list := cache.list()
+	// Wait for twice the interval and assert that the cluster is still there.
+	time.Sleep(2 * interval)
+	assert.Assert(t, is.Len(cache.list(), 1))
 
-	assert.Equal(t, 0, len(list))
+	// Mark the cluster to be expired.
+	cache.setExpiration(testCluster1.Name, "some_ingress_name", "some_ingress_namespace")
+
+	// The cluster should eventually disappear.
+	wait.PollImmediate(interval, 5*time.Second, func() (bool, error) {
+		return len(cache.list()) == 0, nil
+	})
+	assert.Assert(t, is.Len(cache.list(), 0))
 }
 
 func TestListWhenThereAreNoClusters(t *testing.T) {
 	cache := newClustersCache()
-
-	list := cache.list()
-
-	assert.Equal(t, 0, len(list))
+	assert.Assert(t, is.Len(cache.list(), 0))
 }
diff --git a/pkg/generator/ingress_translator_test.go b/pkg/generator/ingress_translator_test.go
index a4774829..532d8851 100644
--- a/pkg/generator/ingress_translator_test.go
+++ b/pkg/generator/ingress_translator_test.go
@@ -17,503 +17,495 @@ limitations under the License.
 package generator
 
 import (
-	"context"
-	"fmt"
-	"sort"
 	"testing"
+	"time"
 
 	v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
+	endpoint "github.com/envoyproxy/go-control-plane/envoy/api/v2/endpoint"
 	route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route"
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"google.golang.org/protobuf/types/known/durationpb"
+	"google.golang.org/protobuf/types/known/wrapperspb"
 	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	envoy "knative.dev/net-kourier/pkg/envoy/api"
 	"knative.dev/networking/pkg/apis/networking/v1alpha1"
 	pkgtest "knative.dev/pkg/reconciler/testing"
 )
 
-// Tests that when there is a traffic split defined in the ingress:
-// - Creates a route with weighted clusters defined.
-// - There's one weighted cluster for each traffic split and each contains:
-// 		- The traffic percentage.
-//		- The headers to add (namespace and revision name).
-//		- The cluster name, based on the revision name plus the path.
-// - The weighted clusters exists also in the clusters cache with the same
-//   name.
-//
-// Note: for now, the name of the cluster is the name of the revision plus the
-// path. That might change in the future.
-func TestTrafficSplits(t *testing.T) {
-	ingress := v1alpha1.Ingress{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "Ingress",
-			APIVersion: "networking.internal.knative.dev/v1alpha1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "hello-world",
+func TestIngressTranslator(t *testing.T) {
+	tests := []struct {
+		name  string
+		in    *v1alpha1.Ingress
+		state []runtime.Object
+		want  *translatedIngress
+	}{{
+		name: "simple",
+		in:   ing("simplens", "simplename"),
+		state: []runtime.Object{
+			svc("servicens", "servicename"),
+			eps("servicens", "servicename"),
 		},
-		Spec: v1alpha1.IngressSpec{
-			Rules: []v1alpha1.IngressRule{{
-				HTTP: &v1alpha1.HTTPIngressRuleValue{
-					Paths: []v1alpha1.HTTPIngressPath{{
-						Splits: []v1alpha1.IngressBackendSplit{{
-							IngressBackend: v1alpha1.IngressBackend{
-								ServiceNamespace: "default",
-								ServiceName:      "hello-world-rev1",
-								ServicePort: intstr.IntOrString{
-									Type:   intstr.Int,
-									IntVal: 80,
-								},
-							},
-							Percent: 60,
-							AppendHeaders: map[string]string{
-								"Knative-Serving-Namespace": "default",
-								"Knative-Serving-Revision":  "hello-world-rev1",
-							},
-						}, {
-							IngressBackend: v1alpha1.IngressBackend{
-								ServiceNamespace: "default",
-								ServiceName:      "hello-world-rev2",
-								ServicePort: intstr.IntOrString{
-									Type:   intstr.Int,
-									IntVal: 80,
-								},
-							},
-							Percent: 40,
-							AppendHeaders: map[string]string{
-								"Knative-Serving-Namespace": "default",
-								"Knative-Serving-Revision":  "hello-world-rev2",
+		want: func() *translatedIngress {
+			vHosts := []*route.VirtualHost{
+				envoy.NewVirtualHost(
+					"simplename",
+					[]string{"foo.example.com", "foo.example.com:*"},
+					[]*route.Route{envoy.NewRoute(
+						"simplename_simplens_/test",
+						[]*route.HeaderMatcher{{
+							Name: "testheader",
+							HeaderMatchSpecifier: &route.HeaderMatcher_ExactMatch{
+								ExactMatch: "foo",
 							},
 						}},
-					}},
-				},
-			}},
-		},
-		Status: v1alpha1.IngressStatus{},
-	}
-
-	kubeClient := fake.NewSimpleClientset()
-	ctx := context.Background()
-	// Create the Kubernetes services associated to the Knative services that
-	// appear in the ingress above
-	if err := createServicesWithNames(
-		ctx,
-		kubeClient,
-		[]string{"hello-world-rev1", "hello-world-rev2"},
-		"default",
-	); err != nil {
-		t.Error(err)
-	}
-
-	ingressTranslator := NewIngressTranslator(
-		mockedSecretGetter, mockedEndpointsGetter, mockedServiceGetter, &pkgtest.FakeTracker{})
-
-	ingressTranslation, err := ingressTranslator.translateIngress(ctx, &ingress, false)
-	if err != nil {
-		t.Error(err)
-	}
-	assert.Equal(t, 1, len(ingressTranslation.internalVirtualHosts))
-	assert.Equal(t, 1, len(ingressTranslation.internalVirtualHosts[0].GetRoutes()))
-
-	// Check that there are 2 weighted clusters for the route
-	envoyRoute := ingressTranslation.internalVirtualHosts[0].GetRoutes()[0]
-	weightedClusters := envoyRoute.GetRoute().GetWeightedClusters().Clusters
-	assert.Equal(t, 2, len(weightedClusters))
+						"/test",
+						[]*route.WeightedCluster_ClusterWeight{
+							envoy.NewWeightedCluster("servicename/test", 100, map[string]string{"baz": "gna"}),
+						},
+						0,
+						map[string]string{"foo": "bar"},
+						"rewritten.example.com"),
+					},
+				),
+			}
 
-	// Check the first weighted cluster
-	assertWeightedClusterCorrect(
-		t,
-		weightedClusters[0],
-		"hello-world-rev1/",
-		uint32(60),
-		map[string]string{
-			"Knative-Serving-Namespace": "default",
-			"Knative-Serving-Revision":  "hello-world-rev1",
+			return &translatedIngress{
+				name: types.NamespacedName{
+					Namespace: "simplens",
+					Name:      "simplename",
+				},
+				sniMatches: []*envoy.SNIMatch{},
+				clusters: []*v2.Cluster{
+					envoy.NewCluster(
+						"servicename/test",
+						5*time.Second,
+						lbEndpoints,
+						false,
+						v2.Cluster_STATIC,
+					),
+				},
+				externalVirtualHosts: vHosts,
+				internalVirtualHosts: vHosts,
+			}
+		}(),
+	}, {
+		name: "tls",
+		in: ing("testspace", "testname", func(ing *v1alpha1.Ingress) {
+			ing.Spec.TLS = []v1alpha1.IngressTLS{{
+				Hosts:           []string{"foo.example.com"},
+				SecretNamespace: "secretns",
+				SecretName:      "secretname",
+			}}
+		}),
+		state: []runtime.Object{
+			svc("servicens", "servicename"),
+			eps("servicens", "servicename"),
+			secret("secretns", "secretname"),
 		},
-	)
+		want: func() *translatedIngress {
+			vHosts := []*route.VirtualHost{
+				envoy.NewVirtualHost(
+					"testname",
+					[]string{"foo.example.com", "foo.example.com:*"},
+					[]*route.Route{envoy.NewRoute(
+						"testname_testspace_/test",
+						[]*route.HeaderMatcher{{
+							Name: "testheader",
+							HeaderMatchSpecifier: &route.HeaderMatcher_ExactMatch{
+								ExactMatch: "foo",
+							},
+						}},
+						"/test",
+						[]*route.WeightedCluster_ClusterWeight{
+							envoy.NewWeightedCluster("servicename/test", 100, map[string]string{"baz": "gna"}),
+						},
+						0,
+						map[string]string{"foo": "bar"},
+						"rewritten.example.com"),
+					},
+				),
+			}
 
-	// Check the second weighted cluster
-	assertWeightedClusterCorrect(
-		t,
-		weightedClusters[1],
-		"hello-world-rev2/",
-		uint32(40),
-		map[string]string{
-			"Knative-Serving-Namespace": "default",
-			"Knative-Serving-Revision":  "hello-world-rev2",
+			return &translatedIngress{
+				name: types.NamespacedName{
+					Namespace: "testspace",
+					Name:      "testname",
+				},
+				sniMatches: []*envoy.SNIMatch{{
+					Hosts: []string{"foo.example.com"},
+					CertSource: types.NamespacedName{
+						Namespace: "secretns",
+						Name:      "secretname",
+					},
+					CertificateChain: cert,
+					PrivateKey:       privateKey,
+				}},
+				clusters: []*v2.Cluster{
+					envoy.NewCluster(
+						"servicename/test",
+						5*time.Second,
+						lbEndpoints,
+						false,
+						v2.Cluster_STATIC,
+					),
+				},
+				externalVirtualHosts: vHosts,
+				internalVirtualHosts: vHosts,
+			}
+		}(),
+	}, {
+		name: "split",
+		in: ing("testspace", "testname", func(ing *v1alpha1.Ingress) {
+			path := &ing.Spec.Rules[0].HTTP.Paths[0]
+			path.Splits[0].Percent = 33
+			path.Splits = append(path.Splits, v1alpha1.IngressBackendSplit{
+				Percent: 33,
+				IngressBackend: v1alpha1.IngressBackend{
+					ServiceNamespace: "servicens2",
+					ServiceName:      "servicename2",
+					ServicePort:      intstr.FromString("http"),
+				},
+			}, v1alpha1.IngressBackendSplit{
+				Percent: 34,
+				IngressBackend: v1alpha1.IngressBackend{
+					ServiceNamespace: "servicens3",
+					ServiceName:      "servicename3",
+					ServicePort:      intstr.FromString("http"),
+				},
+			})
+		}),
+		state: []runtime.Object{
+			svc("servicens", "servicename"),
+			eps("servicens", "servicename"),
+			svc("servicens2", "servicename2"),
+			eps("servicens2", "servicename2"),
+			svc("servicens3", "servicename3", func(svc *corev1.Service) {
+				svc.Spec.Type = corev1.ServiceTypeExternalName
+				svc.Spec.ExternalName = "example.com"
+			}),
 		},
-	)
-
-	// Check the clusters cache
-	assert.Equal(
-		t,
-		true,
-		clustersExist([]string{"hello-world-rev1/", "hello-world-rev2/"}, ingressTranslation.clusters),
-	)
-}
-
-func TestExternalNameService(t *testing.T) {
-	ctx := context.Background()
+		want: func() *translatedIngress {
+			vHosts := []*route.VirtualHost{
+				envoy.NewVirtualHost(
+					"testname",
+					[]string{"foo.example.com", "foo.example.com:*"},
+					[]*route.Route{envoy.NewRoute(
+						"testname_testspace_/test",
+						[]*route.HeaderMatcher{{
+							Name: "testheader",
+							HeaderMatchSpecifier: &route.HeaderMatcher_ExactMatch{
+								ExactMatch: "foo",
+							},
+						}},
+						"/test",
+						[]*route.WeightedCluster_ClusterWeight{
+							envoy.NewWeightedCluster("servicename/test", 33, map[string]string{"baz": "gna"}),
+							envoy.NewWeightedCluster("servicename2/test", 33, nil),
+							envoy.NewWeightedCluster("servicename3/test", 34, nil),
+						},
+						0,
+						map[string]string{"foo": "bar"},
+						"rewritten.example.com"),
+					},
+				),
+			}
 
-	ingress := createIngress("test", []string{"foo", "bar"}, v1alpha1.IngressVisibilityExternalIP)
-	svc := &corev1.Service{
-		Spec: corev1.ServiceSpec{
-			Type:         corev1.ServiceTypeExternalName,
-			ExternalName: "example.com",
-			Ports: []corev1.ServicePort{{
-				Name:       "http",
-				Port:       80,
-				TargetPort: intstr.FromInt(8080),
-			}},
+			return &translatedIngress{
+				name: types.NamespacedName{
+					Namespace: "testspace",
+					Name:      "testname",
+				},
+				sniMatches: []*envoy.SNIMatch{},
+				clusters: []*v2.Cluster{
+					envoy.NewCluster(
+						"servicename/test",
+						5*time.Second,
+						lbEndpoints,
+						false,
+						v2.Cluster_STATIC,
+					),
+					envoy.NewCluster(
+						"servicename2/test",
+						5*time.Second,
+						lbEndpoints,
+						false,
+						v2.Cluster_STATIC,
+					),
+					envoy.NewCluster(
+						"servicename3/test",
+						5*time.Second,
+						[]*endpoint.LbEndpoint{envoy.NewLBEndpoint("example.com", 80)},
+						false,
+						v2.Cluster_LOGICAL_DNS,
+					),
+				},
+				externalVirtualHosts: vHosts,
+				internalVirtualHosts: vHosts,
+			}
+		}(),
+	}, {
+		name: "path defaulting",
+		in: ing("testspace", "testname", func(ing *v1alpha1.Ingress) {
+			ing.Spec.Rules[0].HTTP.Paths[0].Path = ""
+		}),
+		state: []runtime.Object{
+			svc("servicens", "servicename"),
+			eps("servicens", "servicename"),
 		},
-	}
+		want: func() *translatedIngress {
+			vHosts := []*route.VirtualHost{
+				envoy.NewVirtualHost(
+					"testname",
+					[]string{"foo.example.com", "foo.example.com:*"},
+					[]*route.Route{envoy.NewRoute(
+						"testname_testspace_",
+						[]*route.HeaderMatcher{{
+							Name: "testheader",
+							HeaderMatchSpecifier: &route.HeaderMatcher_ExactMatch{
+								ExactMatch: "foo",
+							},
+						}},
+						"/",
+						[]*route.WeightedCluster_ClusterWeight{
+							envoy.NewWeightedCluster("servicename/", 100, map[string]string{"baz": "gna"}),
+						},
+						0,
+						map[string]string{"foo": "bar"},
+						"rewritten.example.com"),
+					},
+				),
+			}
 
-	ingressTranslator := NewIngressTranslator(
-		mockedSecretGetter,
-		mockedEndpointsGetter,
-		func(ns, name string) (*corev1.Service, error) {
-			return svc, nil
+			return &translatedIngress{
+				name: types.NamespacedName{
+					Namespace: "testspace",
+					Name:      "testname",
+				},
+				sniMatches: []*envoy.SNIMatch{},
+				clusters: []*v2.Cluster{
+					envoy.NewCluster(
+						"servicename/",
+						5*time.Second,
+						lbEndpoints,
+						false,
+						v2.Cluster_STATIC,
+					),
+				},
+				externalVirtualHosts: vHosts,
+				internalVirtualHosts: vHosts,
+			}
+		}(),
+	}, {
+		name: "external service",
+		in:   ing("testspace", "testname"),
+		state: []runtime.Object{
+			svc("servicens", "servicename", func(svc *corev1.Service) {
+				svc.Spec.Type = corev1.ServiceTypeExternalName
+				svc.Spec.ExternalName = "example.com"
+			}),
 		},
-		&pkgtest.FakeTracker{})
-
-	translatedIngress, err := ingressTranslator.translateIngress(ctx, ingress, false)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	clusters := translatedIngress.clusters
-	assert.Assert(t, is.Len(clusters, 1))
-
-	cluster := clusters[0]
-	assert.Assert(t, is.Nil(cluster.Http2ProtocolOptions))
-	assert.Equal(t, cluster.GetType(), v2.Cluster_LOGICAL_DNS)
-
-	localityEps := cluster.GetLoadAssignment().GetEndpoints()
-	assert.Assert(t, is.Len(localityEps, 1))
-
-	eps := localityEps[0].LbEndpoints
-	assert.Assert(t, is.Len(eps, 1))
-
-	ep := eps[0]
-	assert.Equal(t, ep.GetEndpoint().GetAddress().GetSocketAddress().GetAddress(), "example.com")
-}
+		want: func() *translatedIngress {
+			vHosts := []*route.VirtualHost{
+				envoy.NewVirtualHost(
+					"testname",
+					[]string{"foo.example.com", "foo.example.com:*"},
+					[]*route.Route{envoy.NewRoute(
+						"testname_testspace_/test",
+						[]*route.HeaderMatcher{{
+							Name: "testheader",
+							HeaderMatchSpecifier: &route.HeaderMatcher_ExactMatch{
+								ExactMatch: "foo",
+							},
+						}},
+						"/test",
+						[]*route.WeightedCluster_ClusterWeight{
+							envoy.NewWeightedCluster("servicename/test", 100, map[string]string{"baz": "gna"}),
+						},
+						0,
+						map[string]string{"foo": "bar"},
+						"rewritten.example.com"),
+					},
+				),
+			}
 
-func TestIngressVisibility(t *testing.T) {
-	ctx := context.Background()
-	kubeClient := fake.NewSimpleClientset()
-	tests := []struct {
-		name       string
-		hosts      []string
-		extDomains []string
-		intDomains []string
-		visibility v1alpha1.IngressVisibility
-	}{{
-		name:       "external visibility",
-		hosts:      []string{"hello.default.example.com"},
-		extDomains: []string{"hello.default.example.com", "hello.default.example.com:*"},
-		// External should also be accessible internally
-		intDomains: []string{"hello.default.example.com", "hello.default.example.com:*"},
-		visibility: v1alpha1.IngressVisibilityExternalIP,
+			return &translatedIngress{
+				name: types.NamespacedName{
+					Namespace: "testspace",
+					Name:      "testname",
+				},
+				sniMatches: []*envoy.SNIMatch{},
+				clusters: []*v2.Cluster{
+					envoy.NewCluster(
+						"servicename/test",
+						5*time.Second,
+						[]*endpoint.LbEndpoint{envoy.NewLBEndpoint("example.com", 80)},
+						false,
+						v2.Cluster_LOGICAL_DNS,
+					),
+				},
+				externalVirtualHosts: vHosts,
+				internalVirtualHosts: vHosts,
+			}
+		}(),
+	}, {
+		name: "missing service",
+		in:   ing("testspace", "testname"),
 	}, {
-		name:       "cluster local visibility",
-		hosts:      []string{"hello.default", "hello.default.svc", "hello.default.svc.cluster.local"},
-		intDomains: []string{"hello.default", "hello.default:*", "hello.default.svc", "hello.default.svc:*", "hello.default.svc.cluster.local", "hello.default.svc.cluster.local:*"},
-		visibility: v1alpha1.IngressVisibilityClusterLocal,
+		name:  "missing endpoints",
+		in:    ing("testspace", "testname"),
+		state: []runtime.Object{svc("servicens", "servicename")},
 	}}
 
-	for num, test := range tests {
+	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			ingress := createIngress(fmt.Sprintf("hello-%d", num), test.hosts, test.visibility)
+			ctx, _ := pkgtest.SetupFakeContext(t)
+			kubeclient := fake.NewSimpleClientset(test.state...)
 
-			// Create the Kubernetes services associated to the Knative service that
-			// appears in the ingress above
-			if err := createServicesWithNames(ctx, kubeClient, []string{ingress.ObjectMeta.Name}, ingress.ObjectMeta.Namespace); err != nil {
-				t.Error(err)
-			}
-
-			ingressTranslator := NewIngressTranslator(
-				mockedSecretGetter, mockedEndpointsGetter, mockedServiceGetter, &pkgtest.FakeTracker{})
-
-			translatedIngress, err := ingressTranslator.translateIngress(ctx, ingress, false)
-			if err != nil {
-				t.Error(err)
-			}
-			var extDomains, intDomains []string
-			var extHosts, intHosts []*route.VirtualHost
-			extHosts = translatedIngress.externalVirtualHosts
-			intHosts = translatedIngress.internalVirtualHosts
-
-			for _, v := range extHosts {
-				extDomains = append(extDomains, v.Domains...)
-			}
-			for _, v := range intHosts {
-				intDomains = append(intDomains, v.Domains...)
-			}
-			sort.Strings(extDomains)
-			sort.Strings(intDomains)
-			sort.Strings(test.extDomains)
-			sort.Strings(test.intDomains)
-			assert.DeepEqual(t, extDomains, test.extDomains)
-			assert.DeepEqual(t, intDomains, test.intDomains)
+			translator := NewIngressTranslator(
+				func(ns, name string) (*corev1.Secret, error) {
+					return kubeclient.CoreV1().Secrets(ns).Get(ctx, name, metav1.GetOptions{})
+				},
+				func(ns, name string) (*corev1.Endpoints, error) {
+					return kubeclient.CoreV1().Endpoints(ns).Get(ctx, name, metav1.GetOptions{})
+				},
+				func(ns, name string) (*corev1.Service, error) {
+					return kubeclient.CoreV1().Services(ns).Get(ctx, name, metav1.GetOptions{})
+				},
+				&pkgtest.FakeTracker{},
+			)
+
+			got, err := translator.translateIngress(ctx, test.in, false)
+			assert.NilError(t, err)
+			assert.DeepEqual(t, got, test.want,
+				cmp.AllowUnexported(translatedIngress{}),
+				cmpopts.IgnoreUnexported(durationpb.Duration{}, wrapperspb.UInt32Value{}, wrapperspb.BoolValue{}),
+			)
 		})
 	}
 }
 
-func TestIngressWithTLS(t *testing.T) {
-	// TLS data for the test
-	tlsSecretName := "tls-secret"
-	tlsSecretNamespace := "default"
-	tlsHosts := []string{"hello-world.example.com"}
-	tlsCert := []byte("some-cert")
-	tlsKey := []byte("tls-key")
-	svcNamespace := "default"
-	ctx := context.Background()
-
-	ingress := createIngressWithTLS(tlsHosts, tlsSecretName, tlsSecretNamespace, svcNamespace)
-
-	kubeClient := fake.NewSimpleClientset()
-
-	// Create the Kubernetes services associated to the Knative service that
-	// appears in the ingress above
-	if err := createServicesWithNames(ctx, kubeClient, []string{ingress.ObjectMeta.Name}, svcNamespace); err != nil {
-		t.Error(err)
-	}
-
-	// Create secret with TLS data
-	tlsSecret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: tlsSecretNamespace,
-			Name:      tlsSecretName,
-		},
-		Data: map[string][]byte{
-			certFieldInSecret: tlsCert,
-			keyFieldInSecret:  tlsKey,
-		},
-	}
-	secretRef := types.NamespacedName{
-		Namespace: tlsSecretNamespace,
-		Name:      tlsSecretName,
-	}
-
-	ingressTranslator := NewIngressTranslator(
-		func(ns, name string) (*corev1.Secret, error) {
-			return tlsSecret, nil
-		},
-		mockedEndpointsGetter,
-		mockedServiceGetter,
-		&pkgtest.FakeTracker{})
-
-	translatedIngress, err := ingressTranslator.translateIngress(ctx, ingress, false)
-	if err != nil {
-		t.Error(err)
-	}
-
-	assert.Equal(t, 1, len(translatedIngress.sniMatches))
-	assert.DeepEqual(
-		t,
-		&envoy.SNIMatch{
-			Hosts:            tlsHosts,
-			CertSource:       secretRef,
-			CertificateChain: tlsCert,
-			PrivateKey:       tlsKey,
-		},
-		translatedIngress.sniMatches[0],
-		cmp.AllowUnexported(envoy.SNIMatch{}),
-	)
-}
-
-func TestReturnsErrorWhenTLSSecretDoesNotExist(t *testing.T) {
-	tlsSecretName := "tls-secret"
-	tlsSecretNamespace := "default"
-	tlsHosts := []string{"hello-world.example.com"}
-	svcNamespace := "default"
-
-	ingress := createIngressWithTLS(tlsHosts, tlsSecretName, tlsSecretNamespace, svcNamespace)
-
-	kubeClient := fake.NewSimpleClientset()
-	ctx := context.Background()
-
-	// Create the Kubernetes services associated to the Knative service that
-	// appears in the ingress above
-	if err := createServicesWithNames(ctx, kubeClient, []string{ingress.ObjectMeta.Name}, "default"); err != nil {
-		t.Error(err)
-	}
-
-	ingressTranslator := NewIngressTranslator(
-		func(ns, name string) (*corev1.Secret, error) {
-			return nil, fmt.Errorf("secrets %q not found", name)
-		},
-		mockedEndpointsGetter,
-		mockedServiceGetter,
-		&pkgtest.FakeTracker{})
-
-	_, err := ingressTranslator.translateIngress(ctx, ingress, false)
-
-	assert.Error(t, err, fmt.Sprintf("failed to fetch secret: secrets %q not found", tlsSecretName))
-}
-
-var mockedSecretGetter = func(ns, name string) (*corev1.Secret, error) {
-	return &corev1.Secret{}, nil
-}
-
-var mockedEndpointsGetter = func(ns, name string) (*corev1.Endpoints, error) {
-	return &corev1.Endpoints{}, nil
-}
-
-var mockedServiceGetter = func(ns, name string) (*corev1.Service, error) {
-	return &corev1.Service{}, nil
-}
-
-func createIngress(name string, hosts []string, visibility v1alpha1.IngressVisibility) *v1alpha1.Ingress {
-	return &v1alpha1.Ingress{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "Ingress",
-			APIVersion: "networking.internal.knative.dev/v1alpha1",
-		},
+func ing(ns, name string, opts ...func(*v1alpha1.Ingress)) *v1alpha1.Ingress {
+	ingress := &v1alpha1.Ingress{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
+			Namespace: ns,
+			Name:      name,
 		},
 		Spec: v1alpha1.IngressSpec{
 			Rules: []v1alpha1.IngressRule{{
-				Hosts:      hosts,
-				Visibility: visibility,
+				Hosts:      []string{"foo.example.com"},
+				Visibility: v1alpha1.IngressVisibilityExternalIP,
 				HTTP: &v1alpha1.HTTPIngressRuleValue{
 					Paths: []v1alpha1.HTTPIngressPath{{
+						RewriteHost: "rewritten.example.com",
+						Headers: map[string]v1alpha1.HeaderMatch{
+							"testheader": {Exact: "foo"},
+						},
+						Path: "/test",
+						AppendHeaders: map[string]string{
+							"foo": "bar",
+						},
 						Splits: []v1alpha1.IngressBackendSplit{{
+							Percent: 100,
+							AppendHeaders: map[string]string{
+								"baz": "gna",
+							},
 							IngressBackend: v1alpha1.IngressBackend{
-								ServiceName: name,
-								ServicePort: intstr.FromInt(80),
+								ServiceNamespace: "servicens",
+								ServiceName:      "servicename",
+								ServicePort:      intstr.FromString("http"),
 							},
 						}},
 					}},
 				},
 			}},
 		},
-		Status: v1alpha1.IngressStatus{},
 	}
+
+	for _, opt := range opts {
+		opt(ingress)
+	}
+
+	return ingress
 }
 
-func createIngressWithTLS(hosts []string, secretName string, secretNamespace string, svcNamespace string) *v1alpha1.Ingress {
-	return &v1alpha1.Ingress{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "Ingress",
-			APIVersion: "networking.internal.knative.dev/v1alpha1",
-		},
+func svc(ns, name string, opts ...func(*corev1.Service)) *corev1.Service {
+	service := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "hello-world",
+			Namespace: ns,
+			Name:      name,
 		},
-		Spec: v1alpha1.IngressSpec{
-			TLS: []v1alpha1.IngressTLS{{
-				Hosts:           hosts,
-				SecretName:      secretName,
-				SecretNamespace: secretNamespace,
-			}},
-			Rules: []v1alpha1.IngressRule{{
-				Visibility: v1alpha1.IngressVisibilityExternalIP,
-				HTTP: &v1alpha1.HTTPIngressRuleValue{
-					Paths: []v1alpha1.HTTPIngressPath{{
-						Splits: []v1alpha1.IngressBackendSplit{{
-							IngressBackend: v1alpha1.IngressBackend{
-								ServiceNamespace: svcNamespace,
-								ServiceName:      "hello-world",
-								ServicePort: intstr.IntOrString{
-									Type:   intstr.Int,
-									IntVal: 80,
-								},
-							},
-							AppendHeaders: map[string]string{
-								"Knative-Serving-Namespace": svcNamespace,
-								"Knative-Serving-Revision":  "hello-world",
-							},
-						}},
-					}},
-				},
+		Spec: corev1.ServiceSpec{
+			Type:      corev1.ServiceTypeClusterIP,
+			ClusterIP: "1.1.1.1",
+			Ports: []corev1.ServicePort{{
+				Name:       "foo",
+				Port:       1337,
+				TargetPort: intstr.FromInt(1338),
+			}, {
+				Name:       "http",
+				Port:       80,
+				TargetPort: intstr.FromInt(8080),
 			}},
 		},
-		Status: v1alpha1.IngressStatus{},
 	}
 
-}
-
-func createServicesWithNames(ctx context.Context, kubeclient kubernetes.Interface, names []string, namespace string) error {
-	for _, serviceName := range names {
-		service := corev1.Service{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: serviceName,
-			},
-		}
-
-		_, err := kubeclient.CoreV1().Services(namespace).Create(ctx, &service, metav1.CreateOptions{})
-
-		if err != nil {
-			return err
-		}
+	for _, opt := range opts {
+		opt(service)
 	}
 
-	return nil
+	return service
 }
 
-func clustersExist(names []string, clusters []*v2.Cluster) bool {
-	// Create map that contains names that are present
-	present := make(map[string]bool)
-	for _, cacheCluster := range clusters {
-		present[cacheCluster.Name] = true
-	}
-
-	// Verify if the received names are present
-	for _, name := range names {
-		if !present[name] {
-			return false
-		}
+func eps(ns, name string) *corev1.Endpoints {
+	return &corev1.Endpoints{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: ns,
+			Name:      name,
+		},
+		Subsets: []corev1.EndpointSubset{{
+			Addresses: []corev1.EndpointAddress{{
+				IP: "2.2.2.2",
+			}, {
+				IP: "3.3.3.3",
+			}},
+		}, {
+			Addresses: []corev1.EndpointAddress{{
+				IP: "4.4.4.4",
+			}, {
+				IP: "5.5.5.5",
+			}},
+		}},
 	}
-
-	return true
 }
 
-// Checks whether the weightedCluster received has the received name, traffic
-// percentage and headers to add
-func assertWeightedClusterCorrect(t *testing.T,
-	weightedCluster *route.WeightedCluster_ClusterWeight,
-	name string,
-	trafficPerc uint32,
-	headersToAdd map[string]string) {
-
-	assert.Equal(t, name, weightedCluster.Name)
-
-	assert.Equal(t, trafficPerc, weightedCluster.Weight.Value)
-
-	// Collect headers for easier comparison
-	clusterHeaders := make(map[string]string)
-	for _, header := range weightedCluster.RequestHeadersToAdd {
-		clusterHeaders[header.Header.Key] = header.Header.Value
-	}
-	assert.DeepEqual(t, clusterHeaders, headersToAdd)
+var lbEndpoints = []*endpoint.LbEndpoint{
+	envoy.NewLBEndpoint("2.2.2.2", 8080),
+	envoy.NewLBEndpoint("3.3.3.3", 8080),
+	envoy.NewLBEndpoint("4.4.4.4", 8080),
+	envoy.NewLBEndpoint("5.5.5.5", 8080),
 }
 
-func TestDomainsForRule(t *testing.T) {
-	domains := domainsForRule(v1alpha1.IngressRule{
-		Hosts: []string{
-			"helloworld-go.default.svc.cluster.local",
-			"helloworld-go.default.svc",
-			"helloworld-go.default",
+func secret(ns, name string) *corev1.Secret {
+	return &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: ns,
+			Name:      name,
+		},
+		Data: map[string][]byte{
+			"tls.crt": cert,
+			"tls.key": privateKey,
 		},
-	})
-
-	expected := []string{
-		"helloworld-go.default",
-		"helloworld-go.default:*",
-		"helloworld-go.default.svc",
-		"helloworld-go.default.svc:*",
-		"helloworld-go.default.svc.cluster.local",
-		"helloworld-go.default.svc.cluster.local:*",
 	}
-	sort.Strings(domains)
-	sort.Strings(expected)
-	assert.DeepEqual(t, domains, expected)
 }
+
+var (
+	cert       = []byte("cert")
+	privateKey = []byte("key")
+)