Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All Hops Encrypted: alpha Kourier support for encrypted for internal traffic #824

Merged
merged 4 commits into from
Apr 15, 2022
Merged

All Hops Encrypted: alpha Kourier support for encrypted for internal traffic #824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
574aa0d
All Hops Encrypted: alpha Kourier support for encrypted for internal …
nak3 Apr 1, 2022
723a8ea
Rename configuration name
nak3 Apr 15, 2022
ee6baf1
Fix review comments
nak3 Apr 15, 2022
b532a02
Fix lint
nak3 Apr 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions config/200-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""
4 changes: 4 additions & 0 deletions config/300-gateway.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
8 changes: 8 additions & 0 deletions pkg/config/configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
}
}

Expand All @@ -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
}
Expand All @@ -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
}
8 changes: 6 additions & 2 deletions pkg/config/configmap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
33 changes: 33 additions & 0 deletions pkg/generator/caches.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Copy link
Contributor

@skonto skonto Apr 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only diff between internal and external for newXXXEnvoyListenerWithOneCertFilterChain is the namespace and secretaName pair?

os.Getenv(envCertsSecretNamespace) , os.Getenv(envCertsSecretName)
vs
system.Namespace(), certSecretName,

Should we simplify this by merging newInternalEnvoyListenerWithOneCertFilterChain and newInternalEnvoyListenerWithOneCert to one func and then just the appropriate values for both internal/external cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, rather than making the cert name configurable, you could hard code it, and use the presence / absence of the cert to activate the encryption on the internal interface.

In my experience, this can work fairly well if the component is installed in its own namespace and isn't expected to share. In that case, you could default the secret name to e.g. kourier-internal-domain-cert or the like, and then document that if that cert is created, Kourier will use it.

Copy link
Contributor Author

@nak3 nak3 Apr 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard cord name is fine, but I assumed that the future plan as:

  1. Create the secret(=certs) automatically by cert-manager or the library in knative.dev/pkg.
  2. Knative always use TLS traffic with the auto generated certs.

At step-2, cluster-cert-secrete in the ConfigMap starts having the default value like:

kind: ConfigMap
  name: config-kourier
    cluster-cert-secret: "kourier-internal-domain-cert"

And if users want to use their own certs rather than auto-gen certs, they can set the different value in cluster-cert-secret.

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)
}
51 changes: 51 additions & 0 deletions pkg/generator/caches_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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(
Expand Down