From 14a74cf424826650e06cd45f77fa503b2ada7fe0 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Tue, 15 Aug 2023 10:14:17 -0400 Subject: [PATCH 01/24] allow activator configstore to track network config --- pkg/activator/config/store.go | 16 +++++++++++++--- pkg/activator/config/store_test.go | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/pkg/activator/config/store.go b/pkg/activator/config/store.go index ade9962cb766..e155d3e671d8 100644 --- a/pkg/activator/config/store.go +++ b/pkg/activator/config/store.go @@ -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" ) @@ -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. @@ -51,15 +53,23 @@ 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() + } + 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..., ) diff --git a/pkg/activator/config/store_test.go b/pkg/activator/config/store_test.go index 94c84658f358..4c6c843ea18c 100644 --- a/pkg/activator/config/store_test.go +++ b/pkg/activator/config/store_test.go @@ -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" ) @@ -35,10 +36,20 @@ var tracingConfig = &corev1.ConfigMap{ }, } +var networkingConfig = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: netcfg.ConfigMapName, + }, + Data: map[string]string{ + "dataplane-trust": "Disabled", + }, +} + 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) @@ -46,6 +57,9 @@ func TestStore(t *testing.T) { if got, want := cfg.Tracing.Backend, tracingconfig.None; got != want { t.Fatalf("Tracing.Backend = %v, want %v", got, want) } + if got, want := cfg.Network.DataplaneTrust, netcfg.TrustDisabled; got != want { + t.Fatalf("Networking.DataplaneTrust = %v, want %v", got, want) + } newConfig := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ From dfd8453c2727fe603202abcc3004147a41d01192 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Tue, 15 Aug 2023 10:16:13 -0400 Subject: [PATCH 02/24] add new security mode metrics tag --- pkg/metrics/key.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/metrics/key.go b/pkg/metrics/key.go index cc9dbc805c50..e5afc5050664 100644 --- a/pkg/metrics/key.go +++ b/pkg/metrics/key.go @@ -61,6 +61,9 @@ const ( // LabelResponseTimeout is the label timeout. LabelResponseTimeout = metricskey.LabelResponseTimeout + // LabelSecurityMode is the label for Security Mode Knative is configured to use (see dataplane-trust in config-networking). + LabelSecurityMode = "security_mode" + // ValueUnknown is the default value if the field is unknown, e.g. project will be unknown if Knative // is not running on GKE. ValueUnknown = metricskey.ValueUnknown @@ -77,4 +80,5 @@ var ( ResponseCodeKey = tag.MustNewKey(LabelResponseCode) ResponseCodeClassKey = tag.MustNewKey(LabelResponseCodeClass) RouteTagKey = tag.MustNewKey(LabelRouteTag) + SecurityMode = tag.MustNewKey(LabelSecurityMode) ) From 2f36a818053f0e63865fceaf18766f558e0cec10 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Tue, 15 Aug 2023 10:16:48 -0400 Subject: [PATCH 03/24] activator now adds security mode tag to metrics * based on security mode from config in context --- pkg/activator/handler/handler_test.go | 1 + pkg/activator/handler/metric_handler.go | 5 ++++ pkg/activator/handler/metric_handler_test.go | 24 ++++++++++++++++++++ pkg/activator/handler/metrics.go | 6 ++--- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/pkg/activator/handler/handler_test.go b/pkg/activator/handler/handler_test.go index c206b46304dc..42d7d55df8f5 100644 --- a/pkg/activator/handler/handler_test.go +++ b/pkg/activator/handler/handler_test.go @@ -326,6 +326,7 @@ func revision(namespace, name string) *v1.Revision { func setupConfigStore(t testing.TB, logger *zap.SugaredLogger) *activatorconfig.Store { configStore := activatorconfig.NewStore(logger) configStore.OnConfigChanged(tracingConfig(false)) + configStore.OnConfigChanged(networkConfig(false)) return configStore } diff --git a/pkg/activator/handler/metric_handler.go b/pkg/activator/handler/metric_handler.go index 8c3b058f3135..984bc6a3989f 100644 --- a/pkg/activator/handler/metric_handler.go +++ b/pkg/activator/handler/metric_handler.go @@ -20,8 +20,10 @@ import ( "net/http" "time" + "go.opencensus.io/tag" pkgmetrics "knative.dev/pkg/metrics" "knative.dev/serving/pkg/activator" + activatorconfig "knative.dev/serving/pkg/activator/config" "knative.dev/serving/pkg/apis/serving" pkghttp "knative.dev/serving/pkg/http" "knative.dev/serving/pkg/metrics" @@ -46,6 +48,9 @@ func (h *MetricHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { reporterCtx, _ := metrics.PodRevisionContext(h.podName, activator.Name, rev.Namespace, rev.Labels[serving.ServiceLabelKey], rev.Labels[serving.ConfigurationLabelKey], rev.Name) + securityMode := activatorconfig.FromContext(r.Context()).Network.DataplaneTrust + reporterCtx, _ = tag.New(reporterCtx, tag.Upsert(metrics.SecurityMode, string(securityMode))) + start := time.Now() rr := pkghttp.NewResponseRecorder(w, http.StatusOK) diff --git a/pkg/activator/handler/metric_handler_test.go b/pkg/activator/handler/metric_handler_test.go index a7ab19204d29..8086755f68da 100644 --- a/pkg/activator/handler/metric_handler_test.go +++ b/pkg/activator/handler/metric_handler_test.go @@ -26,10 +26,15 @@ import ( "testing" "go.opencensus.io/resource" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + netcfg "knative.dev/networking/pkg/config" + "knative.dev/pkg/logging" "knative.dev/pkg/metrics/metricstest" _ "knative.dev/pkg/metrics/testing" "knative.dev/serving/pkg/activator" + activatorconfig "knative.dev/serving/pkg/activator/config" "knative.dev/serving/pkg/apis/serving" "knative.dev/serving/pkg/metrics" ) @@ -108,13 +113,19 @@ func TestRequestMetricHandler(t *testing.T) { metrics.LabelContainerName: activator.Name, metrics.LabelResponseCode: strconv.Itoa(labelCode), metrics.LabelResponseCodeClass: strconv.Itoa(labelCode/100) + "xx", + metrics.LabelSecurityMode: string(netcfg.TrustDisabled), } metricstest.AssertMetric(t, metricstest.IntMetric(requestCountM.Name(), 1, wantTags).WithResource(wantResource)) metricstest.AssertMetricExists(t, responseTimeInMsecM.Name()) }() + cm := networkConfig(false) + reqCtx := WithRevisionAndID(context.Background(), rev, types.NamespacedName{Namespace: testNamespace, Name: testRevName}) + configStore := activatorconfig.NewStore(logging.FromContext(reqCtx)) + configStore.OnConfigChanged(cm) + reqCtx = configStore.ToContext(reqCtx) handler.ServeHTTP(resp, req.WithContext(reqCtx)) }) } @@ -148,3 +159,16 @@ func BenchmarkMetricHandler(b *testing.B) { }) }) } + +func networkConfig(internalTLSEnabled bool) *corev1.ConfigMap { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: netcfg.ConfigMapName, + }, + Data: map[string]string{}, + } + if internalTLSEnabled { + cm.Data["dataplane-trust"] = string(netcfg.TrustEnabled) + } + return cm +} diff --git a/pkg/activator/handler/metrics.go b/pkg/activator/handler/metrics.go index 1d2a5c52766a..b0ad63f61fdc 100644 --- a/pkg/activator/handler/metrics.go +++ b/pkg/activator/handler/metrics.go @@ -57,19 +57,19 @@ func register() { Description: "Concurrent requests that are routed to Activator", Measure: requestConcurrencyM, Aggregation: view.LastValue(), - TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey}, + TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.SecurityMode}, }, &view.View{ Description: "The number of requests that are routed to Activator", Measure: requestCountM, Aggregation: view.Count(), - TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey}, + TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.SecurityMode}, }, &view.View{ Description: "The response time in millisecond", Measure: responseTimeInMsecM, Aggregation: defaultLatencyDistribution, - TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey}, + TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.SecurityMode}, }, ); err != nil { panic(err) From 83089bfcd8d61328ad313d4c566df7162c21643a Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Tue, 15 Aug 2023 10:17:51 -0400 Subject: [PATCH 04/24] queue adds security mode tag to metrics * security mode set as env var based on config from reconciler --- pkg/queue/request_metric.go | 21 ++++++++++++------- pkg/queue/request_metric_test.go | 20 +++++++++++------- pkg/queue/sharedmain/main.go | 6 +++++- .../revision/resources/deploy_test.go | 6 ++++++ pkg/reconciler/revision/resources/queue.go | 3 +++ .../revision/resources/queue_test.go | 14 +++++++++++++ 6 files changed, 53 insertions(+), 17 deletions(-) diff --git a/pkg/queue/request_metric.go b/pkg/queue/request_metric.go index 6f43a01bc3e8..098ce14d5079 100644 --- a/pkg/queue/request_metric.go +++ b/pkg/queue/request_metric.go @@ -25,6 +25,7 @@ import ( "go.opencensus.io/stats/view" "go.opencensus.io/tag" + netcfg "knative.dev/networking/pkg/config" netheader "knative.dev/networking/pkg/http/header" pkgmetrics "knative.dev/pkg/metrics" pkghttp "knative.dev/serving/pkg/http" @@ -62,8 +63,9 @@ var ( ) type requestMetricsHandler struct { - next http.Handler - statsCtx context.Context + next http.Handler + statsCtx context.Context + securityMode netcfg.Trust } type appRequestMetricsHandler struct { @@ -74,8 +76,8 @@ type appRequestMetricsHandler struct { // NewRequestMetricsHandler creates an http.Handler that emits request metrics. func NewRequestMetricsHandler(next http.Handler, - ns, service, config, rev, pod string) (http.Handler, error) { - keys := []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.RouteTagKey} + ns, service, config, rev, pod string, securityMode netcfg.Trust) (http.Handler, error) { + keys := []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.RouteTagKey, metrics.SecurityMode} if err := pkgmetrics.RegisterResourceView( &view.View{ Description: "The number of requests that are routed to queue-proxy", @@ -99,8 +101,9 @@ func NewRequestMetricsHandler(next http.Handler, } return &requestMetricsHandler{ - next: next, - statsCtx: ctx, + next: next, + statsCtx: ctx, + securityMode: securityMode, }, nil } @@ -108,6 +111,8 @@ func (h *requestMetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request rr := pkghttp.NewResponseRecorder(w, http.StatusOK) startTime := time.Now() + ctx, _ := tag.New(h.statsCtx, tag.Upsert(metrics.SecurityMode, string(h.securityMode))) + defer func() { // Filter probe requests for revision metrics. if netheader.IsProbe(r) { @@ -119,13 +124,13 @@ func (h *requestMetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request latency := time.Since(startTime) routeTag := GetRouteTagNameFromRequest(r) if err != nil { - ctx := metrics.AugmentWithResponseAndRouteTag(h.statsCtx, + ctx = metrics.AugmentWithResponseAndRouteTag(ctx, http.StatusInternalServerError, routeTag) pkgmetrics.RecordBatch(ctx, requestCountM.M(1), responseTimeInMsecM.M(float64(latency.Milliseconds()))) panic(err) } - ctx := metrics.AugmentWithResponseAndRouteTag(h.statsCtx, + ctx = metrics.AugmentWithResponseAndRouteTag(ctx, rr.ResponseCode, routeTag) pkgmetrics.RecordBatch(ctx, requestCountM.M(1), responseTimeInMsecM.M(float64(latency.Milliseconds()))) diff --git a/pkg/queue/request_metric_test.go b/pkg/queue/request_metric_test.go index 79bd841d4fdb..cc9f4b6ea6ae 100644 --- a/pkg/queue/request_metric_test.go +++ b/pkg/queue/request_metric_test.go @@ -27,6 +27,7 @@ import ( "knative.dev/pkg/metrics/metricstest" "knative.dev/serving/pkg/metrics" + netcfg "knative.dev/networking/pkg/config" _ "knative.dev/pkg/metrics/testing" ) @@ -34,7 +35,7 @@ const targetURI = "http://example.com" func TestNewRequestMetricsHandlerFailure(t *testing.T) { t.Cleanup(reset) - if _, err := NewRequestMetricsHandler(nil /*next*/, "a", "b", "c", "d", "shøüld fail"); err == nil { + if _, err := NewRequestMetricsHandler(nil /*next*/, "a", "b", "c", "d", "shøüld fail", netcfg.TrustDisabled); err == nil { t.Error("Should get error when tag value is not ascii") } } @@ -42,7 +43,7 @@ func TestNewRequestMetricsHandlerFailure(t *testing.T) { func TestRequestMetricsHandler(t *testing.T) { defer reset() baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") + handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) if err != nil { t.Fatal("Failed to create handler:", err) } @@ -57,6 +58,7 @@ func TestRequestMetricsHandler(t *testing.T) { metrics.LabelResponseCode: "200", metrics.LabelResponseCodeClass: "2xx", "route_tag": disabledTagName, + metrics.LabelSecurityMode: string(netcfg.TrustDisabled), } wantResource := &resource.Resource{ Type: "knative_revision", @@ -81,7 +83,7 @@ func TestRequestMetricsHandler(t *testing.T) { func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { defer reset() baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") + handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) if err != nil { t.Fatal("Failed to create handler:", err) } @@ -98,6 +100,7 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { metrics.LabelResponseCode: "200", metrics.LabelResponseCodeClass: "2xx", metrics.LabelRouteTag: "test-tag", + metrics.LabelSecurityMode: string(netcfg.TrustDisabled), } wantResource := &resource.Resource{ Type: "knative_revision", @@ -113,7 +116,7 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { // Testing for default route reset() - handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") + handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) req.Header.Del(netheader.RouteTagKey) req.Header.Set(netheader.DefaultRouteKey, "true") handler.ServeHTTP(resp, req) @@ -121,7 +124,7 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { metricstest.AssertMetric(t, metricstest.IntMetric("request_count", 1, wantTags).WithResource(wantResource)) reset() - handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") + handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) req.Header.Set(netheader.RouteTagKey, "test-tag") req.Header.Set(netheader.DefaultRouteKey, "true") handler.ServeHTTP(resp, req) @@ -129,7 +132,7 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { metricstest.AssertMetric(t, metricstest.IntMetric("request_count", 1, wantTags).WithResource(wantResource)) reset() - handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") + handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) req.Header.Set(netheader.RouteTagKey, "test-tag") req.Header.Set(netheader.DefaultRouteKey, "false") handler.ServeHTTP(resp, req) @@ -149,7 +152,7 @@ func TestRequestMetricsHandlerPanickingHandler(t *testing.T) { baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { panic("no!") }) - handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") + handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) if err != nil { t.Fatal("Failed to create handler:", err) } @@ -166,6 +169,7 @@ func TestRequestMetricsHandlerPanickingHandler(t *testing.T) { metrics.LabelResponseCode: "500", metrics.LabelResponseCodeClass: "5xx", "route_tag": disabledTagName, + metrics.LabelSecurityMode: string(netcfg.TrustDisabled), } wantResource := &resource.Resource{ Type: "knative_revision", @@ -292,7 +296,7 @@ func TestAppRequestMetricsHandler(t *testing.T) { func BenchmarkRequestMetricsHandler(b *testing.B) { baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - handler, _ := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") + handler, _ := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) req := httptest.NewRequest(http.MethodPost, "http://example.com", nil) b.Run("sequential", func(b *testing.B) { diff --git a/pkg/queue/sharedmain/main.go b/pkg/queue/sharedmain/main.go index 6a38b12d8488..952c5189227c 100644 --- a/pkg/queue/sharedmain/main.go +++ b/pkg/queue/sharedmain/main.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/types" "knative.dev/networking/pkg/certificates" + netcfg "knative.dev/networking/pkg/config" netstats "knative.dev/networking/pkg/http/stats" pkglogging "knative.dev/pkg/logging" "knative.dev/pkg/logging/logkey" @@ -106,6 +107,9 @@ type config struct { TracingConfigSampleRate float64 `split_words:"true"` // optional TracingConfigZipkinEndpoint string `split_words:"true"` // optional + // SecurityMode is the trust level for internal encryption, see config-networking.data.dataplane-trust + SecurityMode netcfg.Trust `split_words:"true" required:"true"` + Env } @@ -408,7 +412,7 @@ func requestLogHandler(logger *zap.SugaredLogger, currentHandler http.Handler, e func requestMetricsHandler(logger *zap.SugaredLogger, currentHandler http.Handler, env config) http.Handler { h, err := queue.NewRequestMetricsHandler(currentHandler, env.ServingNamespace, - env.ServingService, env.ServingConfiguration, env.ServingRevision, env.ServingPod) + env.ServingService, env.ServingConfiguration, env.ServingRevision, env.ServingPod, env.SecurityMode) if err != nil { logger.Errorw("Error setting up request metrics reporter. Request metrics will be unavailable.", zap.Error(err)) return currentHandler diff --git a/pkg/reconciler/revision/resources/deploy_test.go b/pkg/reconciler/revision/resources/deploy_test.go index a668d5efe1d3..36e4d6ffdfec 100644 --- a/pkg/reconciler/revision/resources/deploy_test.go +++ b/pkg/reconciler/revision/resources/deploy_test.go @@ -43,6 +43,7 @@ import ( "knative.dev/serving/pkg/deployment" "knative.dev/serving/pkg/queue" + netcfg "knative.dev/networking/pkg/config" _ "knative.dev/pkg/metrics/testing" . "knative.dev/serving/pkg/testing/v1" ) @@ -188,6 +189,9 @@ var ( }, { Name: "ENABLE_HTTP2_AUTO_DETECTION", Value: "false", + }, { + Name: "SECURITY_MODE", + Value: "", }, { Name: "ROOT_CA", Value: "", @@ -539,6 +543,7 @@ func TestMakePodSpec(t *testing.T) { defaults *apicfg.Defaults dc deployment.Config fc apicfg.Features + nc netcfg.Config want *corev1.PodSpec }{{ name: "user-defined user port, queue proxy have PORT env", @@ -1379,6 +1384,7 @@ func TestMakePodSpec(t *testing.T) { cfg.Observability = &test.oc cfg.Deployment = &test.dc cfg.Features = &test.fc + cfg.Network = &test.nc if test.defaults != nil { cfg.Defaults = test.defaults } diff --git a/pkg/reconciler/revision/resources/queue.go b/pkg/reconciler/revision/resources/queue.go index 1fb964a53dab..82494f3ece10 100644 --- a/pkg/reconciler/revision/resources/queue.go +++ b/pkg/reconciler/revision/resources/queue.go @@ -418,6 +418,9 @@ func makeQueueContainer(rev *v1.Revision, cfg *config.Config) (*corev1.Container }, { Name: "ENABLE_HTTP2_AUTO_DETECTION", Value: strconv.FormatBool(cfg.Features.AutoDetectHTTP2 == apicfg.Enabled), + }, { + Name: "SECURITY_MODE", + Value: string(cfg.Network.DataplaneTrust), }, { Name: "ROOT_CA", Value: cfg.Deployment.QueueSidecarRootCA, diff --git a/pkg/reconciler/revision/resources/queue_test.go b/pkg/reconciler/revision/resources/queue_test.go index f8998a549dcb..776a9d91f601 100644 --- a/pkg/reconciler/revision/resources/queue_test.go +++ b/pkg/reconciler/revision/resources/queue_test.go @@ -417,6 +417,18 @@ func TestMakeQueueContainer(t *testing.T) { "ENABLE_HTTP2_AUTO_DETECTION": "false", }) }), + }, { + name: "SecurityMode set", + rev: revision("bar", "foo", + withContainers(containers)), + nc: netcfg.Config{ + DataplaneTrust: netcfg.TrustEnabled, + }, + want: queueContainer(func(c *corev1.Container) { + c.Env = env(map[string]string{ + "SECURITY_MODE": "enabled", + }) + }), }} for _, test := range tests { @@ -434,6 +446,7 @@ func TestMakeQueueContainer(t *testing.T) { Logging: &test.lc, Observability: &test.oc, Deployment: &test.dc, + Network: &test.nc, Config: &apicfg.Config{ Features: &test.fc, }, @@ -1049,6 +1062,7 @@ var defaultEnv = map[string]string{ "REVISION_TIMEOUT_SECONDS": "45", "REVISION_RESPONSE_START_TIMEOUT_SECONDS": "0", "REVISION_IDLE_TIMEOUT_SECONDS": "0", + "SECURITY_MODE": "", "SERVING_CONFIGURATION": "", "SERVING_ENABLE_PROBE_REQUEST_LOG": "false", "SERVING_ENABLE_REQUEST_LOG": "false", From b0787184f49af5a83ffccd10f98d0126b463961c Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Tue, 15 Aug 2023 10:41:56 -0400 Subject: [PATCH 05/24] test: add test image for reading request metrics --- test/conformance.go | 12 +- test/test_images/metricsreader/README.md | 47 +++++ .../test_images/metricsreader/helpers/util.go | 50 +++++ .../metricsreader/metricsreader.go | 171 ++++++++++++++++++ test/test_images/metricsreader/service.yaml | 15 ++ 5 files changed, 290 insertions(+), 5 deletions(-) create mode 100644 test/test_images/metricsreader/README.md create mode 100644 test/test_images/metricsreader/helpers/util.go create mode 100644 test/test_images/metricsreader/metricsreader.go create mode 100644 test/test_images/metricsreader/service.yaml diff --git a/test/conformance.go b/test/conformance.go index df8a605eac83..3b69bbb8eea7 100644 --- a/test/conformance.go +++ b/test/conformance.go @@ -43,6 +43,7 @@ const ( HelloWorld = "helloworld" HTTPProxy = "httpproxy" InvalidHelloWorld = "invalidhelloworld" // Not a real image + MetricsReader = "metricsreader" PizzaPlanet1 = "pizzaplanetv1" PizzaPlanet2 = "pizzaplanetv2" Protocols = "protocols" @@ -57,11 +58,12 @@ const ( SlowStart = "slowstart" // Constants for test image output. - PizzaPlanetText1 = "What a spaceport!" - PizzaPlanetText2 = "Re-energize yourself with a slice of pepperoni!" - HelloWorldText = "Hello World! How about some tasty noodles?" - HelloHTTP2Text = "Hello, New World! How about donuts and coffee?" - EmptyDirText = "From file in empty dir!" + PizzaPlanetText1 = "What a spaceport!" + PizzaPlanetText2 = "Re-energize yourself with a slice of pepperoni!" + HelloWorldText = "Hello World! How about some tasty noodles?" + HelloHTTP2Text = "Hello, New World! How about donuts and coffee?" + EmptyDirText = "From file in empty dir!" + MetricsReaderText = "Come back with a POST for metrics." MultiContainerResponse = "Yay!! multi-container works" diff --git a/test/test_images/metricsreader/README.md b/test/test_images/metricsreader/README.md new file mode 100644 index 000000000000..b9ca1c52b11a --- /dev/null +++ b/test/test_images/metricsreader/README.md @@ -0,0 +1,47 @@ +# Metricsreader test image + +This directory contains the test image used in the Interal Encryption e2e test. + +The image contains a simple Go webserver, `metricsreader.go`, that will, by +default, listen on port `8080` and expose a service at `/`. + +A `GET` request just returns a simple hello message. + +A `POST` request with the IP addresses of the Activator and the Pod for the latest revision will prompt the server to make requests to the metrics endpoints on each IP, and collect the `*_request_count` metrics. It will check for the tag `security_mode` on each metric, and report the counts based on the value of the tag. The `security_mode` tag corresponds to the possible values for the config option `dataplane-trust` in `config-network`. + +This is used in the test to make sure that, when Internal Encryption is enabled (`dataplane-trust != Disabled`), the Activator and Queue Proxies are handling TLS connections. + +An example request and response looks like this: +``` +❯ curl -X POST http://metricsreader-test-image.default.kauz.tanzu.biz -H "Content-Type: application/json" -d '{"activator_ip": "10.24.1.132", "queue_ip": "10.24.3.164"}' | jq . + +{ + "ActivatorData": { + "disabled": 0, + "enabled": 0, + "identity": 0, + "minimal": 1, + "mutual": 0 + }, + "QueueData": { + "disabled": 0, + "enabled": 0, + "identity": 0, + "minimal": 1, + "mutual": 0 + } +} +``` + +By default, the Knative Service is set with an initial-scale, min-scale, and max-scale of 1. This is to make it possible to know the IP of the Queue Proxy before making the call, and to avoid any complications due to multiple instances. + +## Trying out + +To run the image as a Service outside of the test suite: + +`ko apply -f service.yaml` + +## Building + +For details about building and adding new images, see the +[section about test images](/test/README.md#test-images). diff --git a/test/test_images/metricsreader/helpers/util.go b/test/test_images/metricsreader/helpers/util.go new file mode 100644 index 000000000000..e32f27fbf1f3 --- /dev/null +++ b/test/test_images/metricsreader/helpers/util.go @@ -0,0 +1,50 @@ +/* +Copyright 2021 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 helpers + +//. "knative.dev/serving/pkg/testing/v1" + +import ( + netcfg "knative.dev/networking/pkg/config" +) + +type PostData struct { + ActivatorIP string `json:"activator_ip"` + QueueIP string `json:"queue_ip"` +} + +type ResponseData struct { + Activator map[netcfg.Trust]int `json:"activator"` + Queue map[netcfg.Trust]int `json:"queue"` +} + +func NewResponse() *ResponseData { + return &ResponseData{ + Activator: NewSecurityModeMap(), + Queue: NewSecurityModeMap(), + } +} + +func NewSecurityModeMap() map[netcfg.Trust]int { + return map[netcfg.Trust]int{ + netcfg.TrustDisabled: 0, + netcfg.TrustEnabled: 0, + netcfg.TrustIdentity: 0, + netcfg.TrustMinimal: 0, + netcfg.TrustMutual: 0, + } +} diff --git a/test/test_images/metricsreader/metricsreader.go b/test/test_images/metricsreader/metricsreader.go new file mode 100644 index 000000000000..0c9762b0c4a3 --- /dev/null +++ b/test/test_images/metricsreader/metricsreader.go @@ -0,0 +1,171 @@ +/* +Copyright 2018 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 main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "log" + "net" + "net/http" + "time" + + netcfg "knative.dev/networking/pkg/config" + "knative.dev/serving/pkg/metrics" + pkgnet "knative.dev/serving/pkg/networking" + "knative.dev/serving/test" + "knative.dev/serving/test/test_images/metricsreader/helpers" + + io_prometheus_client "github.com/prometheus/client_model/go" + "github.com/prometheus/common/expfmt" +) + +const ( + ActivatorMetricKey = "activator_request_count" + QueueMetricKey = "revision_request_count" +) + +type MetricsClient struct { + http.Client +} + +func (mc *MetricsClient) GetMetricsData(source string) (map[string]*io_prometheus_client.MetricFamily, error) { + resp, err := mc.Get(fmt.Sprintf("http://%s/metrics", source)) + if err != nil { + return nil, fmt.Errorf("error: couldn't call %s metrics endpoint: %w", source, err) + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("error: couldn't read response body from %s metrics endpoint: %w", source, err) + } + return parseMetricsData(body) +} + +func NewMetricsClient(d *helpers.PostData) *MetricsClient { + dialer := &net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + } + transport := http.Transport{ + Proxy: http.ProxyFromEnvironment, + Dial: dialer.Dial, + TLSHandshakeTimeout: 10 * time.Second, + DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + if addr == "activator:80" { + addr = fmt.Sprintf("%s:9090", d.ActivatorIP) + } else if addr == "queue:80" { + addr = fmt.Sprintf("%s:%d", d.QueueIP, pkgnet.UserQueueMetricsPort) + } + return dialer.DialContext(ctx, network, addr) + }, + } + + return &MetricsClient{http.Client{ + Transport: &transport, + Timeout: 4 * time.Second, + }} +} + +func buildResponse(activatorRequestCount, queueRequestCount *io_prometheus_client.MetricFamily) *helpers.ResponseData { + results := helpers.NewResponse() + + for _, l := range activatorRequestCount.Metric[0].Label { + if *l.Name == string(metrics.LabelSecurityMode) { + results.Activator[netcfg.Trust(*l.Value)] = int(*activatorRequestCount.Metric[0].Counter.Value) + } + } + + for _, l := range queueRequestCount.Metric[0].Label { + if *l.Name == string(metrics.LabelSecurityMode) { + results.Queue[netcfg.Trust(*l.Value)] = int(*queueRequestCount.Metric[0].Counter.Value) + } + } + + return results +} + +func parseMetricsData(d []byte) (map[string]*io_prometheus_client.MetricFamily, error) { + //parse the response + var parser expfmt.TextParser + mf, err := parser.TextToMetricFamilies(bytes.NewReader(d)) + if err != nil { + return nil, fmt.Errorf("error: couldn't parse metrics output: %w", err) + } + return mf, nil +} + +func getMetrics(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/" { + http.Error(w, "404 not found.", http.StatusNotFound) + } + switch r.Method { + case "GET": + log.Print("Metricsreader received a GET request.") + w.Write([]byte("Come back with a POST for metrics.")) + case "POST": + log.Print("Metricsreader received a POST request.") + + decoder := json.NewDecoder(r.Body) + var d helpers.PostData + err := decoder.Decode(&d) + if err != nil { + msg := fmt.Errorf("failed to decode POST data: %w", err) + log.Print(msg) + http.Error(w, msg.Error(), http.StatusInternalServerError) + } + + metricsClient := NewMetricsClient(&d) + + activatorData, err := metricsClient.GetMetricsData("activator") + if err != nil { + log.Print(err.Error()) + http.Error(w, err.Error(), http.StatusInternalServerError) + } + + queueData, err := metricsClient.GetMetricsData("queue") + if err != nil { + log.Print(err.Error()) + http.Error(w, err.Error(), http.StatusInternalServerError) + } + + results := buildResponse(activatorData[ActivatorMetricKey], queueData[QueueMetricKey]) + + jsonResponseData, err := json.Marshal(results) + if err != nil { + msg := fmt.Errorf("failed to marshal results to json: %w", err) + log.Print(msg) + http.Error(w, msg.Error(), http.StatusInternalServerError) + } else { + w.Write(jsonResponseData) + } + + default: + msg := "Sorry, only GET and POST methods are supported." + log.Print(msg) + http.Error(w, msg, http.StatusMethodNotAllowed) + } +} + +func main() { + log.Print("metricsreader app started.") + + test.ListenAndServeGracefully(":8080", getMetrics) +} diff --git a/test/test_images/metricsreader/service.yaml b/test/test_images/metricsreader/service.yaml new file mode 100644 index 000000000000..c38616a7fbee --- /dev/null +++ b/test/test_images/metricsreader/service.yaml @@ -0,0 +1,15 @@ +apiVersion: serving.knative.dev/v1 +kind: Service +metadata: + name: metricsreader-test-image + namespace: default +spec: + template: + metadata: + annotations: + autoscaling.knative.dev/initial-scale: "1" + autoscaling.knative.dev/min-scale: "1" + autoscaling.knative.dev/max-scale: "1" + spec: + containers: + - image: ko://knative.dev/serving/test/test_images/metricsreader From c0e4365382e2df3f72dafd2fda49c21bcc47499b Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Tue, 15 Aug 2023 10:42:23 -0400 Subject: [PATCH 06/24] test: add internal encryption e2e tests --- test/e2e-common.sh | 7 + test/e2e-internal-encryption-tests.sh | 19 --- test/e2e-tests.sh | 8 + test/e2e/internalencryption/README.md | 15 ++ .../internalencryption_test.go | 145 ++++++++++++++++++ 5 files changed, 175 insertions(+), 19 deletions(-) delete mode 100755 test/e2e-internal-encryption-tests.sh create mode 100644 test/e2e/internalencryption/README.md create mode 100644 test/e2e/internalencryption/internalencryption_test.go diff --git a/test/e2e-common.sh b/test/e2e-common.sh index b4f216ce9432..bcdad3c7de57 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -453,6 +453,13 @@ 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" diff --git a/test/e2e-internal-encryption-tests.sh b/test/e2e-internal-encryption-tests.sh deleted file mode 100755 index 37e94c6f31b3..000000000000 --- a/test/e2e-internal-encryption-tests.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/usr/bin/env bash - -# Copyright 2022 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. - -echo "TODO(KauzClay): Implement Me!" -exit 0 - diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 4276dec8e33d..f4d0c1eec22d 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -81,6 +81,14 @@ 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 dataplane-trust enabled config-network || fail_test +# with the current implementation, Activator is always in the request path, and needs to be restarted after configuring dataplane-trust +restart_pod ${SYSTEM_NAMESPACE} "app=activator" +go_test_e2e -timeout=2m ./test/e2e/internalencryption ${TEST_OPTIONS} || failed=1 +toggle_feature dataplane-trust disabled config-network || fail_test +# with the current implementation, Activator is always in the request path, and needs to be restarted after configuring dataplane-trust +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 diff --git a/test/e2e/internalencryption/README.md b/test/e2e/internalencryption/README.md new file mode 100644 index 000000000000..16204e1f390e --- /dev/null +++ b/test/e2e/internalencryption/README.md @@ -0,0 +1,15 @@ +# Internal Encryption E2E Tests + +In order to test Internal Encryption, this test looks at the `security_mode` tag on `request_count` metrics from the Activator and QueueProxy. + +The `metricsreader` test image was created for this purpose. Given the PodIPs of the Activator and the Knative Service pod (i.e. the QueueProxy), it will make requests to each respective `/metrics` endpoint, pull out the `*_request_count` metric, look for the tag `security_mode`, and respond with a map of tag values to counts. The [README.md](../../test_images/metricsreader/README.md) will explain in more detail. + +The test works as follows: +* The [setup script](../../e2e-internal-encryption-tests.sh) configures the `dataplane-trust` config in `config-network` to `enabled`. +* The [test](internalencryption_test.go) deploys the `metricsreader` Knative Service. This service uses annotations to set the initial, min, and max scale to 1. This is to guarantee the PodIP is consistent during the test, and avoid complications of having multiple instances. +* The test then extracts the PodIPs of the activator and the pod of the latest `metricsreader` Revision. +* The test will make 3 requests to the `metricsreader` pod: + * A GET request to make sure it is alive. + * A first POST request to get the initial `request_count`s. + * A second POST request to get the updated `request_count`s. +* The test will then compare the counts between the initial and updated requests to determine if the counts have increased, indicating that Internal Encryption is indeed happening. diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go new file mode 100644 index 000000000000..28ee73bc15ab --- /dev/null +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -0,0 +1,145 @@ +//go:build e2e +// +build e2e + +/* +Copyright 2021 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 internalencryption + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" + + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + netcfg "knative.dev/networking/pkg/config" + pkgTest "knative.dev/pkg/test" + "knative.dev/pkg/test/spoof" + + //. "knative.dev/serving/pkg/testing/v1" + "knative.dev/serving/test" + "knative.dev/serving/test/test_images/metricsreader/helpers" + v1test "knative.dev/serving/test/v1" +) + +var ( + ExpectedSecurityMode = netcfg.TrustEnabled +) + +// TestInitContainers tests init containers support. +func TestInternalEncryption(t *testing.T) { + t.Parallel() + clients := test.Setup(t) + + names := test.ResourceNames{ + Service: test.ObjectNameForTest(t), + Image: test.MetricsReader, + } + + 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) + } + + url := resources.Route.Status.URL.URL() + if _, err := pkgTest.CheckEndpointState( + context.Background(), + clients.KubeClient, + t.Logf, + url, + spoof.MatchesAllOf(spoof.IsStatusOK, spoof.MatchesBody(test.MetricsReaderText)), + "MetricsReaderText", + test.ServingFlags.ResolvableDomain, + ); err != nil { + t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.MetricsReaderText, err) + } + + 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) + } + var postData helpers.PostData + + if len(pods.Items) > 0 { + postData.QueueIP = pods.Items[0].Status.PodIP + } + + pods, err = clients.KubeClient.CoreV1().Pods("knative-serving").List(context.TODO(), v1.ListOptions{ + LabelSelector: "app=activator", + }) + if err != nil { + t.Fatalf("Failed to get pods: %v", err) + } + if len(pods.Items) > 0 { + postData.ActivatorIP = pods.Items[0].Status.PodIP + } + + initialCounts, err := getTLSCounts(url.String(), &postData) + if err != nil { + t.Fatalf("Failed to get initial TLS Connection Counts: %v", err) + } + t.Logf("Initial Counts: %#v", initialCounts) + + updatedCounts, err := getTLSCounts(url.String(), &postData) + if err != nil { + t.Fatalf("Failed to get updated TLS Connection Counts: %v", err) + } + t.Logf("Updated Counts: %#v", updatedCounts) + + if updatedCounts.Activator[ExpectedSecurityMode] <= initialCounts.Activator[ExpectedSecurityMode] { + t.Fatalf("Connection Count with SecurityMode (%s) at Activator pod failed to increase", ExpectedSecurityMode) + } + + if updatedCounts.Queue[ExpectedSecurityMode] <= initialCounts.Queue[ExpectedSecurityMode] { + t.Fatalf("Connection Count with SecurityMode (%s) at QueueProxy pod failed to increase", ExpectedSecurityMode) + } + +} + +func getTLSCounts(url string, d *helpers.PostData) (*helpers.ResponseData, error) { + counts := &helpers.ResponseData{} + + jsonPostData, err := json.Marshal(d) + if err != nil { + return nil, fmt.Errorf("failed to marshal post data request to JSON:\n %#v\n %w", *d, err) + } + resp, err := http.Post(url, "application/json", bytes.NewBuffer(jsonPostData)) + if err != nil { + return nil, fmt.Errorf("failed to make POST request to %s: %w", url, err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("Failed to read response body from %s: %w", url, err) + } + + err = json.Unmarshal(body, &counts) + if err != nil { + return nil, fmt.Errorf("Failed to unmarshal response body:\n body: %s\n err: %w", string(body), err) + } + + return counts, nil +} From 698306692dc1590fe0cef48f4d5528a64bbea536 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Tue, 15 Aug 2023 11:39:24 -0400 Subject: [PATCH 07/24] run update-deps --- go.mod | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go.mod b/go.mod index 3e0054ea4c9c..820738ab129e 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,8 @@ require ( github.com/hashicorp/golang-lru v1.0.2 github.com/influxdata/influxdb-client-go/v2 v2.9.0 github.com/kelseyhightower/envconfig v1.4.0 + github.com/prometheus/client_model v0.4.0 + github.com/prometheus/common v0.44.0 github.com/tsenart/vegeta/v12 v12.11.0 go.opencensus.io v0.24.0 go.uber.org/atomic v1.9.0 From d0a75ba8a9e6fbf08ad42efff960da74897acd34 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 13:42:48 -0400 Subject: [PATCH 08/24] first pass: change internal encryption test to read logs --- test/e2e-tests.sh | 4 + .../internalencryption_test.go | 114 ++++++++++-------- 2 files changed, 70 insertions(+), 48 deletions(-) diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index f4d0c1eec22d..aa8ff7c5dab4 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -82,10 +82,14 @@ 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 dataplane-trust 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 the current implementation, Activator is always in the request path, and needs to be restarted after configuring dataplane-trust restart_pod ${SYSTEM_NAMESPACE} "app=activator" go_test_e2e -timeout=2m ./test/e2e/internalencryption ${TEST_OPTIONS} || failed=1 toggle_feature dataplane-trust 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 dataplane-trust restart_pod ${SYSTEM_NAMESPACE} "app=activator" diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index 28ee73bc15ab..dff5f75441ef 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -20,22 +20,27 @@ limitations under the License. package internalencryption import ( + "bufio" "bytes" "context" - "encoding/json" + "crypto/tls" "fmt" "io" - "net/http" + "log" + "strings" "testing" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "k8s.io/client-go/rest" netcfg "knative.dev/networking/pkg/config" pkgTest "knative.dev/pkg/test" "knative.dev/pkg/test/spoof" + "knative.dev/serving/pkg/apis/serving" //. "knative.dev/serving/pkg/testing/v1" "knative.dev/serving/test" - "knative.dev/serving/test/test_images/metricsreader/helpers" v1test "knative.dev/serving/test/v1" ) @@ -43,6 +48,11 @@ var ( ExpectedSecurityMode = netcfg.TrustEnabled ) +type RequestLog struct { + RequestURL string `json:"requestUrl"` + TLS tls.ConnectionState `json:"tls"` +} + // TestInitContainers tests init containers support. func TestInternalEncryption(t *testing.T) { t.Parallel() @@ -50,7 +60,7 @@ func TestInternalEncryption(t *testing.T) { names := test.ResourceNames{ Service: test.ObjectNameForTest(t), - Image: test.MetricsReader, + Image: test.HelloWorld, } test.EnsureTearDown(t, clients, &names) @@ -67,79 +77,87 @@ func TestInternalEncryption(t *testing.T) { clients.KubeClient, t.Logf, url, - spoof.MatchesAllOf(spoof.IsStatusOK, spoof.MatchesBody(test.MetricsReaderText)), - "MetricsReaderText", + spoof.MatchesAllOf(spoof.IsStatusOK, spoof.MatchesBody(test.HelloWorldText)), + "HelloWorldText", test.ServingFlags.ResolvableDomain, ); err != nil { - t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.MetricsReaderText, err) + t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.HelloWorldText, err) } - 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) + revision := resources.Revision + if val := revision.Labels[serving.ConfigurationLabelKey]; val != names.Config { + t.Fatalf("Got revision label configuration=%q, want=%q ", names.Config, val) } - var postData helpers.PostData - - if len(pods.Items) > 0 { - postData.QueueIP = pods.Items[0].Status.PodIP + if val := revision.Labels[serving.ServiceLabelKey]; val != names.Service { + t.Fatalf("Got revision label service=%q, want=%q", val, names.Service) } - pods, err = clients.KubeClient.CoreV1().Pods("knative-serving").List(context.TODO(), v1.ListOptions{ + // Check on the logs for the activator + pods, err := clients.KubeClient.CoreV1().Pods("knative-serving").List(context.TODO(), v1.ListOptions{ LabelSelector: "app=activator", }) if err != nil { t.Fatalf("Failed to get pods: %v", err) } - if len(pods.Items) > 0 { - postData.ActivatorIP = pods.Items[0].Status.PodIP - } + activatorPod := pods.Items[0] - initialCounts, err := getTLSCounts(url.String(), &postData) - if err != nil { - t.Fatalf("Failed to get initial TLS Connection Counts: %v", err) - } - t.Logf("Initial Counts: %#v", initialCounts) + req := clients.KubeClient.CoreV1().Pods(activatorPod.Namespace).GetLogs(activatorPod.Name, &corev1.PodLogOptions{}) + _, nilCount, err := getPodLogs(req) - updatedCounts, err := getTLSCounts(url.String(), &postData) - if err != nil { - t.Fatalf("Failed to get updated TLS Connection Counts: %v", err) + if nilCount > 0 { + t.Fatal("TLS not used on requests to activator") } - t.Logf("Updated Counts: %#v", updatedCounts) - if updatedCounts.Activator[ExpectedSecurityMode] <= initialCounts.Activator[ExpectedSecurityMode] { - t.Fatalf("Connection Count with SecurityMode (%s) at Activator pod failed to increase", ExpectedSecurityMode) + // Check on the logs for the queue-proxy + 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) } + helloWorldPod := pods.Items[0] + req = clients.KubeClient.CoreV1().Pods(helloWorldPod.Namespace).GetLogs(helloWorldPod.Name, &corev1.PodLogOptions{}) + _, nilCount, err = getPodLogs(req) - if updatedCounts.Queue[ExpectedSecurityMode] <= initialCounts.Queue[ExpectedSecurityMode] { - t.Fatalf("Connection Count with SecurityMode (%s) at QueueProxy pod failed to increase", ExpectedSecurityMode) + if nilCount > 0 { + t.Fatal("TLS not used on requests to queue-proxy") } - } -func getTLSCounts(url string, d *helpers.PostData) (*helpers.ResponseData, error) { - counts := &helpers.ResponseData{} +func getPodLogs(req *rest.Request) (tlsCount int, nilCount int, err error) { + tlsCount, nilCount = 0, 0 - jsonPostData, err := json.Marshal(d) + podLogs, err := req.Stream(context.Background()) if err != nil { - return nil, fmt.Errorf("failed to marshal post data request to JSON:\n %#v\n %w", *d, err) + err = fmt.Errorf("Failed to stream activator logs: %v", err) + return } - resp, err := http.Post(url, "application/json", bytes.NewBuffer(jsonPostData)) + + buf := new(bytes.Buffer) + _, err = io.Copy(buf, podLogs) + podLogs.Close() if err != nil { - return nil, fmt.Errorf("failed to make POST request to %s: %w", url, err) + err = fmt.Errorf("Failed to read activator logs from buffer: %v", err) + return } - defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("Failed to read response body from %s: %w", url, err) + scanner := bufio.NewScanner(buf) + for scanner.Scan() { + log.Printf("log: %s", scanner.Text()) + if strings.Contains(scanner.Text(), "TLS") { + if strings.Contains(scanner.Text(), "TLS: ") { + nilCount += 1 + + } else if strings.Contains(scanner.Text(), "TLS: [") { + tlsCount += 1 + } + } } - err = json.Unmarshal(body, &counts) - if err != nil { - return nil, fmt.Errorf("Failed to unmarshal response body:\n body: %s\n err: %w", string(body), err) + if err = scanner.Err(); err != nil { + err = fmt.Errorf("Failed to scan activator logs: %v", err) + return } - return counts, nil + return } From 23878401e56874ad4233ad048c6f85651a7515a8 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 13:43:24 -0400 Subject: [PATCH 09/24] remove metricreader image --- test/conformance.go | 12 +- test/test_images/metricsreader/README.md | 47 ----- .../test_images/metricsreader/helpers/util.go | 50 ----- .../metricsreader/metricsreader.go | 171 ------------------ test/test_images/metricsreader/service.yaml | 15 -- 5 files changed, 5 insertions(+), 290 deletions(-) delete mode 100644 test/test_images/metricsreader/README.md delete mode 100644 test/test_images/metricsreader/helpers/util.go delete mode 100644 test/test_images/metricsreader/metricsreader.go delete mode 100644 test/test_images/metricsreader/service.yaml diff --git a/test/conformance.go b/test/conformance.go index 3b69bbb8eea7..df8a605eac83 100644 --- a/test/conformance.go +++ b/test/conformance.go @@ -43,7 +43,6 @@ const ( HelloWorld = "helloworld" HTTPProxy = "httpproxy" InvalidHelloWorld = "invalidhelloworld" // Not a real image - MetricsReader = "metricsreader" PizzaPlanet1 = "pizzaplanetv1" PizzaPlanet2 = "pizzaplanetv2" Protocols = "protocols" @@ -58,12 +57,11 @@ const ( SlowStart = "slowstart" // Constants for test image output. - PizzaPlanetText1 = "What a spaceport!" - PizzaPlanetText2 = "Re-energize yourself with a slice of pepperoni!" - HelloWorldText = "Hello World! How about some tasty noodles?" - HelloHTTP2Text = "Hello, New World! How about donuts and coffee?" - EmptyDirText = "From file in empty dir!" - MetricsReaderText = "Come back with a POST for metrics." + PizzaPlanetText1 = "What a spaceport!" + PizzaPlanetText2 = "Re-energize yourself with a slice of pepperoni!" + HelloWorldText = "Hello World! How about some tasty noodles?" + HelloHTTP2Text = "Hello, New World! How about donuts and coffee?" + EmptyDirText = "From file in empty dir!" MultiContainerResponse = "Yay!! multi-container works" diff --git a/test/test_images/metricsreader/README.md b/test/test_images/metricsreader/README.md deleted file mode 100644 index b9ca1c52b11a..000000000000 --- a/test/test_images/metricsreader/README.md +++ /dev/null @@ -1,47 +0,0 @@ -# Metricsreader test image - -This directory contains the test image used in the Interal Encryption e2e test. - -The image contains a simple Go webserver, `metricsreader.go`, that will, by -default, listen on port `8080` and expose a service at `/`. - -A `GET` request just returns a simple hello message. - -A `POST` request with the IP addresses of the Activator and the Pod for the latest revision will prompt the server to make requests to the metrics endpoints on each IP, and collect the `*_request_count` metrics. It will check for the tag `security_mode` on each metric, and report the counts based on the value of the tag. The `security_mode` tag corresponds to the possible values for the config option `dataplane-trust` in `config-network`. - -This is used in the test to make sure that, when Internal Encryption is enabled (`dataplane-trust != Disabled`), the Activator and Queue Proxies are handling TLS connections. - -An example request and response looks like this: -``` -❯ curl -X POST http://metricsreader-test-image.default.kauz.tanzu.biz -H "Content-Type: application/json" -d '{"activator_ip": "10.24.1.132", "queue_ip": "10.24.3.164"}' | jq . - -{ - "ActivatorData": { - "disabled": 0, - "enabled": 0, - "identity": 0, - "minimal": 1, - "mutual": 0 - }, - "QueueData": { - "disabled": 0, - "enabled": 0, - "identity": 0, - "minimal": 1, - "mutual": 0 - } -} -``` - -By default, the Knative Service is set with an initial-scale, min-scale, and max-scale of 1. This is to make it possible to know the IP of the Queue Proxy before making the call, and to avoid any complications due to multiple instances. - -## Trying out - -To run the image as a Service outside of the test suite: - -`ko apply -f service.yaml` - -## Building - -For details about building and adding new images, see the -[section about test images](/test/README.md#test-images). diff --git a/test/test_images/metricsreader/helpers/util.go b/test/test_images/metricsreader/helpers/util.go deleted file mode 100644 index e32f27fbf1f3..000000000000 --- a/test/test_images/metricsreader/helpers/util.go +++ /dev/null @@ -1,50 +0,0 @@ -/* -Copyright 2021 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 helpers - -//. "knative.dev/serving/pkg/testing/v1" - -import ( - netcfg "knative.dev/networking/pkg/config" -) - -type PostData struct { - ActivatorIP string `json:"activator_ip"` - QueueIP string `json:"queue_ip"` -} - -type ResponseData struct { - Activator map[netcfg.Trust]int `json:"activator"` - Queue map[netcfg.Trust]int `json:"queue"` -} - -func NewResponse() *ResponseData { - return &ResponseData{ - Activator: NewSecurityModeMap(), - Queue: NewSecurityModeMap(), - } -} - -func NewSecurityModeMap() map[netcfg.Trust]int { - return map[netcfg.Trust]int{ - netcfg.TrustDisabled: 0, - netcfg.TrustEnabled: 0, - netcfg.TrustIdentity: 0, - netcfg.TrustMinimal: 0, - netcfg.TrustMutual: 0, - } -} diff --git a/test/test_images/metricsreader/metricsreader.go b/test/test_images/metricsreader/metricsreader.go deleted file mode 100644 index 0c9762b0c4a3..000000000000 --- a/test/test_images/metricsreader/metricsreader.go +++ /dev/null @@ -1,171 +0,0 @@ -/* -Copyright 2018 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 main - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "io" - "log" - "net" - "net/http" - "time" - - netcfg "knative.dev/networking/pkg/config" - "knative.dev/serving/pkg/metrics" - pkgnet "knative.dev/serving/pkg/networking" - "knative.dev/serving/test" - "knative.dev/serving/test/test_images/metricsreader/helpers" - - io_prometheus_client "github.com/prometheus/client_model/go" - "github.com/prometheus/common/expfmt" -) - -const ( - ActivatorMetricKey = "activator_request_count" - QueueMetricKey = "revision_request_count" -) - -type MetricsClient struct { - http.Client -} - -func (mc *MetricsClient) GetMetricsData(source string) (map[string]*io_prometheus_client.MetricFamily, error) { - resp, err := mc.Get(fmt.Sprintf("http://%s/metrics", source)) - if err != nil { - return nil, fmt.Errorf("error: couldn't call %s metrics endpoint: %w", source, err) - } - defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("error: couldn't read response body from %s metrics endpoint: %w", source, err) - } - return parseMetricsData(body) -} - -func NewMetricsClient(d *helpers.PostData) *MetricsClient { - dialer := &net.Dialer{ - Timeout: 30 * time.Second, - KeepAlive: 30 * time.Second, - } - transport := http.Transport{ - Proxy: http.ProxyFromEnvironment, - Dial: dialer.Dial, - TLSHandshakeTimeout: 10 * time.Second, - DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { - if addr == "activator:80" { - addr = fmt.Sprintf("%s:9090", d.ActivatorIP) - } else if addr == "queue:80" { - addr = fmt.Sprintf("%s:%d", d.QueueIP, pkgnet.UserQueueMetricsPort) - } - return dialer.DialContext(ctx, network, addr) - }, - } - - return &MetricsClient{http.Client{ - Transport: &transport, - Timeout: 4 * time.Second, - }} -} - -func buildResponse(activatorRequestCount, queueRequestCount *io_prometheus_client.MetricFamily) *helpers.ResponseData { - results := helpers.NewResponse() - - for _, l := range activatorRequestCount.Metric[0].Label { - if *l.Name == string(metrics.LabelSecurityMode) { - results.Activator[netcfg.Trust(*l.Value)] = int(*activatorRequestCount.Metric[0].Counter.Value) - } - } - - for _, l := range queueRequestCount.Metric[0].Label { - if *l.Name == string(metrics.LabelSecurityMode) { - results.Queue[netcfg.Trust(*l.Value)] = int(*queueRequestCount.Metric[0].Counter.Value) - } - } - - return results -} - -func parseMetricsData(d []byte) (map[string]*io_prometheus_client.MetricFamily, error) { - //parse the response - var parser expfmt.TextParser - mf, err := parser.TextToMetricFamilies(bytes.NewReader(d)) - if err != nil { - return nil, fmt.Errorf("error: couldn't parse metrics output: %w", err) - } - return mf, nil -} - -func getMetrics(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/" { - http.Error(w, "404 not found.", http.StatusNotFound) - } - switch r.Method { - case "GET": - log.Print("Metricsreader received a GET request.") - w.Write([]byte("Come back with a POST for metrics.")) - case "POST": - log.Print("Metricsreader received a POST request.") - - decoder := json.NewDecoder(r.Body) - var d helpers.PostData - err := decoder.Decode(&d) - if err != nil { - msg := fmt.Errorf("failed to decode POST data: %w", err) - log.Print(msg) - http.Error(w, msg.Error(), http.StatusInternalServerError) - } - - metricsClient := NewMetricsClient(&d) - - activatorData, err := metricsClient.GetMetricsData("activator") - if err != nil { - log.Print(err.Error()) - http.Error(w, err.Error(), http.StatusInternalServerError) - } - - queueData, err := metricsClient.GetMetricsData("queue") - if err != nil { - log.Print(err.Error()) - http.Error(w, err.Error(), http.StatusInternalServerError) - } - - results := buildResponse(activatorData[ActivatorMetricKey], queueData[QueueMetricKey]) - - jsonResponseData, err := json.Marshal(results) - if err != nil { - msg := fmt.Errorf("failed to marshal results to json: %w", err) - log.Print(msg) - http.Error(w, msg.Error(), http.StatusInternalServerError) - } else { - w.Write(jsonResponseData) - } - - default: - msg := "Sorry, only GET and POST methods are supported." - log.Print(msg) - http.Error(w, msg, http.StatusMethodNotAllowed) - } -} - -func main() { - log.Print("metricsreader app started.") - - test.ListenAndServeGracefully(":8080", getMetrics) -} diff --git a/test/test_images/metricsreader/service.yaml b/test/test_images/metricsreader/service.yaml deleted file mode 100644 index c38616a7fbee..000000000000 --- a/test/test_images/metricsreader/service.yaml +++ /dev/null @@ -1,15 +0,0 @@ -apiVersion: serving.knative.dev/v1 -kind: Service -metadata: - name: metricsreader-test-image - namespace: default -spec: - template: - metadata: - annotations: - autoscaling.knative.dev/initial-scale: "1" - autoscaling.knative.dev/min-scale: "1" - autoscaling.knative.dev/max-scale: "1" - spec: - containers: - - image: ko://knative.dev/serving/test/test_images/metricsreader From 08c57c426e3a62e3586421c62abb2af95ac6ca5b Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 13:48:00 -0400 Subject: [PATCH 10/24] update internal encryption test readme --- test/e2e/internalencryption/README.md | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/test/e2e/internalencryption/README.md b/test/e2e/internalencryption/README.md index 16204e1f390e..9aa67f6cb0f2 100644 --- a/test/e2e/internalencryption/README.md +++ b/test/e2e/internalencryption/README.md @@ -1,15 +1,11 @@ # Internal Encryption E2E Tests -In order to test Internal Encryption, this test looks at the `security_mode` tag on `request_count` metrics from the Activator and QueueProxy. - -The `metricsreader` test image was created for this purpose. Given the PodIPs of the Activator and the Knative Service pod (i.e. the QueueProxy), it will make requests to each respective `/metrics` endpoint, pull out the `*_request_count` metric, look for the tag `security_mode`, and respond with a map of tag values to counts. The [README.md](../../test_images/metricsreader/README.md) will explain in more detail. - -The test works as follows: -* The [setup script](../../e2e-internal-encryption-tests.sh) configures the `dataplane-trust` config in `config-network` to `enabled`. -* The [test](internalencryption_test.go) deploys the `metricsreader` Knative Service. This service uses annotations to set the initial, min, and max scale to 1. This is to guarantee the PodIP is consistent during the test, and avoid complications of having multiple instances. -* The test then extracts the PodIPs of the activator and the pod of the latest `metricsreader` Revision. -* The test will make 3 requests to the `metricsreader` pod: - * A GET request to make sure it is alive. - * A first POST request to get the initial `request_count`s. - * A second POST request to get the updated `request_count`s. -* The test will then compare the counts between the initial and updated requests to determine if the counts have increased, indicating that Internal Encryption is indeed happening. +In order to test Internal Encryption, this test turns enables request logging and sets the request log template to `TLS: {{.Request.TLS}}`. + +The test setup will enable Internal Encryption, 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: ` appears, which indicates that TLS was not used. If that count is greater than 0, the test will fail. \ No newline at end of file From d5317328f75633c815cc67b1a19ed420fe3100bc Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 14:01:29 -0400 Subject: [PATCH 11/24] run update-deps --- go.mod | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.mod b/go.mod index 820738ab129e..3e0054ea4c9c 100644 --- a/go.mod +++ b/go.mod @@ -14,8 +14,6 @@ require ( github.com/hashicorp/golang-lru v1.0.2 github.com/influxdata/influxdb-client-go/v2 v2.9.0 github.com/kelseyhightower/envconfig v1.4.0 - github.com/prometheus/client_model v0.4.0 - github.com/prometheus/common v0.44.0 github.com/tsenart/vegeta/v12 v12.11.0 go.opencensus.io v0.24.0 go.uber.org/atomic v1.9.0 From b351b70a89716cc17937e8441a74af6c3631f5da Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 15:28:02 -0400 Subject: [PATCH 12/24] remove securitymode metric --- pkg/activator/config/store_test.go | 6 ++--- pkg/activator/handler/metric_handler.go | 5 ---- pkg/activator/handler/metric_handler_test.go | 24 ------------------- pkg/activator/handler/metrics.go | 6 ++--- pkg/metrics/key.go | 4 ---- pkg/queue/request_metric.go | 21 +++++++--------- pkg/queue/request_metric_test.go | 20 +++++++--------- pkg/queue/sharedmain/main.go | 6 +---- .../revision/resources/deploy_test.go | 6 ----- pkg/reconciler/revision/resources/queue.go | 3 --- .../revision/resources/queue_test.go | 14 ----------- 11 files changed, 23 insertions(+), 92 deletions(-) diff --git a/pkg/activator/config/store_test.go b/pkg/activator/config/store_test.go index 4c6c843ea18c..2152b7badf34 100644 --- a/pkg/activator/config/store_test.go +++ b/pkg/activator/config/store_test.go @@ -41,7 +41,7 @@ var networkingConfig = &corev1.ConfigMap{ Name: netcfg.ConfigMapName, }, Data: map[string]string{ - "dataplane-trust": "Disabled", + "ingress-class": "random.ingress.networking.knative.dev", }, } @@ -57,8 +57,8 @@ func TestStore(t *testing.T) { if got, want := cfg.Tracing.Backend, tracingconfig.None; got != want { t.Fatalf("Tracing.Backend = %v, want %v", got, want) } - if got, want := cfg.Network.DataplaneTrust, netcfg.TrustDisabled; got != want { - t.Fatalf("Networking.DataplaneTrust = %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{ diff --git a/pkg/activator/handler/metric_handler.go b/pkg/activator/handler/metric_handler.go index 984bc6a3989f..8c3b058f3135 100644 --- a/pkg/activator/handler/metric_handler.go +++ b/pkg/activator/handler/metric_handler.go @@ -20,10 +20,8 @@ import ( "net/http" "time" - "go.opencensus.io/tag" pkgmetrics "knative.dev/pkg/metrics" "knative.dev/serving/pkg/activator" - activatorconfig "knative.dev/serving/pkg/activator/config" "knative.dev/serving/pkg/apis/serving" pkghttp "knative.dev/serving/pkg/http" "knative.dev/serving/pkg/metrics" @@ -48,9 +46,6 @@ func (h *MetricHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { reporterCtx, _ := metrics.PodRevisionContext(h.podName, activator.Name, rev.Namespace, rev.Labels[serving.ServiceLabelKey], rev.Labels[serving.ConfigurationLabelKey], rev.Name) - securityMode := activatorconfig.FromContext(r.Context()).Network.DataplaneTrust - reporterCtx, _ = tag.New(reporterCtx, tag.Upsert(metrics.SecurityMode, string(securityMode))) - start := time.Now() rr := pkghttp.NewResponseRecorder(w, http.StatusOK) diff --git a/pkg/activator/handler/metric_handler_test.go b/pkg/activator/handler/metric_handler_test.go index 8086755f68da..a7ab19204d29 100644 --- a/pkg/activator/handler/metric_handler_test.go +++ b/pkg/activator/handler/metric_handler_test.go @@ -26,15 +26,10 @@ import ( "testing" "go.opencensus.io/resource" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - netcfg "knative.dev/networking/pkg/config" - "knative.dev/pkg/logging" "knative.dev/pkg/metrics/metricstest" _ "knative.dev/pkg/metrics/testing" "knative.dev/serving/pkg/activator" - activatorconfig "knative.dev/serving/pkg/activator/config" "knative.dev/serving/pkg/apis/serving" "knative.dev/serving/pkg/metrics" ) @@ -113,19 +108,13 @@ func TestRequestMetricHandler(t *testing.T) { metrics.LabelContainerName: activator.Name, metrics.LabelResponseCode: strconv.Itoa(labelCode), metrics.LabelResponseCodeClass: strconv.Itoa(labelCode/100) + "xx", - metrics.LabelSecurityMode: string(netcfg.TrustDisabled), } metricstest.AssertMetric(t, metricstest.IntMetric(requestCountM.Name(), 1, wantTags).WithResource(wantResource)) metricstest.AssertMetricExists(t, responseTimeInMsecM.Name()) }() - cm := networkConfig(false) - reqCtx := WithRevisionAndID(context.Background(), rev, types.NamespacedName{Namespace: testNamespace, Name: testRevName}) - configStore := activatorconfig.NewStore(logging.FromContext(reqCtx)) - configStore.OnConfigChanged(cm) - reqCtx = configStore.ToContext(reqCtx) handler.ServeHTTP(resp, req.WithContext(reqCtx)) }) } @@ -159,16 +148,3 @@ func BenchmarkMetricHandler(b *testing.B) { }) }) } - -func networkConfig(internalTLSEnabled bool) *corev1.ConfigMap { - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: netcfg.ConfigMapName, - }, - Data: map[string]string{}, - } - if internalTLSEnabled { - cm.Data["dataplane-trust"] = string(netcfg.TrustEnabled) - } - return cm -} diff --git a/pkg/activator/handler/metrics.go b/pkg/activator/handler/metrics.go index b0ad63f61fdc..1d2a5c52766a 100644 --- a/pkg/activator/handler/metrics.go +++ b/pkg/activator/handler/metrics.go @@ -57,19 +57,19 @@ func register() { Description: "Concurrent requests that are routed to Activator", Measure: requestConcurrencyM, Aggregation: view.LastValue(), - TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.SecurityMode}, + TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey}, }, &view.View{ Description: "The number of requests that are routed to Activator", Measure: requestCountM, Aggregation: view.Count(), - TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.SecurityMode}, + TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey}, }, &view.View{ Description: "The response time in millisecond", Measure: responseTimeInMsecM, Aggregation: defaultLatencyDistribution, - TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.SecurityMode}, + TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey}, }, ); err != nil { panic(err) diff --git a/pkg/metrics/key.go b/pkg/metrics/key.go index e5afc5050664..cc9dbc805c50 100644 --- a/pkg/metrics/key.go +++ b/pkg/metrics/key.go @@ -61,9 +61,6 @@ const ( // LabelResponseTimeout is the label timeout. LabelResponseTimeout = metricskey.LabelResponseTimeout - // LabelSecurityMode is the label for Security Mode Knative is configured to use (see dataplane-trust in config-networking). - LabelSecurityMode = "security_mode" - // ValueUnknown is the default value if the field is unknown, e.g. project will be unknown if Knative // is not running on GKE. ValueUnknown = metricskey.ValueUnknown @@ -80,5 +77,4 @@ var ( ResponseCodeKey = tag.MustNewKey(LabelResponseCode) ResponseCodeClassKey = tag.MustNewKey(LabelResponseCodeClass) RouteTagKey = tag.MustNewKey(LabelRouteTag) - SecurityMode = tag.MustNewKey(LabelSecurityMode) ) diff --git a/pkg/queue/request_metric.go b/pkg/queue/request_metric.go index 098ce14d5079..6f43a01bc3e8 100644 --- a/pkg/queue/request_metric.go +++ b/pkg/queue/request_metric.go @@ -25,7 +25,6 @@ import ( "go.opencensus.io/stats/view" "go.opencensus.io/tag" - netcfg "knative.dev/networking/pkg/config" netheader "knative.dev/networking/pkg/http/header" pkgmetrics "knative.dev/pkg/metrics" pkghttp "knative.dev/serving/pkg/http" @@ -63,9 +62,8 @@ var ( ) type requestMetricsHandler struct { - next http.Handler - statsCtx context.Context - securityMode netcfg.Trust + next http.Handler + statsCtx context.Context } type appRequestMetricsHandler struct { @@ -76,8 +74,8 @@ type appRequestMetricsHandler struct { // NewRequestMetricsHandler creates an http.Handler that emits request metrics. func NewRequestMetricsHandler(next http.Handler, - ns, service, config, rev, pod string, securityMode netcfg.Trust) (http.Handler, error) { - keys := []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.RouteTagKey, metrics.SecurityMode} + ns, service, config, rev, pod string) (http.Handler, error) { + keys := []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.RouteTagKey} if err := pkgmetrics.RegisterResourceView( &view.View{ Description: "The number of requests that are routed to queue-proxy", @@ -101,9 +99,8 @@ func NewRequestMetricsHandler(next http.Handler, } return &requestMetricsHandler{ - next: next, - statsCtx: ctx, - securityMode: securityMode, + next: next, + statsCtx: ctx, }, nil } @@ -111,8 +108,6 @@ func (h *requestMetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request rr := pkghttp.NewResponseRecorder(w, http.StatusOK) startTime := time.Now() - ctx, _ := tag.New(h.statsCtx, tag.Upsert(metrics.SecurityMode, string(h.securityMode))) - defer func() { // Filter probe requests for revision metrics. if netheader.IsProbe(r) { @@ -124,13 +119,13 @@ func (h *requestMetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request latency := time.Since(startTime) routeTag := GetRouteTagNameFromRequest(r) if err != nil { - ctx = metrics.AugmentWithResponseAndRouteTag(ctx, + ctx := metrics.AugmentWithResponseAndRouteTag(h.statsCtx, http.StatusInternalServerError, routeTag) pkgmetrics.RecordBatch(ctx, requestCountM.M(1), responseTimeInMsecM.M(float64(latency.Milliseconds()))) panic(err) } - ctx = metrics.AugmentWithResponseAndRouteTag(ctx, + ctx := metrics.AugmentWithResponseAndRouteTag(h.statsCtx, rr.ResponseCode, routeTag) pkgmetrics.RecordBatch(ctx, requestCountM.M(1), responseTimeInMsecM.M(float64(latency.Milliseconds()))) diff --git a/pkg/queue/request_metric_test.go b/pkg/queue/request_metric_test.go index cc9f4b6ea6ae..79bd841d4fdb 100644 --- a/pkg/queue/request_metric_test.go +++ b/pkg/queue/request_metric_test.go @@ -27,7 +27,6 @@ import ( "knative.dev/pkg/metrics/metricstest" "knative.dev/serving/pkg/metrics" - netcfg "knative.dev/networking/pkg/config" _ "knative.dev/pkg/metrics/testing" ) @@ -35,7 +34,7 @@ const targetURI = "http://example.com" func TestNewRequestMetricsHandlerFailure(t *testing.T) { t.Cleanup(reset) - if _, err := NewRequestMetricsHandler(nil /*next*/, "a", "b", "c", "d", "shøüld fail", netcfg.TrustDisabled); err == nil { + if _, err := NewRequestMetricsHandler(nil /*next*/, "a", "b", "c", "d", "shøüld fail"); err == nil { t.Error("Should get error when tag value is not ascii") } } @@ -43,7 +42,7 @@ func TestNewRequestMetricsHandlerFailure(t *testing.T) { func TestRequestMetricsHandler(t *testing.T) { defer reset() baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) + handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") if err != nil { t.Fatal("Failed to create handler:", err) } @@ -58,7 +57,6 @@ func TestRequestMetricsHandler(t *testing.T) { metrics.LabelResponseCode: "200", metrics.LabelResponseCodeClass: "2xx", "route_tag": disabledTagName, - metrics.LabelSecurityMode: string(netcfg.TrustDisabled), } wantResource := &resource.Resource{ Type: "knative_revision", @@ -83,7 +81,7 @@ func TestRequestMetricsHandler(t *testing.T) { func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { defer reset() baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) + handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") if err != nil { t.Fatal("Failed to create handler:", err) } @@ -100,7 +98,6 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { metrics.LabelResponseCode: "200", metrics.LabelResponseCodeClass: "2xx", metrics.LabelRouteTag: "test-tag", - metrics.LabelSecurityMode: string(netcfg.TrustDisabled), } wantResource := &resource.Resource{ Type: "knative_revision", @@ -116,7 +113,7 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { // Testing for default route reset() - handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) + handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") req.Header.Del(netheader.RouteTagKey) req.Header.Set(netheader.DefaultRouteKey, "true") handler.ServeHTTP(resp, req) @@ -124,7 +121,7 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { metricstest.AssertMetric(t, metricstest.IntMetric("request_count", 1, wantTags).WithResource(wantResource)) reset() - handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) + handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") req.Header.Set(netheader.RouteTagKey, "test-tag") req.Header.Set(netheader.DefaultRouteKey, "true") handler.ServeHTTP(resp, req) @@ -132,7 +129,7 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) { metricstest.AssertMetric(t, metricstest.IntMetric("request_count", 1, wantTags).WithResource(wantResource)) reset() - handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) + handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") req.Header.Set(netheader.RouteTagKey, "test-tag") req.Header.Set(netheader.DefaultRouteKey, "false") handler.ServeHTTP(resp, req) @@ -152,7 +149,7 @@ func TestRequestMetricsHandlerPanickingHandler(t *testing.T) { baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { panic("no!") }) - handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) + handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") if err != nil { t.Fatal("Failed to create handler:", err) } @@ -169,7 +166,6 @@ func TestRequestMetricsHandlerPanickingHandler(t *testing.T) { metrics.LabelResponseCode: "500", metrics.LabelResponseCodeClass: "5xx", "route_tag": disabledTagName, - metrics.LabelSecurityMode: string(netcfg.TrustDisabled), } wantResource := &resource.Resource{ Type: "knative_revision", @@ -296,7 +292,7 @@ func TestAppRequestMetricsHandler(t *testing.T) { func BenchmarkRequestMetricsHandler(b *testing.B) { baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - handler, _ := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled) + handler, _ := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod") req := httptest.NewRequest(http.MethodPost, "http://example.com", nil) b.Run("sequential", func(b *testing.B) { diff --git a/pkg/queue/sharedmain/main.go b/pkg/queue/sharedmain/main.go index 952c5189227c..6a38b12d8488 100644 --- a/pkg/queue/sharedmain/main.go +++ b/pkg/queue/sharedmain/main.go @@ -35,7 +35,6 @@ import ( "k8s.io/apimachinery/pkg/types" "knative.dev/networking/pkg/certificates" - netcfg "knative.dev/networking/pkg/config" netstats "knative.dev/networking/pkg/http/stats" pkglogging "knative.dev/pkg/logging" "knative.dev/pkg/logging/logkey" @@ -107,9 +106,6 @@ type config struct { TracingConfigSampleRate float64 `split_words:"true"` // optional TracingConfigZipkinEndpoint string `split_words:"true"` // optional - // SecurityMode is the trust level for internal encryption, see config-networking.data.dataplane-trust - SecurityMode netcfg.Trust `split_words:"true" required:"true"` - Env } @@ -412,7 +408,7 @@ func requestLogHandler(logger *zap.SugaredLogger, currentHandler http.Handler, e func requestMetricsHandler(logger *zap.SugaredLogger, currentHandler http.Handler, env config) http.Handler { h, err := queue.NewRequestMetricsHandler(currentHandler, env.ServingNamespace, - env.ServingService, env.ServingConfiguration, env.ServingRevision, env.ServingPod, env.SecurityMode) + env.ServingService, env.ServingConfiguration, env.ServingRevision, env.ServingPod) if err != nil { logger.Errorw("Error setting up request metrics reporter. Request metrics will be unavailable.", zap.Error(err)) return currentHandler diff --git a/pkg/reconciler/revision/resources/deploy_test.go b/pkg/reconciler/revision/resources/deploy_test.go index 36e4d6ffdfec..a668d5efe1d3 100644 --- a/pkg/reconciler/revision/resources/deploy_test.go +++ b/pkg/reconciler/revision/resources/deploy_test.go @@ -43,7 +43,6 @@ import ( "knative.dev/serving/pkg/deployment" "knative.dev/serving/pkg/queue" - netcfg "knative.dev/networking/pkg/config" _ "knative.dev/pkg/metrics/testing" . "knative.dev/serving/pkg/testing/v1" ) @@ -189,9 +188,6 @@ var ( }, { Name: "ENABLE_HTTP2_AUTO_DETECTION", Value: "false", - }, { - Name: "SECURITY_MODE", - Value: "", }, { Name: "ROOT_CA", Value: "", @@ -543,7 +539,6 @@ func TestMakePodSpec(t *testing.T) { defaults *apicfg.Defaults dc deployment.Config fc apicfg.Features - nc netcfg.Config want *corev1.PodSpec }{{ name: "user-defined user port, queue proxy have PORT env", @@ -1384,7 +1379,6 @@ func TestMakePodSpec(t *testing.T) { cfg.Observability = &test.oc cfg.Deployment = &test.dc cfg.Features = &test.fc - cfg.Network = &test.nc if test.defaults != nil { cfg.Defaults = test.defaults } diff --git a/pkg/reconciler/revision/resources/queue.go b/pkg/reconciler/revision/resources/queue.go index 82494f3ece10..1fb964a53dab 100644 --- a/pkg/reconciler/revision/resources/queue.go +++ b/pkg/reconciler/revision/resources/queue.go @@ -418,9 +418,6 @@ func makeQueueContainer(rev *v1.Revision, cfg *config.Config) (*corev1.Container }, { Name: "ENABLE_HTTP2_AUTO_DETECTION", Value: strconv.FormatBool(cfg.Features.AutoDetectHTTP2 == apicfg.Enabled), - }, { - Name: "SECURITY_MODE", - Value: string(cfg.Network.DataplaneTrust), }, { Name: "ROOT_CA", Value: cfg.Deployment.QueueSidecarRootCA, diff --git a/pkg/reconciler/revision/resources/queue_test.go b/pkg/reconciler/revision/resources/queue_test.go index 776a9d91f601..f8998a549dcb 100644 --- a/pkg/reconciler/revision/resources/queue_test.go +++ b/pkg/reconciler/revision/resources/queue_test.go @@ -417,18 +417,6 @@ func TestMakeQueueContainer(t *testing.T) { "ENABLE_HTTP2_AUTO_DETECTION": "false", }) }), - }, { - name: "SecurityMode set", - rev: revision("bar", "foo", - withContainers(containers)), - nc: netcfg.Config{ - DataplaneTrust: netcfg.TrustEnabled, - }, - want: queueContainer(func(c *corev1.Container) { - c.Env = env(map[string]string{ - "SECURITY_MODE": "enabled", - }) - }), }} for _, test := range tests { @@ -446,7 +434,6 @@ func TestMakeQueueContainer(t *testing.T) { Logging: &test.lc, Observability: &test.oc, Deployment: &test.dc, - Network: &test.nc, Config: &apicfg.Config{ Features: &test.fc, }, @@ -1062,7 +1049,6 @@ var defaultEnv = map[string]string{ "REVISION_TIMEOUT_SECONDS": "45", "REVISION_RESPONSE_START_TIMEOUT_SECONDS": "0", "REVISION_IDLE_TIMEOUT_SECONDS": "0", - "SECURITY_MODE": "", "SERVING_CONFIGURATION": "", "SERVING_ENABLE_PROBE_REQUEST_LOG": "false", "SERVING_ENABLE_REQUEST_LOG": "false", From e71fcba0a767393a634c1c05651aa2c876a707d1 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 15:31:11 -0400 Subject: [PATCH 13/24] address linter suggestions --- test/e2e/internalencryption/README.md | 2 +- .../internalencryption_test.go | 20 ++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/test/e2e/internalencryption/README.md b/test/e2e/internalencryption/README.md index 9aa67f6cb0f2..f446ab5eaecf 100644 --- a/test/e2e/internalencryption/README.md +++ b/test/e2e/internalencryption/README.md @@ -8,4 +8,4 @@ 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: ` appears, which indicates that TLS was not used. If that count is greater than 0, the test will fail. \ No newline at end of file +It counts the lines where `TLS: ` appears, which indicates that TLS was not used. If that count is greater than 0, the test will fail. diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index dff5f75441ef..73a2d483ed7e 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -92,6 +92,8 @@ func TestInternalEncryption(t *testing.T) { t.Fatalf("Got revision label service=%q, want=%q", val, names.Service) } + activatorNoTLSCount, queueNoTLSCount := 0, 0 + // Check on the logs for the activator pods, err := clients.KubeClient.CoreV1().Pods("knative-serving").List(context.TODO(), v1.ListOptions{ LabelSelector: "app=activator", @@ -102,9 +104,9 @@ func TestInternalEncryption(t *testing.T) { activatorPod := pods.Items[0] req := clients.KubeClient.CoreV1().Pods(activatorPod.Namespace).GetLogs(activatorPod.Name, &corev1.PodLogOptions{}) - _, nilCount, err := getPodLogs(req) + _, activatorNoTLSCount, err = getPodLogs(req) - if nilCount > 0 { + if activatorNoTLSCount > 0 { t.Fatal("TLS not used on requests to activator") } @@ -117,9 +119,9 @@ func TestInternalEncryption(t *testing.T) { } helloWorldPod := pods.Items[0] req = clients.KubeClient.CoreV1().Pods(helloWorldPod.Namespace).GetLogs(helloWorldPod.Name, &corev1.PodLogOptions{}) - _, nilCount, err = getPodLogs(req) + _, queueNoTLSCount, err = getPodLogs(req) - if nilCount > 0 { + if queueNoTLSCount > 0 { t.Fatal("TLS not used on requests to queue-proxy") } } @@ -129,7 +131,7 @@ func getPodLogs(req *rest.Request) (tlsCount int, nilCount int, err error) { podLogs, err := req.Stream(context.Background()) if err != nil { - err = fmt.Errorf("Failed to stream activator logs: %v", err) + err = fmt.Errorf("Failed to stream activator logs: %w", err) return } @@ -137,7 +139,7 @@ func getPodLogs(req *rest.Request) (tlsCount int, nilCount int, err error) { _, err = io.Copy(buf, podLogs) podLogs.Close() if err != nil { - err = fmt.Errorf("Failed to read activator logs from buffer: %v", err) + err = fmt.Errorf("Failed to read activator logs from buffer: %w", err) return } @@ -146,16 +148,16 @@ func getPodLogs(req *rest.Request) (tlsCount int, nilCount int, err error) { log.Printf("log: %s", scanner.Text()) if strings.Contains(scanner.Text(), "TLS") { if strings.Contains(scanner.Text(), "TLS: ") { - nilCount += 1 + nilCount++ } else if strings.Contains(scanner.Text(), "TLS: [") { - tlsCount += 1 + tlsCount++ } } } if err = scanner.Err(); err != nil { - err = fmt.Errorf("Failed to scan activator logs: %v", err) + err = fmt.Errorf("Failed to scan activator logs: %w", err) return } From e5b35d2eb6d10fb58b27bfa7cd515fe1d395ee04 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 15:37:22 -0400 Subject: [PATCH 14/24] more lint suggestions --- test/e2e/internalencryption/internalencryption_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index 73a2d483ed7e..f1ec14bd0985 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -106,7 +106,9 @@ func TestInternalEncryption(t *testing.T) { req := clients.KubeClient.CoreV1().Pods(activatorPod.Namespace).GetLogs(activatorPod.Name, &corev1.PodLogOptions{}) _, activatorNoTLSCount, err = getPodLogs(req) - if activatorNoTLSCount > 0 { + if err != nil { + t.Fatalf("Failed checking activator logs: %s", err) + } else if activatorNoTLSCount > 0 { t.Fatal("TLS not used on requests to activator") } @@ -121,7 +123,9 @@ func TestInternalEncryption(t *testing.T) { req = clients.KubeClient.CoreV1().Pods(helloWorldPod.Namespace).GetLogs(helloWorldPod.Name, &corev1.PodLogOptions{}) _, queueNoTLSCount, err = getPodLogs(req) - if queueNoTLSCount > 0 { + if err != nil { + t.Fatalf("Failed checking queue-proxy logs: %s", err) + } else if queueNoTLSCount > 0 { t.Fatal("TLS not used on requests to queue-proxy") } } From c7b730cde1fc0a343fcf832e5297bf27355a1868 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 16:45:46 -0400 Subject: [PATCH 15/24] clean up test --- .../internalencryption_test.go | 50 ++++++++----------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index f1ec14bd0985..33d8a0ed8b51 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -26,7 +26,6 @@ import ( "crypto/tls" "fmt" "io" - "log" "strings" "testing" @@ -37,9 +36,7 @@ import ( netcfg "knative.dev/networking/pkg/config" pkgTest "knative.dev/pkg/test" "knative.dev/pkg/test/spoof" - "knative.dev/serving/pkg/apis/serving" - //. "knative.dev/serving/pkg/testing/v1" "knative.dev/serving/test" v1test "knative.dev/serving/test/v1" ) @@ -71,6 +68,7 @@ func TestInternalEncryption(t *testing.T) { 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 url := resources.Route.Status.URL.URL() if _, err := pkgTest.CheckEndpointState( context.Background(), @@ -84,16 +82,6 @@ func TestInternalEncryption(t *testing.T) { t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.HelloWorldText, err) } - revision := resources.Revision - if val := revision.Labels[serving.ConfigurationLabelKey]; val != names.Config { - t.Fatalf("Got revision label configuration=%q, want=%q ", names.Config, val) - } - if val := revision.Labels[serving.ServiceLabelKey]; val != names.Service { - t.Fatalf("Got revision label service=%q, want=%q", val, names.Service) - } - - activatorNoTLSCount, queueNoTLSCount := 0, 0 - // Check on the logs for the activator pods, err := clients.KubeClient.CoreV1().Pods("knative-serving").List(context.TODO(), v1.ListOptions{ LabelSelector: "app=activator", @@ -104,11 +92,11 @@ func TestInternalEncryption(t *testing.T) { activatorPod := pods.Items[0] req := clients.KubeClient.CoreV1().Pods(activatorPod.Namespace).GetLogs(activatorPod.Name, &corev1.PodLogOptions{}) - _, activatorNoTLSCount, err = getPodLogs(req) + activatorTLSCount, err := scanPodLogs(req, matchTLSLog) if err != nil { t.Fatalf("Failed checking activator logs: %s", err) - } else if activatorNoTLSCount > 0 { + } else if activatorTLSCount == 0 { t.Fatal("TLS not used on requests to activator") } @@ -120,18 +108,17 @@ func TestInternalEncryption(t *testing.T) { t.Fatalf("Failed to get pods: %v", err) } helloWorldPod := pods.Items[0] - req = clients.KubeClient.CoreV1().Pods(helloWorldPod.Namespace).GetLogs(helloWorldPod.Name, &corev1.PodLogOptions{}) - _, queueNoTLSCount, err = getPodLogs(req) + 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 queueNoTLSCount > 0 { + } else if queueTLSCount == 0 { t.Fatal("TLS not used on requests to queue-proxy") } } -func getPodLogs(req *rest.Request) (tlsCount int, nilCount int, err error) { - tlsCount, nilCount = 0, 0 +func scanPodLogs(req *rest.Request, matcher func(string) bool) (matchCount int, err error) { podLogs, err := req.Stream(context.Background()) if err != nil { @@ -149,21 +136,26 @@ func getPodLogs(req *rest.Request) (tlsCount int, nilCount int, err error) { scanner := bufio.NewScanner(buf) for scanner.Scan() { - log.Printf("log: %s", scanner.Text()) - if strings.Contains(scanner.Text(), "TLS") { - if strings.Contains(scanner.Text(), "TLS: ") { - nilCount++ - - } else if strings.Contains(scanner.Text(), "TLS: [") { - tlsCount++ - } + if matcher(scanner.Text()) { + matchCount++ } } if err = scanner.Err(); err != nil { - err = fmt.Errorf("Failed to scan activator logs: %w", err) + 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: ") { + return false + } else if strings.Contains(line, "TLS: [") { + return true + } + } + return false +} From bcb33c9ce0070a87c9031222f24ab4e56a65771e Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 16:58:26 -0400 Subject: [PATCH 16/24] make internal encryption test alpha only * also only run for Contour and Kourier right now --- test/e2e/internalencryption/internalencryption_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index 33d8a0ed8b51..25f70d874843 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -52,6 +52,14 @@ type RequestLog struct { // TestInitContainers tests init containers support. 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) From 4e842b09c29e8c68c5d3743b98b2ce8a62d8680b Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 15 Sep 2023 17:05:19 -0400 Subject: [PATCH 17/24] cleanup leftovers from metrics stuff --- pkg/activator/handler/handler_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/activator/handler/handler_test.go b/pkg/activator/handler/handler_test.go index 42d7d55df8f5..c206b46304dc 100644 --- a/pkg/activator/handler/handler_test.go +++ b/pkg/activator/handler/handler_test.go @@ -326,7 +326,6 @@ func revision(namespace, name string) *v1.Revision { func setupConfigStore(t testing.TB, logger *zap.SugaredLogger) *activatorconfig.Store { configStore := activatorconfig.NewStore(logger) configStore.OnConfigChanged(tracingConfig(false)) - configStore.OnConfigChanged(networkConfig(false)) return configStore } From b1059c2f531b02b13411ab3685d84ddf79b2723e Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Mon, 25 Sep 2023 10:32:37 -0400 Subject: [PATCH 18/24] address nits --- pkg/activator/config/store.go | 4 ++++ test/e2e-tests.sh | 2 +- test/e2e/internalencryption/internalencryption_test.go | 5 ++--- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/activator/config/store.go b/pkg/activator/config/store.go index e155d3e671d8..d0a6056960e0 100644 --- a/pkg/activator/config/store.go +++ b/pkg/activator/config/store.go @@ -58,6 +58,10 @@ func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value i 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() diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index aa8ff7c5dab4..4b0c95848e9a 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -84,7 +84,7 @@ go_test_e2e -timeout=2m ./test/e2e/domainmapping ${E2E_TEST_FLAGS} || failed=1 toggle_feature dataplane-trust 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 the current implementation, Activator is always in the request path, and needs to be restarted after configuring dataplane-trust +# with current implementaiton, 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/internalencryption ${TEST_OPTIONS} || failed=1 toggle_feature dataplane-trust disabled config-network || fail_test diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index 25f70d874843..0095b3f5d400 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -2,7 +2,7 @@ // +build e2e /* -Copyright 2021 The Knative Authors +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. @@ -31,12 +31,10 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/rest" netcfg "knative.dev/networking/pkg/config" pkgTest "knative.dev/pkg/test" "knative.dev/pkg/test/spoof" - "knative.dev/serving/test" v1test "knative.dev/serving/test/v1" ) @@ -86,6 +84,7 @@ func TestInternalEncryption(t *testing.T) { 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) } From a3aad968be0616577c108790a6755e9d99d8cf09 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Mon, 25 Sep 2023 17:18:58 -0400 Subject: [PATCH 19/24] fix typo --- test/e2e-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 4b0c95848e9a..8190fd3e8b0f 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -84,7 +84,7 @@ go_test_e2e -timeout=2m ./test/e2e/domainmapping ${E2E_TEST_FLAGS} || failed=1 toggle_feature dataplane-trust 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 implementaiton, Activator must be restarted when configuring system-internal-tls. See https://github.com/knative/serving/issues/13754 +# 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/internalencryption ${TEST_OPTIONS} || failed=1 toggle_feature dataplane-trust disabled config-network || fail_test From 0ec9af36fd58e45d8f328ca0dcf5191320491915 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Tue, 26 Sep 2023 10:48:46 -0400 Subject: [PATCH 20/24] address PR feedback --- test/e2e-tests.sh | 2 +- test/e2e/internalencryption/internalencryption_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 8190fd3e8b0f..c857ec0a9a81 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -86,7 +86,7 @@ toggle_feature "logging.enable-request-log" true config-observability || fail_te 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/internalencryption ${TEST_OPTIONS} || failed=1 +go_test_e2e -timeout=2m ./test/e2e/internalencryption ${E2E_TEST_FLAGS} || failed=1 toggle_feature dataplane-trust disabled config-network || fail_test toggle_feature enable-request-log false config-observability || fail_test toggle_feature request-log-template '' config-observability || fail_test diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index 0095b3f5d400..7d1f8d98bbcd 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -48,7 +48,7 @@ type RequestLog struct { TLS tls.ConnectionState `json:"tls"` } -// TestInitContainers tests init containers support. +// TestInternalEncrytion tests the TLS connections between system components. func TestInternalEncryption(t *testing.T) { if !test.ServingFlags.EnableAlphaFeatures { t.Skip("Alpha features not enabled") From b43139726b8d95a29c45b144ec7f92935635213b Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Thu, 28 Sep 2023 10:32:51 -0400 Subject: [PATCH 21/24] fix failing test --- test/e2e/internalencryption/internalencryption_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index 7d1f8d98bbcd..fd11603c0d7c 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -33,6 +33,7 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" netcfg "knative.dev/networking/pkg/config" + "knative.dev/pkg/system" pkgTest "knative.dev/pkg/test" "knative.dev/pkg/test/spoof" "knative.dev/serving/test" @@ -90,12 +91,15 @@ func TestInternalEncryption(t *testing.T) { } // Check on the logs for the activator - pods, err := clients.KubeClient.CoreV1().Pods("knative-serving").List(context.TODO(), v1.ListOptions{ + 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{}) @@ -114,6 +118,9 @@ func TestInternalEncryption(t *testing.T) { 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) From 192a5784a37e5d57773d437423bbedcbb8128740 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Mon, 2 Oct 2023 12:56:08 -0400 Subject: [PATCH 22/24] use correct symbol when looking for TLS access log --- test/e2e/internalencryption/internalencryption_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/internalencryption/internalencryption_test.go index fd11603c0d7c..510d33dca5b5 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/internalencryption/internalencryption_test.go @@ -76,6 +76,7 @@ func TestInternalEncryption(t *testing.T) { } //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(), @@ -90,7 +91,7 @@ func TestInternalEncryption(t *testing.T) { t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.HelloWorldText, err) } - // Check on the logs for the activator + t.Log("Checking Activator logs") pods, err := clients.KubeClient.CoreV1().Pods(system.Namespace()).List(context.TODO(), v1.ListOptions{ LabelSelector: "app=activator", }) @@ -111,7 +112,7 @@ func TestInternalEncryption(t *testing.T) { t.Fatal("TLS not used on requests to activator") } - // Check on the logs for the queue-proxy + 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), }) @@ -167,7 +168,7 @@ func matchTLSLog(line string) bool { if strings.Contains(line, "TLS") { if strings.Contains(line, "TLS: ") { return false - } else if strings.Contains(line, "TLS: [") { + } else if strings.Contains(line, "TLS: {") { return true } } From 58256a792a9fd9cdef0e784347805d80dcc182b6 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Wed, 4 Oct 2023 16:18:01 -0400 Subject: [PATCH 23/24] refactor toggle_feature * was having some bash issues with properly interpretting the patch --- test/e2e-common.sh | 3 ++- test/e2e-tests.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/e2e-common.sh b/test/e2e-common.sh index bcdad3c7de57..0f48e4cdc6f9 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -465,7 +465,8 @@ function toggle_feature() { 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 diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index c857ec0a9a81..7b67855fc5e0 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -83,7 +83,7 @@ go_test_e2e -timeout=2m ./test/e2e/domainmapping ${E2E_TEST_FLAGS} || failed=1 toggle_feature dataplane-trust 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 +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/internalencryption ${E2E_TEST_FLAGS} || failed=1 From bf694945f5859f1cc07dd5f8ad0558b82b05b2e7 Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Thu, 5 Oct 2023 11:00:39 -0400 Subject: [PATCH 24/24] use new flag name * we have switched to system-internal-tls --- test/e2e-tests.sh | 8 ++++---- .../README.md | 6 +++--- .../system_internal_tls_test.go} | 13 +------------ 3 files changed, 8 insertions(+), 19 deletions(-) rename test/e2e/{internalencryption => systeminternaltls}/README.md (72%) rename test/e2e/{internalencryption/internalencryption_test.go => systeminternaltls/system_internal_tls_test.go} (94%) diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 7b67855fc5e0..843e38ce8643 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -81,16 +81,16 @@ 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 dataplane-trust enabled config-network || fail_test +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/internalencryption ${E2E_TEST_FLAGS} || failed=1 -toggle_feature dataplane-trust disabled config-network || fail_test +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 dataplane-trust +# 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 diff --git a/test/e2e/internalencryption/README.md b/test/e2e/systeminternaltls/README.md similarity index 72% rename from test/e2e/internalencryption/README.md rename to test/e2e/systeminternaltls/README.md index f446ab5eaecf..9e09f147f5bf 100644 --- a/test/e2e/internalencryption/README.md +++ b/test/e2e/systeminternaltls/README.md @@ -1,8 +1,8 @@ -# Internal Encryption E2E Tests +# System Internal TLS E2E Tests -In order to test Internal Encryption, this test turns enables request logging and sets the request log template to `TLS: {{.Request.TLS}}`. +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 Internal Encryption, and then configure the logging settings. +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. diff --git a/test/e2e/internalencryption/internalencryption_test.go b/test/e2e/systeminternaltls/system_internal_tls_test.go similarity index 94% rename from test/e2e/internalencryption/internalencryption_test.go rename to test/e2e/systeminternaltls/system_internal_tls_test.go index 510d33dca5b5..a41a685a89d6 100644 --- a/test/e2e/internalencryption/internalencryption_test.go +++ b/test/e2e/systeminternaltls/system_internal_tls_test.go @@ -17,13 +17,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -package internalencryption +package systeminternaltls import ( "bufio" "bytes" "context" - "crypto/tls" "fmt" "io" "strings" @@ -32,7 +31,6 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" - netcfg "knative.dev/networking/pkg/config" "knative.dev/pkg/system" pkgTest "knative.dev/pkg/test" "knative.dev/pkg/test/spoof" @@ -40,15 +38,6 @@ import ( v1test "knative.dev/serving/test/v1" ) -var ( - ExpectedSecurityMode = netcfg.TrustEnabled -) - -type RequestLog struct { - RequestURL string `json:"requestUrl"` - TLS tls.ConnectionState `json:"tls"` -} - // TestInternalEncrytion tests the TLS connections between system components. func TestInternalEncryption(t *testing.T) { if !test.ServingFlags.EnableAlphaFeatures {