Skip to content

Commit

Permalink
internal encryption e2e tests (#14092)
Browse files Browse the repository at this point in the history
* allow activator configstore to track network config

* add new security mode metrics tag

* activator now adds security mode tag to metrics

* based on security mode from config in context

* queue adds security mode tag to metrics

* security mode set as env var based on config from reconciler

* test: add test image for reading request metrics

* test: add internal encryption e2e tests

* run update-deps

* first pass: change internal encryption test to read logs

* remove metricreader image

* update internal encryption test readme

* run update-deps

* remove securitymode metric

* address linter suggestions

* more lint suggestions

* clean up test

* make internal encryption test alpha only

* also only run for Contour and Kourier right now

* cleanup leftovers from metrics stuff

* address nits

* fix typo

* address PR feedback

* fix failing test

* use correct symbol when looking for TLS access log

* refactor toggle_feature

* was having some bash issues with properly interpretting the patch

* use new flag name

* we have switched to system-internal-tls
  • Loading branch information
KauzClay authored Oct 6, 2023
1 parent 3eb979a commit c183543
Show file tree
Hide file tree
Showing 7 changed files with 228 additions and 23 deletions.
20 changes: 17 additions & 3 deletions pkg/activator/config/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"

"go.uber.org/atomic"
netcfg "knative.dev/networking/pkg/config"
"knative.dev/pkg/configmap"
tracingconfig "knative.dev/pkg/tracing/config"
)
Expand All @@ -29,6 +30,7 @@ type cfgKey struct{}
// Config is the configuration for the activator.
type Config struct {
Tracing *tracingconfig.Config
Network *netcfg.Config
}

// FromContext obtains a Config injected into the passed context.
Expand All @@ -51,15 +53,27 @@ func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value i
// Append an update function to run after a ConfigMap has updated to update the
// current state of the Config.
onAfterStore = append(onAfterStore, func(_ string, _ interface{}) {
s.current.Store(&Config{
Tracing: s.UntypedLoad(tracingconfig.ConfigName).(*tracingconfig.Config).DeepCopy(),
})
c := &Config{}
tracing := s.UntypedLoad(tracingconfig.ConfigName)
if tracing != nil {
c.Tracing = tracing.(*tracingconfig.Config).DeepCopy()
}
// this allows dynamic updating of the config-network
// this is necessary for not requiring activator restart for system-internal-tls in the future
// however, current implementation is not there yet.
// see https://github.com/knative/serving/issues/13754
network := s.UntypedLoad(netcfg.ConfigMapName)
if network != nil {
c.Network = network.(*netcfg.Config).DeepCopy()
}
s.current.Store(c)
})
s.UntypedStore = configmap.NewUntypedStore(
"activator",
logger,
configmap.Constructors{
tracingconfig.ConfigName: tracingconfig.NewTracingConfigFromConfigMap,
netcfg.ConfigMapName: netcfg.NewConfigFromConfigMap,
},
onAfterStore...,
)
Expand Down
14 changes: 14 additions & 0 deletions pkg/activator/config/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
netcfg "knative.dev/networking/pkg/config"
ltesting "knative.dev/pkg/logging/testing"
tracingconfig "knative.dev/pkg/tracing/config"
)
Expand All @@ -35,17 +36,30 @@ var tracingConfig = &corev1.ConfigMap{
},
}

var networkingConfig = &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: netcfg.ConfigMapName,
},
Data: map[string]string{
"ingress-class": "random.ingress.networking.knative.dev",
},
}

