From fca1a70c8d60b7042e205064d1f80d6916beeb30 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 28 Mar 2022 13:59:17 +0900 Subject: [PATCH] Force activator always in path when activator CA is specified As described in the feature doc of https://github.com/knative/serving/issues/11906, activator needs to serve TLS traffic for the traffic from ingress. So, this patch force SKS proxy mode when `activator-ca` in `config-network` is specified. --- pkg/reconciler/autoscaling/config/store.go | 4 + .../config/testdata/config-network.yaml | 1 + pkg/reconciler/autoscaling/kpa/kpa.go | 6 +- pkg/reconciler/autoscaling/kpa/kpa_test.go | 64 +++++++++++++ .../networking/pkg/apis/config/defaults.go | 73 +++++++++++++++ .../networking/pkg/apis/config/doc.go | 21 +++++ .../networking/pkg/apis/config/store.go | 92 +++++++++++++++++++ .../pkg/apis/config/zz_generated.deepcopy.go | 38 ++++++++ vendor/modules.txt | 1 + 9 files changed, 299 insertions(+), 1 deletion(-) create mode 120000 pkg/reconciler/autoscaling/config/testdata/config-network.yaml create mode 100644 vendor/knative.dev/networking/pkg/apis/config/defaults.go create mode 100644 vendor/knative.dev/networking/pkg/apis/config/doc.go create mode 100644 vendor/knative.dev/networking/pkg/apis/config/store.go create mode 100644 vendor/knative.dev/networking/pkg/apis/config/zz_generated.deepcopy.go diff --git a/pkg/reconciler/autoscaling/config/store.go b/pkg/reconciler/autoscaling/config/store.go index 7a9c91ccf926..2f30e8ec6377 100644 --- a/pkg/reconciler/autoscaling/config/store.go +++ b/pkg/reconciler/autoscaling/config/store.go @@ -19,6 +19,7 @@ package config import ( "context" + network "knative.dev/networking/pkg" "knative.dev/pkg/configmap" asconfig "knative.dev/serving/pkg/autoscaler/config" "knative.dev/serving/pkg/autoscaler/config/autoscalerconfig" @@ -31,6 +32,7 @@ type cfgKey struct{} type Config struct { Autoscaler *autoscalerconfig.Config Deployment *deployment.Config + Network *network.Config } // FromContext fetch config from context. @@ -65,6 +67,7 @@ func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value i configmap.Constructors{ asconfig.ConfigName: asconfig.NewConfigFromConfigMap, deployment.ConfigName: deployment.NewConfigFromConfigMap, + network.ConfigName: network.NewConfigFromConfigMap, }, onAfterStore..., ), @@ -82,5 +85,6 @@ func (s *Store) Load() *Config { return &Config{ Autoscaler: s.UntypedLoad(asconfig.ConfigName).(*autoscalerconfig.Config).DeepCopy(), Deployment: s.UntypedLoad(deployment.ConfigName).(*deployment.Config).DeepCopy(), + Network: s.UntypedLoad(network.ConfigName).(*network.Config).DeepCopy(), } } diff --git a/pkg/reconciler/autoscaling/config/testdata/config-network.yaml b/pkg/reconciler/autoscaling/config/testdata/config-network.yaml new file mode 120000 index 000000000000..56cb332a0426 --- /dev/null +++ b/pkg/reconciler/autoscaling/config/testdata/config-network.yaml @@ -0,0 +1 @@ +../../../../../config/core/configmaps/network.yaml \ No newline at end of file diff --git a/pkg/reconciler/autoscaling/kpa/kpa.go b/pkg/reconciler/autoscaling/kpa/kpa.go index 432abbc1b061..be20002fc706 100644 --- a/pkg/reconciler/autoscaling/kpa/kpa.go +++ b/pkg/reconciler/autoscaling/kpa/kpa.go @@ -131,7 +131,11 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pa *autoscalingv1alpha1. // this revision, e.g. after a restart) but PA status is inactive (it was // already scaled to 0). // 2. The excess burst capacity is negative. - if want == 0 || decider.Status.ExcessBurstCapacity < 0 || want == scaleUnknown && pa.Status.IsInactive() { + if len(config.FromContext(ctx).Network.ActivatorCA) > 0 { + // When activator CA is enabled, foce activator always in path. + // See: https://github.com/knative/serving/issues/11906 + mode = nv1alpha1.SKSOperationModeProxy + } else if want == 0 || decider.Status.ExcessBurstCapacity < 0 || want == scaleUnknown && pa.Status.IsInactive() { mode = nv1alpha1.SKSOperationModeProxy } diff --git a/pkg/reconciler/autoscaling/kpa/kpa_test.go b/pkg/reconciler/autoscaling/kpa/kpa_test.go index 1bf8f26a3060..23b315b5fce0 100644 --- a/pkg/reconciler/autoscaling/kpa/kpa_test.go +++ b/pkg/reconciler/autoscaling/kpa/kpa_test.go @@ -61,6 +61,7 @@ import ( "go.uber.org/atomic" "golang.org/x/sync/errgroup" + netpkg "knative.dev/networking/pkg" nv1a1 "knative.dev/networking/pkg/apis/networking/v1alpha1" duckv1 "knative.dev/pkg/apis/duck/v1" "knative.dev/pkg/configmap" @@ -125,15 +126,29 @@ func initialScaleZeroASConfig() *autoscalerconfig.Config { return ac } +func activatorCertsNetConfig() *netpkg.Config { + nc, _ := netpkg.NewConfigFromMap(map[string]string{ + netpkg.ActivatorCAKey: "knative-ca", + netpkg.ActivatorSANKey: "knative-san", + }) + return nc +} + func defaultConfig() *config.Config { ac, _ := asconfig.NewConfigFromMap(defaultConfigMapData()) deploymentConfig, _ := deployment.NewConfigFromMap(map[string]string{ deployment.QueueSidecarImageKey: "bob", deployment.ProgressDeadlineKey: progressDeadline.String(), }) + networkConfig, _ := netpkg.NewConfigFromMap(map[string]string{ + netpkg.ActivatorCAKey: "", + netpkg.ActivatorSANKey: "", + }) + return &config.Config{ Autoscaler: ac, Deployment: deploymentConfig, + Network: networkConfig, } } @@ -152,6 +167,12 @@ func newConfigWatcher() configmap.Watcher { Data: map[string]string{ deployment.QueueSidecarImageKey: "covid is here", }, + }, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: netpkg.ConfigName, + }, + Data: map[string]string{}, }) } @@ -293,6 +314,7 @@ func TestReconcile(t *testing.T) { type deciderKey struct{} type asConfigKey struct{} + type netConfigKey struct{} retryAttempted := false @@ -1120,6 +1142,38 @@ func TestReconcile(t *testing.T) { WithPAMetricsService(privateSvc), WithObservedGeneration(1), ), }}, + }, { + Name: "we have enough burst capacity, but keep proxy mode as activator CA is enabled", + Key: key, + Ctx: context.WithValue(context.WithValue(context.Background(), netConfigKey{}, activatorCertsNetConfig()), deciderKey{}, + decider(testNamespace, testRevision, defaultScale, /* desiredScale */ + 1 /* ebc */)), + Objects: []runtime.Object{ + kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, + WithPAMetricsService(privateSvc), withScales(1, defaultScale), + WithPAStatusService(testRevision), WithObservedGeneration(1)), + defaultProxySKS, + metric(testNamespace, testRevision), + defaultDeployment, + defaultReady}, + // No update from ProxySKS. + }, { + Name: "we have enough burst capacity, but switch to keep proxy mode as activator CA is turned on", + Key: key, + Ctx: context.WithValue(context.WithValue(context.Background(), netConfigKey{}, activatorCertsNetConfig()), deciderKey{}, + decider(testNamespace, testRevision, defaultScale, /* desiredScale */ + 1 /* ebc */)), + Objects: []runtime.Object{ + kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, + WithPAMetricsService(privateSvc), withScales(1, defaultScale), + WithPAStatusService(testRevision), WithObservedGeneration(1)), + defaultSKS, + metric(testNamespace, testRevision), + defaultDeployment, + defaultReady}, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: defaultProxySKS, + }}, }} table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { @@ -1145,6 +1199,9 @@ func TestReconcile(t *testing.T) { if asConfig := ctx.Value(asConfigKey{}); asConfig != nil { testConfigs.Autoscaler = asConfig.(*autoscalerconfig.Config) } + if netConfig := ctx.Value(netConfigKey{}); netConfig != nil { + testConfigs.Network = netConfig.(*netpkg.Config) + } psf := podscalable.Get(ctx) scaler := newScaler(ctx, psf, func(interface{}, time.Duration) {}) scaler.activatorProbe = func(*autoscalingv1alpha1.PodAutoscaler, http.RoundTripper) (bool, error) { return true, nil } @@ -1219,6 +1276,13 @@ func TestGlobalResyncOnUpdateAutoscalerConfigMap(t *testing.T) { deployment.QueueSidecarImageKey: "i'm on a bike", }, }) + watcher.OnChange(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: netpkg.ConfigName, + Namespace: system.Namespace(), + }, + Data: map[string]string{}, + }) grp := errgroup.Group{} waitInformers, err := RunAndSyncInformers(ctx, informers...) diff --git a/vendor/knative.dev/networking/pkg/apis/config/defaults.go b/vendor/knative.dev/networking/pkg/apis/config/defaults.go new file mode 100644 index 000000000000..18517f1c2f51 --- /dev/null +++ b/vendor/knative.dev/networking/pkg/apis/config/defaults.go @@ -0,0 +1,73 @@ +/* +Copyright 2020 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 + + https://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 config + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + + cm "knative.dev/pkg/configmap" +) + +const ( + // DefaultsConfigName is the name of config map for the defaults. + DefaultsConfigName = "config-defaults" + + // DefaultRevisionTimeoutSeconds will be set if timeoutSeconds not specified. + DefaultRevisionTimeoutSeconds = 5 * 60 + + // DefaultMaxRevisionTimeoutSeconds will be set if MaxRevisionTimeoutSeconds is not specified. + DefaultMaxRevisionTimeoutSeconds = 10 * 60 +) + +func defaultConfig() *Defaults { + return &Defaults{ + RevisionTimeoutSeconds: DefaultRevisionTimeoutSeconds, + MaxRevisionTimeoutSeconds: DefaultMaxRevisionTimeoutSeconds, + } +} + +// NewDefaultsConfigFromMap creates a Defaults from the supplied Map. +func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) { + nc := defaultConfig() + + if err := cm.Parse(data, + cm.AsInt64("revision-timeout-seconds", &nc.RevisionTimeoutSeconds), + cm.AsInt64("max-revision-timeout-seconds", &nc.MaxRevisionTimeoutSeconds), + ); err != nil { + return nil, err + } + + if nc.RevisionTimeoutSeconds > nc.MaxRevisionTimeoutSeconds { + return nil, fmt.Errorf("revision-timeout-seconds (%d) cannot be greater than max-revision-timeout-seconds (%d)", nc.RevisionTimeoutSeconds, nc.MaxRevisionTimeoutSeconds) + } + return nc, nil +} + +// NewDefaultsConfigFromConfigMap creates a Defaults from the supplied configMap. +func NewDefaultsConfigFromConfigMap(config *corev1.ConfigMap) (*Defaults, error) { + return NewDefaultsConfigFromMap(config.Data) +} + +// Defaults includes the default values to be populated by the webhook. +type Defaults struct { + RevisionTimeoutSeconds int64 + // This is the timeout set for ingress. + // RevisionTimeoutSeconds must be less than this value. + MaxRevisionTimeoutSeconds int64 +} diff --git a/vendor/knative.dev/networking/pkg/apis/config/doc.go b/vendor/knative.dev/networking/pkg/apis/config/doc.go new file mode 100644 index 000000000000..a0d4319efccf --- /dev/null +++ b/vendor/knative.dev/networking/pkg/apis/config/doc.go @@ -0,0 +1,21 @@ +/* +Copyright 2020 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. +*/ + +// +k8s:deepcopy-gen=package + +// Package config holds the typed objects that define the schemas for +// ConfigMap objects that pertain to our API objects. +package config diff --git a/vendor/knative.dev/networking/pkg/apis/config/store.go b/vendor/knative.dev/networking/pkg/apis/config/store.go new file mode 100644 index 000000000000..a0dfc8645828 --- /dev/null +++ b/vendor/knative.dev/networking/pkg/apis/config/store.go @@ -0,0 +1,92 @@ +/* +Copyright 2020 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 config + +import ( + "context" + + "knative.dev/pkg/configmap" +) + +type cfgKey struct{} + +// Config holds the collection of configurations that we attach to contexts. +// +k8s:deepcopy-gen=false +type Config struct { + Defaults *Defaults +} + +// FromContext extracts a Config from the provided context. +func FromContext(ctx context.Context) *Config { + x, ok := ctx.Value(cfgKey{}).(*Config) + if ok { + return x + } + return nil +} + +// FromContextOrDefaults is like FromContext, but when no Config is attached it +// returns a Config populated with the defaults for each of the Config fields. +func FromContextOrDefaults(ctx context.Context) *Config { + if cfg := FromContext(ctx); cfg != nil { + return cfg + } + defaults, _ := NewDefaultsConfigFromMap(map[string]string{}) + return &Config{ + Defaults: defaults, + } +} + +// ToContext attaches the provided Config to the provided context, returning the +// new context with the Config attached. +func ToContext(ctx context.Context, c *Config) context.Context { + return context.WithValue(ctx, cfgKey{}, c) +} + +// Store is a typed wrapper around configmap.Untyped store to handle our configmaps. +// +k8s:deepcopy-gen=false +type Store struct { + *configmap.UntypedStore +} + +// NewStore creates a new store of Configs and optionally calls functions when ConfigMaps are updated. +func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value interface{})) *Store { + store := &Store{ + UntypedStore: configmap.NewUntypedStore( + "defaults", + logger, + configmap.Constructors{ + DefaultsConfigName: NewDefaultsConfigFromConfigMap, + }, + onAfterStore..., + ), + } + + return store +} + +// ToContext attaches the current Config state to the provided context. +func (s *Store) ToContext(ctx context.Context) context.Context { + return ToContext(ctx, s.Load()) +} + +// Load creates a Config from the current config state of the Store. +func (s *Store) Load() *Config { + return &Config{ + Defaults: s.UntypedLoad(DefaultsConfigName).(*Defaults).DeepCopy(), + } +} diff --git a/vendor/knative.dev/networking/pkg/apis/config/zz_generated.deepcopy.go b/vendor/knative.dev/networking/pkg/apis/config/zz_generated.deepcopy.go new file mode 100644 index 000000000000..b88db63a1d17 --- /dev/null +++ b/vendor/knative.dev/networking/pkg/apis/config/zz_generated.deepcopy.go @@ -0,0 +1,38 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* +Copyright 2020 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. +*/ + +// Code generated by deepcopy-gen. DO NOT EDIT. + +package config + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Defaults) DeepCopyInto(out *Defaults) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Defaults. +func (in *Defaults) DeepCopy() *Defaults { + if in == nil { + return nil + } + out := new(Defaults) + in.DeepCopyInto(out) + return out +} diff --git a/vendor/modules.txt b/vendor/modules.txt index d952af5c11a2..ee1a4d5fc0b0 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1070,6 +1070,7 @@ knative.dev/hack/shell ## explicit knative.dev/networking/config knative.dev/networking/pkg +knative.dev/networking/pkg/apis/config knative.dev/networking/pkg/apis/networking knative.dev/networking/pkg/apis/networking/v1alpha1 knative.dev/networking/pkg/client/clientset/versioned