Skip to content

Commit

Permalink
All Hops Encrypted: Activator serving TLS (#12770)
Browse files Browse the repository at this point in the history
* TLS between Ingress and activator

* Separate github action

* Use fileExists func

* Use patch

* Drop todo

* Clean up

* Drop kind for tls

* Fix namespace

* Revert "Drop kind for tls"

This reverts commit 0256011.

* Fix filterSubsetPorts

* Use deepcopy

* Do not use deepcopy

* fix

* Fix review comments

* Use activatorCA string

* Add unit test for makeIngress

* Use all match for tls

* Drop map

* Use secret to specify activator server cert

* Update comments

* Fix review comment

* wip

* Add comment

* Add append to both

* Use sss.Ports[j]

* Fix script

* Use switch

* Fix format more readable

* Update variable names
  • Loading branch information
nak3 authored Apr 13, 2022
1 parent e75bcf0 commit 2de1474
Show file tree
Hide file tree
Showing 13 changed files with 365 additions and 28 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/kind-e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ jobs:

ingress:
- kourier
- kourier-tls
- istio
- contour
# Disabled due to consistent failures
Expand Down Expand Up @@ -132,6 +133,10 @@ jobs:
- ingress: istio
namespace-resources: virtualservices

- ingress: kourier-tls
ingress-class: kourier
enable-tls: 1

- test-suite: runtime
test-path: ./test/conformance/runtime/...

Expand All @@ -144,6 +149,7 @@ jobs:
env:
KIND: 1
INGRESS_CLASS: ${{ matrix.ingress-class || matrix.ingress }}.ingress.networking.knative.dev
ENABLE_TLS: ${{ matrix.enable-tls || 0 }}

steps:
- name: Set up Go 1.17.x
Expand Down
26 changes: 26 additions & 0 deletions cmd/activator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package main

import (
"context"
"crypto/tls"
"errors"
"fmt"
"log"
Expand Down Expand Up @@ -241,6 +242,31 @@ func main() {
}(name, server)
}

// Enable TLS server when activator-server-cert is specified.
// At this moment activator with TLS does not disable HTTP.
// See also https://github.com/knative/serving/issues/12808.
if networkConfig.ActivatorCertSecret != "" {
secret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, networkConfig.ActivatorCertSecret, metav1.GetOptions{})
if err != nil {
logger.Fatalw("failed to get secret", zap.Error(err))
}
cert, err := tls.X509KeyPair(secret.Data["tls.crt"], secret.Data["tls.key"])
if err != nil {
logger.Fatalw("failed to load certs", zap.Error(err))
}

// TODO: Implement the secret (certificate) rotation like knative.dev/pkg/webhook/certificates/.
// Also, the current activator must be restarted when updating the secret.
name, server := "https", pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah)
go func(name string, s *http.Server) {
s.TLSConfig = &tls.Config{Certificates: []tls.Certificate{cert}, MinVersion: tls.VersionTLS12}
// Don't forward ErrServerClosed as that indicates we're already shutting down.
if err := s.ListenAndServeTLS("", ""); err != nil && !errors.Is(err, http.ErrServerClosed) {
errCh <- fmt.Errorf("%s server failed: %w", name, err)
}
}(name, server)
}