func TestStore(t *testing.T) {
logger := ltesting.TestLogger(t)
store := NewStore(logger)
store.OnConfigChanged(tracingConfig)
store.OnConfigChanged(networkingConfig)

ctx := store.ToContext(context.Background())
cfg := FromContext(ctx)

if got, want := cfg.Tracing.Backend, tracingconfig.None; got != want {
t.Fatalf("Tracing.Backend = %v, want %v", got, want)
}
if got, want := cfg.Network.DefaultIngressClass, "random.ingress.networking.knative.dev"; got != want {
t.Fatalf("Networking.In = %v, want %v", got, want)
}

newConfig := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Expand Down
10 changes: 9 additions & 1 deletion test/e2e-common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -453,12 +453,20 @@ function wait_for_leader_controller() {
return 1
}

function restart_pod() {
local namespace="$1"
local label="$2"
echo -n "Deleting pod in ${namespace} with label ${label}"
kubectl -n ${namespace} delete pod -l ${label}
}

function toggle_feature() {
local FEATURE="$1"
local STATE="$2"
local CONFIG="${3:-config-features}"
echo -n "Setting feature ${FEATURE} to ${STATE}"
kubectl patch cm "${CONFIG}" -n "${SYSTEM_NAMESPACE}" -p '{"data":{"'${FEATURE}'":"'${STATE}'"}}'
local PATCH="{\"data\":{\"${FEATURE}\":\"${STATE}\"}}"
kubectl patch cm "${CONFIG}" -n "${SYSTEM_NAMESPACE}" -p "${PATCH}"
# We don't have a good mechanism for positive handoff so sleep :(
echo "Waiting 30s for change to get picked up."
sleep 30
Expand Down
19 changes: 0 additions & 19 deletions test/e2e-internal-encryption-tests.sh

This file was deleted.

12 changes: 12 additions & 0 deletions test/e2e-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@ toggle_feature allow-zero-initial-scale false config-autoscaler || fail_test

go_test_e2e -timeout=2m ./test/e2e/domainmapping ${E2E_TEST_FLAGS} || failed=1

toggle_feature system-internal-tls enabled config-network || fail_test
toggle_feature "logging.enable-request-log" true config-observability || fail_test
toggle_feature "logging.request-log-template" "TLS: {{.Request.TLS}}" config-observability || fail_test
# with current implementation, Activator must be restarted when configuring system-internal-tls. See https://github.com/knative/serving/issues/13754
restart_pod ${SYSTEM_NAMESPACE} "app=activator"
go_test_e2e -timeout=2m ./test/e2e/systeminternaltls ${E2E_TEST_FLAGS} || failed=1
toggle_feature system-internal-tls disabled config-network || fail_test
toggle_feature enable-request-log false config-observability || fail_test
toggle_feature request-log-template '' config-observability || fail_test
# with the current implementation, Activator is always in the request path, and needs to be restarted after configuring system-internal-tls
restart_pod ${SYSTEM_NAMESPACE} "app=activator"

kubectl get cm "config-gc" -n "${SYSTEM_NAMESPACE}" -o yaml > "${TMP_DIR}"/config-gc.yaml
add_trap "kubectl replace cm 'config-gc' -n ${SYSTEM_NAMESPACE} -f ${TMP_DIR}/config-gc.yaml" SIGKILL SIGTERM SIGQUIT
immediate_gc
Expand Down
11 changes: 11 additions & 0 deletions test/e2e/systeminternaltls/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# System Internal TLS E2E Tests

In order to test System Internal TLS, this test turns enables request logging and sets the request log template to `TLS: {{.Request.TLS}}`.

The test setup will enable System Internal TLS, and then configure the logging settings.

The test then deploys and attempts to reach the HelloWorld test image.

Assuming the request succeeds, the test combs the logs for the Activator and QueueProxy looking for the TLS lines.

It counts the lines where `TLS: <nil>` appears, which indicates that TLS was not used. If that count is greater than 0, the test will fail.
165 changes: 165 additions & 0 deletions test/e2e/systeminternaltls/system_internal_tls_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
//go:build e2e
// +build e2e

/*
Copyright 2023 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 systeminternaltls

import (
"bufio"
"bytes"
"context"
"fmt"
"io"
"strings"
"testing"

corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/rest"
"knative.dev/pkg/system"
pkgTest "knative.dev/pkg/test"
"knative.dev/pkg/test/spoof"
"knative.dev/serving/test"
v1test "knative.dev/serving/test/v1"
)

// TestInternalEncrytion tests the TLS connections between system components.
func TestInternalEncryption(t *testing.T) {
if !test.ServingFlags.EnableAlphaFeatures {
t.Skip("Alpha features not enabled")
}

if !(strings.Contains(test.ServingFlags.IngressClass, "kourier") || strings.Contains(test.ServingFlags.IngressClass, "contour")) {
t.Skip("Skip this test for non-kourier or non-contour ingress.")
}

t.Parallel()
clients := test.Setup(t)

names := test.ResourceNames{
Service: test.ObjectNameForTest(t),
Image: test.HelloWorld,
}

test.EnsureTearDown(t, clients, &names)

t.Log("Creating a new Service")
resources, err := v1test.CreateServiceReady(t, clients, &names)
if err != nil {
t.Fatalf("Failed to create initial Service: %v: %v", names.Service, err)
}

//The request made here should be enough to trigger some request logs on the Activator and QueueProxy
t.Log("Checking Endpoint state")
url := resources.Route.Status.URL.URL()
if _, err := pkgTest.CheckEndpointState(
context.Background(),
clients.KubeClient,
t.Logf,
url,
spoof.MatchesAllOf(spoof.IsStatusOK, spoof.MatchesBody(test.HelloWorldText)),
"HelloWorldText",
test.ServingFlags.ResolvableDomain,
test.AddRootCAtoTransport(context.Background(), t.Logf, clients, test.ServingFlags.HTTPS),
); err != nil {
t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.HelloWorldText, err)
}

t.Log("Checking Activator logs")
pods, err := clients.KubeClient.CoreV1().Pods(system.Namespace()).List(context.TODO(), v1.ListOptions{
LabelSelector: "app=activator",
})
if err != nil {
t.Fatalf("Failed to get pods: %v", err)
}
if len(pods.Items) == 0 {
t.Fatalf("No pods detected for activator: %v", err)
}
activatorPod := pods.Items[0]

req := clients.KubeClient.CoreV1().Pods(activatorPod.Namespace).GetLogs(activatorPod.Name, &corev1.PodLogOptions{})
activatorTLSCount, err := scanPodLogs(req, matchTLSLog)

if err != nil {
t.Fatalf("Failed checking activator logs: %s", err)
} else if activatorTLSCount == 0 {
t.Fatal("TLS not used on requests to activator")
}

t.Log("Checking Queue-Proxy logs")
pods, err = clients.KubeClient.CoreV1().Pods("serving-tests").List(context.TODO(), v1.ListOptions{
LabelSelector: fmt.Sprintf("serving.knative.dev/configuration=%s", names.Config),
})
if err != nil {
t.Fatalf("Failed to get pods: %v", err)
}
if len(pods.Items) == 0 {
t.Fatalf("No pods detected for test app: %v", err)
}
helloWorldPod := pods.Items[0]
req = clients.KubeClient.CoreV1().Pods(helloWorldPod.Namespace).GetLogs(helloWorldPod.Name, &corev1.PodLogOptions{Container: "queue-proxy"})
queueTLSCount, err := scanPodLogs(req, matchTLSLog)

if err != nil {
t.Fatalf("Failed checking queue-proxy logs: %s", err)
} else if queueTLSCount == 0 {
t.Fatal("TLS not used on requests to queue-proxy")
}
}

func scanPodLogs(req *rest.Request, matcher func(string) bool) (matchCount int, err error) {

podLogs, err := req.Stream(context.Background())
if err != nil {
err = fmt.Errorf("Failed to stream activator logs: %w", err)
return
}

buf := new(bytes.Buffer)
_, err = io.Copy(buf, podLogs)
podLogs.Close()
if err != nil {
err = fmt.Errorf("Failed to read activator logs from buffer: %w", err)
return
}

scanner := bufio.NewScanner(buf)
for scanner.Scan() {
if matcher(scanner.Text()) {
matchCount++
}
}

if err = scanner.Err(); err != nil {
err = fmt.Errorf("Failed scanning activator logs: %w", err)
return
}

return
}

func matchTLSLog(line string) bool {
if strings.Contains(line, "TLS") {
if strings.Contains(line, "TLS: <nil>") {
return false
} else if strings.Contains(line, "TLS: {") {
return true
}
}
return false
}

0 comments on commit c183543

Please sign in to comment.