Repository: knative-sandbox/net-kourier
Pull request #330 (2020-11-04 11:44:01)

Avoid certificate conversion to string from secret
A secret delivers `[]byte` and the API requires `[]byte`, so there is no reason to cast in between. Casting into a string is relatively costly and not necessary at all in this case as the value is opaque anyway.

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/330#" 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":[]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-sandbox-net-kourier-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/generator/generator.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative-sandbox_net-kourier/330/pull-knative-sandbox-net-kourier-go-coverage/1323954165025607680/artifacts/line-cov.html#file1) | 59.0% | 57.6% | -1.4

codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/net-kourier/pull/330?src=pr&el=h1) Report
> Merging [#330](https://codecov.io/gh/knative-sandbox/net-kourier/pull/330?src=pr&el=desc) into [master](https://codecov.io/gh/knative-sandbox/net-kourier/commit/0b3d2897a373bff8b4f9ce2ee811b0792bd00ca1?el=desc) will **decrease** coverage by `0.11%`.
> The diff coverage is `100.00%`.

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

```diff
@@            Coverage Diff             @@
##           master     #330      +/-   ##
==========================================
- Coverage   68.18%   68.07%   -0.12%     
==========================================
  Files          15       15              
  Lines         572      570       -2     
==========================================
- Hits          390      388       -2     
  Misses        154      154              
  Partials       28       28              
```


| [Impacted Files](https://codecov.io/gh/knative-sandbox/net-kourier/pull/330?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/envoy/api/listener.go](https://codecov.io/gh/knative-sandbox/net-kourier/pull/330/diff?src=pr&el=tree#diff-cGtnL2Vudm95L2FwaS9saXN0ZW5lci5nbw==) | `81.05% <100.00%> (ø)` | |
| [pkg/generator/generator.go](https://codecov.io/gh/knative-sandbox/net-kourier/pull/330/diff?src=pr&el=tree#diff-cGtnL2dlbmVyYXRvci9nZW5lcmF0b3IuZ28=) | `47.76% <100.00%> (-1.52%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/net-kourier/pull/330?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/330?src=pr&el=footer). Last update [0b3d289...9667d33](https://codecov.io/gh/knative-sandbox/net-kourier/pull/330?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/config/config.go b/pkg/config/config.go
index fabd5cb8..74335fcf 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -16,6 +16,13 @@ limitations under the License.
 
 package config
 
+import (
+	"os"
+
+	"knative.dev/pkg/network"
+	"knative.dev/pkg/system"
+)
+
 const (
 	ControllerName = "kourier"
 
@@ -34,3 +41,19 @@ const (
 
 	KourierIngressClassName = "kourier.ingress.networking.knative.dev"
 )
+
+// ServiceHostnames returns the external and internal service's respective hostname.
+//
+// Example: kourier.kourier-system.svc.cluster.local.
+func ServiceHostnames() (string, string) {
+	return network.GetServiceHostname(ExternalServiceName, GatewayNamespace()),
+		network.GetServiceHostname(InternalServiceName, GatewayNamespace())
+}
+
+func GatewayNamespace() string {
+	namespace := os.Getenv(GatewayNamespaceEnv)
+	if namespace == "" {
+		return system.Namespace()
+	}
+	return namespace
+}
diff --git a/pkg/envoy/api/listener.go b/pkg/envoy/api/listener.go
index 5f01beeb..ebd6e2b4 100644
--- a/pkg/envoy/api/listener.go
+++ b/pkg/envoy/api/listener.go
@@ -30,11 +30,11 @@ import (
 
 type SNIMatch struct {
 	hosts            []string
-	certificateChain string
-	privateKey       string
+	certificateChain []byte
+	privateKey       []byte
 }
 
-func NewSNIMatch(hosts []string, certificateChain string, privateKey string) SNIMatch {
+func NewSNIMatch(hosts []string, certificateChain []byte, privateKey []byte) SNIMatch {
 	return SNIMatch{
 		hosts:            hosts,
 		certificateChain: certificateChain,
@@ -63,8 +63,8 @@ func NewHTTPListener(manager *httpconnmanagerv2.HttpConnectionManager, port uint
 
 func NewHTTPSListener(manager *httpconnmanagerv2.HttpConnectionManager,
 	port uint32,
-	certificateChain string,
-	privateKey string) (*v2.Listener, error) {
+	certificateChain []byte,
+	privateKey []byte) (*v2.Listener, error) {
 
 	filters, err := createFilters(manager)
 	if err != nil {
@@ -184,16 +184,16 @@ func createFilterChainsForTLS(manager *httpconnmanagerv2.HttpConnectionManager,
 	return res, nil
 }
 
-func createTLSContext(certificate string, privateKey string) *auth.DownstreamTlsContext {
+func createTLSContext(certificate []byte, privateKey []byte) *auth.DownstreamTlsContext {
 	return &auth.DownstreamTlsContext{
 		CommonTlsContext: &auth.CommonTlsContext{
 			TlsCertificates: []*auth.TlsCertificate{
 				{
 					CertificateChain: &core.DataSource{
-						Specifier: &core.DataSource_InlineBytes{InlineBytes: []byte(certificate)},
+						Specifier: &core.DataSource_InlineBytes{InlineBytes: certificate},
 					},
 					PrivateKey: &core.DataSource{
-						Specifier: &core.DataSource_InlineBytes{InlineBytes: []byte(privateKey)},
+						Specifier: &core.DataSource_InlineBytes{InlineBytes: privateKey},
 					},
 				},
 			},
diff --git a/pkg/generator/generator.go b/pkg/generator/generator.go
index f27ece89..2f099128 100644
--- a/pkg/generator/generator.go
+++ b/pkg/generator/generator.go
@@ -147,24 +147,19 @@ func useHTTPSListenerWithOneCert() bool {
 		os.Getenv(envCertsSecretName) != ""
 }
 
-func sslCreds(ctx context.Context, kubeClient kubeclient.Interface, secretNamespace string, secretName string) (certificateChain string, privateKey string, err error) {
+func sslCreds(ctx context.Context, kubeClient kubeclient.Interface, secretNamespace string, secretName string) (certificateChain []byte, privateKey []byte, err error) {
 	secret, err := kubeClient.CoreV1().Secrets(secretNamespace).Get(ctx, secretName, metav1.GetOptions{})
-
 	if err != nil {
-		return "", "", err
+		return nil, nil, err
 	}
 
-	certificateChain = string(secret.Data[certFieldInSecret])
-	privateKey = string(secret.Data[keyFieldInSecret])
-
-	return certificateChain, privateKey, nil
+	return secret.Data[certFieldInSecret], secret.Data[keyFieldInSecret], nil
 }
 
 func newExternalEnvoyListenerWithOneCert(ctx context.Context, manager *httpconnmanagerv2.HttpConnectionManager, kubeClient kubeclient.Interface) (*v2.Listener, error) {
 	certificateChain, privateKey, err := sslCreds(
 		ctx, kubeClient, os.Getenv(envCertsSecretNamespace), os.Getenv(envCertsSecretName),
 	)
-
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/generator/ingress_translator.go b/pkg/generator/ingress_translator.go
index 1d76957c..aa97d6fb 100644
--- a/pkg/generator/ingress_translator.go
+++ b/pkg/generator/ingress_translator.go
@@ -29,7 +29,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	kubeclient "k8s.io/client-go/kubernetes"
 	envoy "knative.dev/net-kourier/pkg/envoy/api"
-	"knative.dev/net-kourier/pkg/knative"
 	"knative.dev/networking/pkg/apis/networking/v1alpha1"
 	"knative.dev/pkg/kmeta"
 	"knative.dev/pkg/logging"
@@ -155,20 +154,19 @@ func (translator *IngressTranslator) translateIngress(ctx context.Context, ingre
 			return nil, nil
 		}
 
-		domains := knative.Domains(rule)
 		var virtualHost *route.VirtualHost
 		if extAuthzEnabled {
 			contextExtensions := kmeta.UnionMaps(map[string]string{
 				"client":     "kourier",
 				"visibility": string(rule.Visibility),
 			}, ingress.GetLabels())
-			virtualHost = envoy.NewVirtualHostWithExtAuthz(ingress.Name, contextExtensions, domains, routes)
+			virtualHost = envoy.NewVirtualHostWithExtAuthz(ingress.Name, contextExtensions, domainsForRule(rule), routes)
 		} else {
-			virtualHost = envoy.NewVirtualHost(ingress.Name, domains, routes)
+			virtualHost = envoy.NewVirtualHost(ingress.Name, domainsForRule(rule), routes)
 		}
 
 		internalHosts = append(internalHosts, virtualHost)
-		if knative.RuleIsExternal(rule) {
+		if rule.Visibility == v1alpha1.IngressVisibilityExternalIP {
 			externalHosts = append(externalHosts, virtualHost)
 		}
 	}
@@ -255,3 +253,21 @@ func sniMatchFromIngressTLS(ctx context.Context, ingressTLS v1alpha1.IngressTLS,
 	sniMatch := envoy.NewSNIMatch(ingressTLS.Hosts, certChain, privateKey)
 	return &sniMatch, nil
 }
+
+// domainsForRule returns all domains for the given rule.
+//
+// For example, external domains returns domains with the following formats:
+// 	- sub-route_host.namespace.example.com
+// 	- sub-route_host.namespace.example.com:*
+//
+// Somehow envoy doesn't match properly gRPC authorities with ports.
+// The fix is to include ":*" in the domains.
+// This applies both for internal and external domains.
+// More info https://github.com/envoyproxy/envoy/issues/886
+func domainsForRule(rule v1alpha1.IngressRule) []string {
+	domains := make([]string, 0, 2*len(rule.Hosts))
+	for _, host := range rule.Hosts {
+		domains = append(domains, host, host+":*")
+	}
+	return domains
+}
diff --git a/pkg/knative/ingress.go b/pkg/knative/ingress.go
deleted file mode 100644
index bdde7296..00000000
--- a/pkg/knative/ingress.go
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
-Copyright 2020 The Knative Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package knative
-
-import (
-	"os"
-
-	"knative.dev/net-kourier/pkg/config"
-	networkingv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
-	"knative.dev/pkg/network"
-	"knative.dev/pkg/system"
-)
-
-func MarkIngressReady(ingress *networkingv1alpha1.Ingress) {
-	internalDomain := domainForServiceName(config.InternalServiceName)
-	externalDomain := domainForServiceName(config.ExternalServiceName)
-
-	ingress.Status.MarkLoadBalancerReady(
-		[]networkingv1alpha1.LoadBalancerIngressStatus{{
-			DomainInternal: externalDomain,
-		}},
-		[]networkingv1alpha1.LoadBalancerIngressStatus{{
-			DomainInternal: internalDomain,
-		}},
-	)
-	ingress.Status.MarkNetworkConfigured()
-}
-
-func domainForServiceName(serviceName string) string {
-	return serviceName + "." + GetGatewayNamespace() + ".svc." + network.GetClusterDomainName()
-}
-
-func GetGatewayNamespace() string {
-	namespace := os.Getenv(config.GatewayNamespaceEnv)
-	if namespace == "" {
-		return system.Namespace()
-	}
-	return namespace
-}
diff --git a/pkg/knative/ingress_rule.go b/pkg/knative/ingress_rule.go
deleted file mode 100644
index b4679485..00000000
--- a/pkg/knative/ingress_rule.go
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
-Copyright 2020 The Knative Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package knative
-
-import (
-	"knative.dev/networking/pkg/apis/networking/v1alpha1"
-)
-
-// Domains returns domains.
-//
-// For example, external domains returns domains with the following formats:
-// 	- sub-route_host.namespace.example.com
-// 	- sub-route_host.namespace.example.com:*
-//
-// Somehow envoy doesn't match properly gRPC authorities with ports.
-// The fix is to include ":*" in the domains.
-// This applies both for internal and external domains.
-// More info https://github.com/envoyproxy/envoy/issues/886
-//
-func Domains(rule v1alpha1.IngressRule) []string {
-	domains := make([]string, 0, 2*len(rule.Hosts))
-	for _, host := range rule.Hosts {
-		domains = append(domains, host, host+":*")
-	}
-	return domains
-}
-
-func RuleIsExternal(rule v1alpha1.IngressRule) bool {
-	return rule.Visibility == v1alpha1.IngressVisibilityExternalIP
-}
diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go
index 43c05ff2..72b2a5d6 100644
--- a/pkg/reconciler/ingress/ingress.go
+++ b/pkg/reconciler/ingress/ingress.go
@@ -25,7 +25,6 @@ import (
 	"knative.dev/net-kourier/pkg/config"
 	envoy "knative.dev/net-kourier/pkg/envoy/server"
 	"knative.dev/net-kourier/pkg/generator"
-	"knative.dev/net-kourier/pkg/knative"
 	"knative.dev/networking/pkg/apis/networking/v1alpha1"
 	"knative.dev/networking/pkg/client/injection/reconciler/networking/v1alpha1/ingress"
 	"knative.dev/networking/pkg/status"
@@ -57,7 +56,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, ing *v1alpha1.Ingress) r
 			return fmt.Errorf("failed to probe Ingress %s/%s: %w", ing.GetNamespace(), ing.GetName(), err)
 		}
 		if ready {
-			knative.MarkIngressReady(ing)
+			external, internal := config.ServiceHostnames()
+			ing.Status.MarkLoadBalancerReady(
+				[]v1alpha1.LoadBalancerIngressStatus{{DomainInternal: external}},
+				[]v1alpha1.LoadBalancerIngressStatus{{DomainInternal: internal}},
+			)
+			ing.Status.MarkNetworkConfigured()
 		} else {
 			ing.Status.MarkLoadBalancerNotReady()
 		}
diff --git a/pkg/reconciler/ingress/lister.go b/pkg/reconciler/ingress/lister.go
index be779562..4dbdc33e 100644
--- a/pkg/reconciler/ingress/lister.go
+++ b/pkg/reconciler/ingress/lister.go
@@ -26,7 +26,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	corev1listers "k8s.io/client-go/listers/core/v1"
 	"knative.dev/net-kourier/pkg/config"
-	"knative.dev/net-kourier/pkg/knative"
 	"knative.dev/networking/pkg/apis/networking/v1alpha1"
 	"knative.dev/networking/pkg/status"
 )
@@ -45,7 +44,7 @@ type gatewayPodTargetLister struct {
 
 func (l *gatewayPodTargetLister) ListProbeTargets(ctx context.Context, ing *v1alpha1.Ingress) ([]status.ProbeTarget, error) {
 
-	eps, err := l.endpointsLister.Endpoints(knative.GetGatewayNamespace()).Get(config.InternalServiceName)
+	eps, err := l.endpointsLister.Endpoints(config.GatewayNamespace()).Get(config.InternalServiceName)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get internal service: %w", err)
 	}
@@ -72,7 +71,7 @@ func (l *gatewayPodTargetLister) getIngressUrls(ing *v1alpha1.Ingress, gatewayIp
 		domains := rule.Hosts
 		scheme := "http"
 
-		if knative.RuleIsExternal(rule) {
+		if rule.Visibility == v1alpha1.IngressVisibilityExternalIP {
 			target = status.ProbeTarget{
 				PodIPs: ips,
 			}


Test patch:
diff --git a/pkg/envoy/api/listener_test.go b/pkg/envoy/api/listener_test.go
index 60365502..9b7e5574 100644
--- a/pkg/envoy/api/listener_test.go
+++ b/pkg/envoy/api/listener_test.go
@@ -50,7 +50,10 @@ func TestNewHTTPListener(t *testing.T) {
 func TestNewHTTPSListener(t *testing.T) {
 	manager := NewHTTPConnectionManager([]*route.VirtualHost{})
 
-	l, err := NewHTTPSListener(&manager, 8081, "some_certificate_chain", "some_private_key")
+	certChain := []byte("some_certificate_chain")
+	privateKey := []byte("some_private_key")
+
+	l, err := NewHTTPSListener(&manager, 8081, certChain, privateKey)
 	if err != nil {
 		t.Error(err)
 	}
@@ -60,23 +63,23 @@ func TestNewHTTPSListener(t *testing.T) {
 	assert.Equal(t, uint32(8081), l.Address.GetSocketAddress().GetPortValue())
 
 	// Check that TLS is configured
-	certChain, privateKey, err := getTLSCreds(l.FilterChains[0])
+	gotCertChain, gotPrivateKey, err := getTLSCreds(l.FilterChains[0])
 	assert.NilError(t, err)
-	assert.Equal(t, "some_certificate_chain", certChain)
-	assert.Equal(t, "some_private_key", privateKey)
+	assert.DeepEqual(t, certChain, gotCertChain)
+	assert.DeepEqual(t, privateKey, gotPrivateKey)
 }
 
 func TestNewHTTPSListenerWithSNI(t *testing.T) {
 	sniMatches := []*SNIMatch{
 		{
 			hosts:            []string{"some_host.com"},
-			certificateChain: "cert1",
-			privateKey:       "key1",
+			certificateChain: []byte("cert1"),
+			privateKey:       []byte("key1"),
 		},
 		{
 			hosts:            []string{"another_host.com"},
-			certificateChain: "cert2",
-			privateKey:       "key2",
+			certificateChain: []byte("cert2"),
+			privateKey:       []byte("key2"),
 		},
 	}
 
@@ -116,8 +119,8 @@ func assertListenerHasSNIMatchConfigured(t *testing.T, listener *envoy_api_v2.Li
 
 	certChain, privateKey, err := getTLSCreds(filterChainFirstSNIMatch)
 	assert.NilError(t, err)
-	assert.Equal(t, match.certificateChain, certChain)
-	assert.Equal(t, match.privateKey, privateKey)
+	assert.DeepEqual(t, match.certificateChain, certChain)
+	assert.DeepEqual(t, match.privateKey, privateKey)
 
 	vHostsDomains, err := getVHostDomains(filterChainFirstSNIMatch)
 	assert.NilError(t, err)
@@ -137,22 +140,22 @@ func getFilterChainByServerName(listener *envoy_api_v2.Listener, serverNames []s
 }
 
 // Note: Returns an error when there are multiple certificates
-func getTLSCreds(filterChain *envoy_api_v2_listener.FilterChain) (certChain string, privateKey string, err error) {
+func getTLSCreds(filterChain *envoy_api_v2_listener.FilterChain) (certChain []byte, privateKey []byte, err error) {
 	downstreamTLSContext := &auth.DownstreamTlsContext{}
 	err = ptypes.UnmarshalAny(
 		filterChain.GetTransportSocket().GetTypedConfig(), downstreamTLSContext,
 	)
 	if err != nil {
-		return "", "", err
+		return nil, nil, err
 	}
 
 	if len(downstreamTLSContext.CommonTlsContext.TlsCertificates) > 1 {
-		return "", "", fmt.Errorf("more than one certificate configured")
+		return nil, nil, fmt.Errorf("more than one certificate configured")
 	}
 
 	certs := downstreamTLSContext.CommonTlsContext.TlsCertificates[0]
-	certChain = string(certs.CertificateChain.GetInlineBytes())
-	privateKey = string(certs.PrivateKey.GetInlineBytes())
+	certChain = certs.CertificateChain.GetInlineBytes()
+	privateKey = certs.PrivateKey.GetInlineBytes()
 
 	return certChain, privateKey, nil
 }
diff --git a/pkg/generator/ingress_translator_test.go b/pkg/generator/ingress_translator_test.go
index 18d70a3b..a1f3d7cb 100644
--- a/pkg/generator/ingress_translator_test.go
+++ b/pkg/generator/ingress_translator_test.go
@@ -229,8 +229,8 @@ func TestIngressWithTLS(t *testing.T) {
 	tlsSecretName := "tls-secret"
 	tlsSecretNamespace := "default"
 	tlsHosts := []string{"hello-world.example.com"}
-	tlsCert := "some-cert"
-	tlsKey := "tls-key"
+	tlsCert := []byte("some-cert")
+	tlsKey := []byte("tls-key")
 	svcNamespace := "default"
 	ctx := context.Background()
 
@@ -377,14 +377,14 @@ func createIngressWithTLS(hosts []string, secretName string, secretNamespace str
 
 }
 
-func createSecret(ctx context.Context, kubeClient kubernetes.Interface, secretNamespace string, secretName string, cert string, key string) error {
+func createSecret(ctx context.Context, kubeClient kubernetes.Interface, secretNamespace string, secretName string, cert []byte, key []byte) error {
 	tlsSecret := corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: secretName,
 		},
 		Data: map[string][]byte{
-			certFieldInSecret: []byte(cert),
-			keyFieldInSecret:  []byte(key),
+			certFieldInSecret: cert,
+			keyFieldInSecret:  key,
 		},
 	}
 
@@ -446,3 +446,25 @@ func assertWeightedClusterCorrect(t *testing.T,
 	}
 	assert.DeepEqual(t, clusterHeaders, headersToAdd)
 }
+
+func TestDomainsForRule(t *testing.T) {
+	domains := domainsForRule(v1alpha1.IngressRule{
+		Hosts: []string{
+			"helloworld-go.default.svc.cluster.local",
+			"helloworld-go.default.svc",
+			"helloworld-go.default",
+		},
+	})
+
+	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)
+}
diff --git a/pkg/knative/ingress_rule_test.go b/pkg/knative/ingress_rule_test.go
deleted file mode 100644
index 0a448b93..00000000
--- a/pkg/knative/ingress_rule_test.go
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
-Copyright 2020 The Knative Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package knative
-
-import (
-	"sort"
-	"testing"
-
-	"gotest.tools/assert"
-	"knative.dev/networking/pkg/apis/networking/v1alpha1"
-)
-
-var testRule = v1alpha1.IngressRule{
-	Hosts: []string{
-		"helloworld-go.default.svc.cluster.local",
-		"helloworld-go.default.svc",
-		"helloworld-go.default",
-	},
-}
-
-func TestDomains(t *testing.T) {
-	domains := Domains(testRule)
-
-	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)
-}
-
-func TestRuleIsExternalWithVisibility(t *testing.T) {
-	externalRule := v1alpha1.IngressRule{
-		Visibility: v1alpha1.IngressVisibilityExternalIP,
-	}
-	internalRule := v1alpha1.IngressRule{
-		Visibility: v1alpha1.IngressVisibilityClusterLocal,
-	}
-
-	assert.Equal(t, RuleIsExternal(externalRule), true)
-	assert.Equal(t, RuleIsExternal(internalRule), false)
-}