Repository: knative-sandbox/net-kourier
Pull request #824 (2022-04-13 13:02:00)

All Hops Encrypted: alpha Kourier support for encrypted for internal traffic
Fix https://github.com/knative-sandbox/net-kourier/issues/805

As described in https://github.com/knative-sandbox/net-kourier/issues/805, service to service traffic does not
support TLS encryption. `kourier-internal` service does not even open the port.

This patch fixes it. 

/cc @skonto @rhuss


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

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/net-kourier/pull/824#" title="Author self-approved">nak3</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/main/OWNERS)~~ [nak3]

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":[]} -->
nak3: @evankanderson https://github.com/knative/serving/issues/11906 did not mention about the internal traffic (`*.svc.cluster.local`) but our customer requested that "all" traffic so we need to add this. Please let us know if you think we should support it in `config-network` rather than Kourier specific.
skonto: @nak3 for the coverage reports I dont have access to evaluate what is wrong, obviously some files liek config.go could have more coverage. I left a comment in #802 
evankanderson: How does this work with Addressable resources? Will they refuse to connect without trusting the (self-signed?) CA for the `*.svc.cluster.local` address?

I'm concerned about unintended side effects of skipping this into the 1.4 build with only 3-4 business days of testing before the release. If we are going to do it, it should definitely be confined to `net-kourier` so that side effects can be mitigated by switching specific services to other ingress implementations.
skonto: @nak3 the coverage report is [here](https://codecov.io/gh/knative-sandbox/net-kourier/compare/483cd470dc95d03d63c9c853877d8df104d946e6...4a5366996df81b2524357458d1bb92bc14272dbe#).
The files marked with red are the offending ones, need better coverage. 
nak3: The coverage https://codecov.io/gh/knative-sandbox/net-kourier/compare/483cd470dc95d03d63c9c853877d8df104d946e6...4a5366996df81b2524357458d1bb92bc14272dbe# compared with non-related old commits. Just rebased and passed codeconv. 
skonto: /lgtm /hold for @evankanderson 

Patch:
diff --git a/config/200-config.yaml b/config/200-config.yaml
index fd5732f3..46bdc2fe 100644
--- a/config/200-config.yaml
+++ b/config/200-config.yaml
@@ -49,3 +49,12 @@ data:
     # across multiple layers of TCP proxies.
     # NOTE THAT THIS IS AN EXPERIMENTAL / ALPHA FEATURE
     enable-proxy-protocol: "false"
+
+    # The server certificates to serve the internal TLS traffic for Kourier Gateway.
+    # It is specified by the secret name in controller namespace, which has
+    # the "tls.crt" and "tls.key" data field.
+    # Use an empty value to disable the feature (default).
+    #
+    # NOTE: This flag is in an alpha state and is mostly here to enable internal testing
+    #       for now. Use with caution.
+    cluster-cert-secret: ""
diff --git a/config/300-gateway.yaml b/config/300-gateway.yaml
index 2f9f0886..c1fa96a5 100644
--- a/config/300-gateway.yaml
+++ b/config/300-gateway.yaml
@@ -135,6 +135,10 @@ spec:
       port: 80
       protocol: TCP
       targetPort: 8081
+    - name: https
+      port: 443
+      protocol: TCP
+      targetPort: 8444
   selector:
     app: 3scale-kourier-gateway
   type: ClusterIP
diff --git a/pkg/config/config.go b/pkg/config/config.go
index a45ba49a..b9cd8a0f 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -35,12 +35,19 @@ const (
 
 	// HTTPPortExternal is the port for external availability.
 	HTTPPortExternal = uint32(8080)
+
 	// HTTPPortInternal is the port for internal availability.
 	HTTPPortInternal = uint32(8081)
+
+	// HTTPSPortInternal is the port for internal HTTPS availability.
+	HTTPSPortInternal = uint32(8444)
+
 	// HTTPSPortExternal is the port for external HTTPS availability.
 	HTTPSPortExternal = uint32(8443)
+
 	// HTTPPortProb is the port for prob
 	HTTPPortProb = uint32(8090)
+
 	// HTTPSPortProb is the port for prob
 	HTTPSPortProb = uint32(9443)
 
diff --git a/pkg/config/configmap.go b/pkg/config/configmap.go
index 68314531..d39dc1ed 100644
--- a/pkg/config/configmap.go
+++ b/pkg/config/configmap.go
@@ -32,12 +32,16 @@ const (
 
 	// enableProxyProtocol is the config map key for enabling proxy protocol
 	enableProxyProtocol = "enable-proxy-protocol"
+
+	// clusterCert is the config map key for kourier internal certificates
+	clusterCert = "cluster-cert-secret"
 )
 
 func DefaultConfig() *Kourier {
 	return &Kourier{
 		EnableServiceAccessLogging: true, // true is the default for backwards-compat
 		EnableProxyProtocol:        false,
+		ClusterCertSecret:          "",
 	}
 }
 
@@ -48,6 +52,7 @@ func NewConfigFromMap(configMap map[string]string) (*Kourier, error) {
 	if err := cm.Parse(configMap,
 		cm.AsBool(enableServiceAccessLoggingKey, &nc.EnableServiceAccessLogging),
 		cm.AsBool(enableProxyProtocol, &nc.EnableProxyProtocol),
+		cm.AsString(clusterCert, &nc.ClusterCertSecret),
 	); err != nil {
 		return nil, err
 	}
@@ -68,4 +73,7 @@ type Kourier struct {
 	EnableServiceAccessLogging bool
 	// EnableProxyProtocol specifies whether proxy protocol feature is enabled
 	EnableProxyProtocol bool
+	// ClusterCertSecret specifies the secret name for the server certificates of
+	// Kourier Internal.
+	ClusterCertSecret string
 }
diff --git a/pkg/generator/caches.go b/pkg/generator/caches.go
index fe7f89ec..e3668e72 100644
--- a/pkg/generator/caches.go
+++ b/pkg/generator/caches.go
@@ -36,6 +36,7 @@ import (
 	"knative.dev/net-kourier/pkg/config"
 	envoy "knative.dev/net-kourier/pkg/envoy/api"
 	rconfig "knative.dev/net-kourier/pkg/reconciler/ingress/config"
+	"knative.dev/pkg/system"
 )
 
 const (
@@ -46,6 +47,7 @@ const (
 	externalRouteConfigName    = "external_services"
 	externalTLSRouteConfigName = "external_tls_services"
 	internalRouteConfigName    = "internal_services"
+	internalTLSRouteConfigName = "internal_tls_services"
 )
 
 // ErrDomainConflict is an error produces when two ingresses have conflicting domains.
@@ -241,6 +243,25 @@ func generateListenersAndRouteConfigs(
 	}
 	listeners = append(listeners, probHTTPListener)
 
+	// Add internal listeners and routes when internal cert secret is specified.
+	if cfg.Kourier.ClusterCertSecret != "" {
+		internalTLSRouteConfig := envoy.NewRouteConfig(internalTLSRouteConfigName, clusterLocalVirtualHosts)
+		internalTLSManager := envoy.NewHTTPConnectionManager(internalTLSRouteConfig.Name, cfg.Kourier.EnableServiceAccessLogging, cfg.Kourier.EnableProxyProtocol)
+
+		internalHTTPSEnvoyListener, err := newInternalEnvoyListenerWithOneCert(
+			ctx, internalTLSManager, kubeclient,
+			cfg.Kourier.EnableProxyProtocol,
+			cfg.Kourier.ClusterCertSecret,
+		)
+
+		if err != nil {
+			return nil, nil, err
+		}
+
+		listeners = append(listeners, internalHTTPSEnvoyListener)
+		routes = append(routes, internalTLSRouteConfig)
+	}
+
 	// Configure TLS Listener. If there's at least one ingress that contains the
 	// TLS field, that takes precedence. If there is not, TLS will be configured
 	// using a single cert for all the services if the creds are given via ENV.
@@ -336,3 +357,15 @@ func newExternalEnvoyListenerWithOneCert(ctx context.Context, manager *httpconnm
 
 	return envoy.NewHTTPSListener(config.HTTPSPortExternal, []*v3.FilterChain{filterChain}, enableProxyProtocol)
 }
+
+func newInternalEnvoyListenerWithOneCert(ctx context.Context, manager *httpconnmanagerv3.HttpConnectionManager, kubeClient kubeclient.Interface, enableProxyProtocol bool, certSecretName string) (*v3.Listener, error) {
+	certificateChain, privateKey, err := sslCreds(ctx, kubeClient, system.Namespace(), certSecretName)
+	if err != nil {
+		return nil, err
+	}
+	filterChain, err := envoy.CreateFilterChainFromCertificateAndPrivateKey(manager, certificateChain, privateKey)
+	if err != nil {
+		return nil, err
+	}
+	return envoy.NewHTTPSListener(config.HTTPSPortInternal, []*v3.FilterChain{filterChain}, enableProxyProtocol)
+}


Test patch:
diff --git a/pkg/config/configmap_test.go b/pkg/config/configmap_test.go
index 47ddff1d..588cae36 100644
--- a/pkg/config/configmap_test.go
+++ b/pkg/config/configmap_test.go
@@ -50,24 +50,28 @@ func TestKourierConfig(t *testing.T) {
 			enableServiceAccessLoggingKey: "foo",
 		},
 	}, {
-		name: "enable proxy protocol and logging",
+		name: "enable proxy protocol, logging and internal cert",
 		want: &Kourier{
 			EnableServiceAccessLogging: true,
 			EnableProxyProtocol:        true,
+			ClusterCertSecret:          "my-cert",
 		},
 		data: map[string]string{
 			enableServiceAccessLoggingKey: "true",
 			enableProxyProtocol:           "true",
+			clusterCert:                   "my-cert",
 		},
 	}, {
-		name: "enable proxy protocol and disable logging",
+		name: "enable proxy protocol and disable logging, empty internal cert",
 		want: &Kourier{
 			EnableServiceAccessLogging: false,
 			EnableProxyProtocol:        true,
+			ClusterCertSecret:          "",
 		},
 		data: map[string]string{
 			enableServiceAccessLoggingKey: "false",
 			enableProxyProtocol:           "true",
+			clusterCert:                   "",
 		},
 	}, {
 		name:    "not a bool for proxy protocol",
diff --git a/pkg/generator/caches_test.go b/pkg/generator/caches_test.go
index 9ac2a922..43b5427d 100644
--- a/pkg/generator/caches_test.go
+++ b/pkg/generator/caches_test.go
@@ -25,12 +25,17 @@ import (
 	listener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
 	route "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
 	"github.com/envoyproxy/go-control-plane/pkg/resource/v3"
+	"github.com/envoyproxy/go-control-plane/pkg/wellknown"
 	"google.golang.org/protobuf/testing/protocmp"
 	"gotest.tools/v3/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
 	"knative.dev/net-kourier/pkg/config"
 	envoy "knative.dev/net-kourier/pkg/envoy/api"
+	rconfig "knative.dev/net-kourier/pkg/reconciler/ingress/config"
+	network "knative.dev/networking/pkg"
 )
 
 func TestDeleteIngressInfo(t *testing.T) {
@@ -246,6 +251,52 @@ func TestTLSListenerWithEnvCertsSecret(t *testing.T) {
 	})
 }
 
+// TestTLSListenerWithInternalCertSecret verfies that
+// filter is added when secret name is specified by cluster-cert-secret.
+func TestTLSListenerWithInternalCertSecret(t *testing.T) {
+	testConfig := &rconfig.Config{
+		Network: &network.Config{},
+		Kourier: &config.Kourier{
+			ClusterCertSecret:   "test-ca",
+			EnableProxyProtocol: true,
+		},
+	}
+
+	internalSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-ca",
+		},
+		Data: map[string][]byte{
+			caDataName: cert,
+		},
+	}
+
+	kubeClient := fake.Clientset{}
+	cfg := testConfig.DeepCopy()
+	ctx := (&testConfigStore{config: cfg}).ToContext(context.Background())
+
+	_, err := kubeClient.CoreV1().Secrets("knative-serving").Create(ctx, internalSecret, metav1.CreateOptions{})
+	assert.NilError(t, err)
+
+	caches, err := NewCaches(ctx, &kubeClient, false)
+	assert.NilError(t, err)
+
+	t.Run("without SNI matches", func(t *testing.T) {
+		translatedIngress := &translatedIngress{
+			sniMatches: nil,
+		}
+		err := caches.addTranslatedIngress(translatedIngress)
+		assert.NilError(t, err)
+
+		snapshot, err := caches.ToEnvoySnapshot(ctx)
+		assert.NilError(t, err)
+
+		tlsListener := snapshot.GetResources(resource.ListenerType)[envoy.CreateListenerName(config.HTTPSPortInternal)].(*listener.Listener)
+		assert.Assert(t, len(tlsListener.ListenerFilters) == 1)
+		assert.Assert(t, (tlsListener.ListenerFilters[0]).Name == wellknown.ProxyProtocol)
+	})
+}
+
 // Creates an ingress translation and listeners from the given names an
 // associates them with the ingress name/namespace received.
 func createTestDataForIngress(