// Wait for the signal to drain.
select {
case <-sigCtx.Done():
Expand Down
3 changes: 3 additions & 0 deletions config/core/deployments/activator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,7 @@ spec:
- name: http2
port: 81
targetPort: 8013
- name: https
port: 443
targetPort: 8112
type: ClusterIP
3 changes: 3 additions & 0 deletions pkg/networking/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ const (
// BackendHTTP2Port is the backend, i.e. `targetPort` that we setup for HTTP/2 services.
BackendHTTP2Port = 8013

// BackendHTTPSPort is the backend. i.e. `targetPort` that we setup for HTTPS services.
BackendHTTPSPort = 8112

// QueueAdminPort specifies the port number for
// health check and lifecycle hooks for queue-proxy.
QueueAdminPort = 8022
Expand Down
26 changes: 17 additions & 9 deletions pkg/reconciler/route/resources/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func makeIngressSpec(
rules := make([]netv1alpha1.IngressRule, 0, len(names))

featuresConfig := config.FromContextOrDefaults(ctx).Features
networkConfig := config.FromContextOrDefaults(ctx).Network

for _, name := range names {
visibilities := []netv1alpha1.IngressVisibility{netv1alpha1.IngressVisibilityClusterLocal}
Expand All @@ -148,7 +149,7 @@ func makeIngressSpec(
return netv1alpha1.IngressSpec{}, err
}
rule := makeIngressRule(domains, r.Namespace,
visibility, tc.Targets[name], ro.RolloutsByTag(name))
visibility, tc.Targets[name], ro.RolloutsByTag(name), networkConfig.ActivatorCA)
if featuresConfig.TagHeaderBasedRouting == apicfg.Enabled {
if rule.HTTP.Paths[0].AppendHeaders == nil {
rule.HTTP.Paths[0].AppendHeaders = make(map[string]string, 1)
Expand All @@ -170,7 +171,7 @@ func makeIngressSpec(
// Since names are sorted `DefaultTarget == ""` is the first one,
// so just pass the subslice.
rule.HTTP.Paths = append(
makeTagBasedRoutingIngressPaths(r.Namespace, tc, ro, names[1:]), rule.HTTP.Paths...)
makeTagBasedRoutingIngressPaths(r.Namespace, tc, ro, networkConfig.ActivatorCA, names[1:]), rule.HTTP.Paths...)
} else {
// If a request is routed by a tag-attached hostname instead of the tag header,
// the request may not have the tag header "Knative-Serving-Tag",
Expand Down Expand Up @@ -263,24 +264,25 @@ func MakeACMEIngressPaths(acmeChallenges []netv1alpha1.HTTP01Challenge, domains
func makeIngressRule(domains []string, ns string,
visibility netv1alpha1.IngressVisibility,
targets traffic.RevisionTargets,
roCfgs []*traffic.ConfigurationRollout) netv1alpha1.IngressRule {
roCfgs []*traffic.ConfigurationRollout,
activatorCA string) netv1alpha1.IngressRule {
return netv1alpha1.IngressRule{
Hosts: domains,
Visibility: visibility,
HTTP: &netv1alpha1.HTTPIngressRuleValue{
Paths: []netv1alpha1.HTTPIngressPath{
*makeBaseIngressPath(ns, targets, roCfgs),
*makeBaseIngressPath(ns, targets, roCfgs, activatorCA),
},
},
}
}

// `names` must not include `""` — the DefaultTarget.
func makeTagBasedRoutingIngressPaths(ns string, tc *traffic.Config, ro *traffic.Rollout, names []string) []netv1alpha1.HTTPIngressPath {
func makeTagBasedRoutingIngressPaths(ns string, tc *traffic.Config, ro *traffic.Rollout, activatorCA string, names []string) []netv1alpha1.HTTPIngressPath {
paths := make([]netv1alpha1.HTTPIngressPath, 0, len(names))

for _, name := range names {
path := makeBaseIngressPath(ns, tc.Targets[name], ro.RolloutsByTag(name))
path := makeBaseIngressPath(ns, tc.Targets[name], ro.RolloutsByTag(name), activatorCA)
path.Headers = map[string]netv1alpha1.HeaderMatch{network.TagHeaderName: {Exact: name}}
paths = append(paths, *path)
}
Expand All @@ -300,7 +302,7 @@ func rolloutConfig(cfgName string, ros []*traffic.ConfigurationRollout) *traffic
}

func makeBaseIngressPath(ns string, targets traffic.RevisionTargets,
roCfgs []*traffic.ConfigurationRollout) *netv1alpha1.HTTPIngressPath {
roCfgs []*traffic.ConfigurationRollout, activatorCA string) *netv1alpha1.HTTPIngressPath {
// Optimistically allocate |targets| elements.
splits := make([]netv1alpha1.IngressBackendSplit, 0, len(targets))
for _, t := range targets {
Expand All @@ -312,6 +314,12 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets,
if t.LatestRevision != nil && *t.LatestRevision {
cfg = rolloutConfig(t.ConfigurationName, roCfgs)
}
var servicePort intstr.IntOrString
if len(activatorCA) != 0 {
servicePort = intstr.FromInt(networking.ServiceHTTPSPort)
} else {
servicePort = intstr.FromInt(networking.ServicePort(t.Protocol))
}
if cfg == nil || len(cfg.Revisions) < 2 {
// No rollout in progress.
splits = append(splits, netv1alpha1.IngressBackendSplit{
Expand All @@ -320,7 +328,7 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets,
ServiceName: t.RevisionName,
// Port on the public service must match port on the activator.
// Otherwise, the serverless services can't guarantee seamless positive handoff.
ServicePort: intstr.FromInt(networking.ServicePort(t.Protocol)),
ServicePort: servicePort,
},
Percent: int(*t.Percent),
AppendHeaders: map[string]string{
Expand All @@ -337,7 +345,7 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets,
ServiceName: rev.RevisionName,
// Port on the public service must match port on the activator.
// Otherwise, the serverless services can't guarantee seamless positive handoff.
ServicePort: intstr.FromInt(networking.ServicePort(t.Protocol)),
ServicePort: servicePort,
},
Percent: rev.Percent,
AppendHeaders: map[string]string{
Expand Down
135 changes: 132 additions & 3 deletions pkg/reconciler/route/resources/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ func TestMakeIngressRuleVanilla(t *testing.T) {
}
ro := tc.BuildRollout()
rule := makeIngressRule(domains, ns,
netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget))
netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget), "" /* activatorCA */)
expected := netv1alpha1.IngressRule{
Hosts: []string{
"a.com",
Expand Down Expand Up @@ -920,7 +920,7 @@ func TestMakeIngressRuleZeroPercentTarget(t *testing.T) {
}
ro := tc.BuildRollout()
rule := makeIngressRule(domains, ns,
netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget))
netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget), "" /* activatorCA */)
expected := netv1alpha1.IngressRule{
Hosts: []string{"test.org"},
HTTP: &netv1alpha1.HTTPIngressRuleValue{
Expand Down Expand Up @@ -970,7 +970,7 @@ func TestMakeIngressRuleTwoTargets(t *testing.T) {
ro := tc.BuildRollout()
domains := []string{"test.org"}
rule := makeIngressRule(domains, ns, netv1alpha1.IngressVisibilityExternalIP,
targets, ro.RolloutsByTag("a-tag"))
targets, ro.RolloutsByTag("a-tag"), "" /* activatorCA */)
expected := netv1alpha1.IngressRule{
Hosts: []string{"test.org"},
HTTP: &netv1alpha1.HTTPIngressRuleValue{
Expand Down Expand Up @@ -1089,6 +1089,128 @@ func TestMakeIngressWithHTTPOption(t *testing.T) {
}
}

func TestMakeIngressWithActivatorCA(t *testing.T) {
targets := map[string]traffic.RevisionTargets{
traffic.DefaultTarget: {{
TrafficTarget: v1.TrafficTarget{
ConfigurationName: "config",
RevisionName: "v2",
Percent: ptr.Int64(100),
},
}},
"v1": {{
TrafficTarget: v1.TrafficTarget{
ConfigurationName: "config",
RevisionName: "v1",
Percent: ptr.Int64(100),
},
}},
}

r := Route(ns, "test-route", WithURL)

expected := []netv1alpha1.IngressRule{{
Hosts: []string{
"test-route." + ns,
"test-route." + ns + ".svc",
pkgnet.GetServiceHostname("test-route", ns),
},
HTTP: &netv1alpha1.HTTPIngressRuleValue{
Paths: []netv1alpha1.HTTPIngressPath{{
Splits: []netv1alpha1.IngressBackendSplit{{
IngressBackend: netv1alpha1.IngressBackend{
ServiceNamespace: ns,
ServiceName: "v2",
ServicePort: intstr.FromInt(networking.ServiceHTTPSPort),
},
Percent: 100,
AppendHeaders: map[string]string{
"Knative-Serving-Revision": "v2",
"Knative-Serving-Namespace": ns,
},
}},
}},
},
Visibility: netv1alpha1.IngressVisibilityClusterLocal,
}, {
Hosts: []string{
"test-route." + ns + ".example.com",
},
HTTP: &netv1alpha1.HTTPIngressRuleValue{
Paths: []netv1alpha1.HTTPIngressPath{{
Splits: []netv1alpha1.IngressBackendSplit{{
IngressBackend: netv1alpha1.IngressBackend{
ServiceNamespace: ns,
ServiceName: "v2",
ServicePort: intstr.FromInt(networking.ServiceHTTPSPort),
},
Percent: 100,
AppendHeaders: map[string]string{
"Knative-Serving-Revision": "v2",
"Knative-Serving-Namespace": ns,
},
}},
}},
},
Visibility: netv1alpha1.IngressVisibilityExternalIP,
}, {
Hosts: []string{
"v1-test-route." + ns,
"v1-test-route." + ns + ".svc",
pkgnet.GetServiceHostname("v1-test-route", ns),
},
HTTP: &netv1alpha1.HTTPIngressRuleValue{
Paths: []netv1alpha1.HTTPIngressPath{{
Splits: []netv1alpha1.IngressBackendSplit{{
IngressBackend: netv1alpha1.IngressBackend{
ServiceNamespace: ns,
ServiceName: "v1",
ServicePort: intstr.FromInt(networking.ServiceHTTPSPort),
},
Percent: 100,
AppendHeaders: map[string]string{
"Knative-Serving-Revision": "v1",
"Knative-Serving-Namespace": ns,
},
}},
}},
},
Visibility: netv1alpha1.IngressVisibilityClusterLocal,
}, {
Hosts: []string{
"v1-test-route." + ns + ".example.com",
},
HTTP: &netv1alpha1.HTTPIngressRuleValue{
Paths: []netv1alpha1.HTTPIngressPath{{
Splits: []netv1alpha1.IngressBackendSplit{{
IngressBackend: netv1alpha1.IngressBackend{
ServiceNamespace: ns,
ServiceName: "v1",
ServicePort: intstr.FromInt(networking.ServiceHTTPSPort),
},
Percent: 100,
AppendHeaders: map[string]string{
"Knative-Serving-Revision": "v1",
"Knative-Serving-Namespace": ns,
},
}},
}},
},
Visibility: netv1alpha1.IngressVisibilityExternalIP,
}}

tc := &traffic.Config{Targets: targets}
ro := tc.BuildRollout()
ci, err := makeIngressSpec(testContextWithActivatorCA(), r, nil /*tls*/, tc, ro)
if err != nil {
t.Error("Unexpected error", err)
}

if !cmp.Equal(expected, ci.Rules) {
t.Error("Unexpected rules (-want, +got):", cmp.Diff(expected, ci.Rules))
}
}

func TestMakeIngressTLS(t *testing.T) {
cert := &netv1alpha1.Certificate{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1300,3 +1422,10 @@ func testContextWithHTTPOption() context.Context {
cfg.Network.HTTPProtocol = network.HTTPRedirected
return config.ToContext(context.Background(), cfg)
}

func testContextWithActivatorCA() context.Context {
cfg := testConfig()
cfg.Network.ActivatorCA = "ca-ame"
cfg.Network.ActivatorSAN = "san-name"
return config.ToContext(context.Background(), cfg)
}
Loading

0 comments on commit 2de1474

Please sign in to comment.