Force activator always in path when activator CA is specified (#12790)

* Force activator always in path when activator CA is specified

As described in the feature doc of #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.

* Fix typo

* Use switch

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(),
 	}
 }

pkg/reconciler/autoscaling/config/store_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	logtesting "knative.dev/pkg/logging/testing"
 
+	network "knative.dev/networking/pkg"
 	. "knative.dev/pkg/configmap/testing"
 	autoscalerconfig "knative.dev/serving/pkg/autoscaler/config"
 	"knative.dev/serving/pkg/deployment"
@@ -34,8 +35,10 @@ func TestStoreLoadWithContext(t *testing.T) {
 
 	autoscalerConfig := ConfigMapFromTestFile(t, autoscalerconfig.ConfigName)
 	depConfig := ConfigMapFromTestFile(t, deployment.ConfigName, deployment.DeprecatedQueueSidecarImageKey)
+	netConfig := ConfigMapFromTestFile(t, network.ConfigName)
 	store.OnConfigChanged(autoscalerConfig)
 	store.OnConfigChanged(depConfig)
+	store.OnConfigChanged(netConfig)
 	config := FromContext(store.ToContext(context.Background()))
 
 	wantAS, _ := autoscalerconfig.NewConfigFromConfigMap(autoscalerConfig)
@@ -46,6 +49,10 @@ func TestStoreLoadWithContext(t *testing.T) {
 	if !cmp.Equal(wantD, config.Deployment) {
 		t.Error("Deployment ConfigMap mismatch (-want, +got):", cmp.Diff(wantD, config.Deployment))
 	}
+	wantNet, _ := network.NewConfigFromConfigMap(netConfig)
+	if !cmp.Equal(wantNet, config.Network) {
+		t.Error("Network ConfigMap mismatch (-want, +got):", cmp.Diff(wantNet, config.Network))
+	}
 }
 
 func TestStoreImmutableConfig(t *testing.T) {
@@ -54,16 +61,22 @@ func TestStoreImmutableConfig(t *testing.T) {
 	store.OnConfigChanged(ConfigMapFromTestFile(t, autoscalerconfig.ConfigName))
 	store.OnConfigChanged(ConfigMapFromTestFile(t, deployment.ConfigName,
 		deployment.DeprecatedQueueSidecarImageKey))
+	store.OnConfigChanged(ConfigMapFromTestFile(t, network.ConfigName))
 
 	config := store.Load()
 	config.Autoscaler.MaxScaleUpRate = 100.0
 	config.Deployment.ProgressDeadline = 3 * time.Minute
+	config.Network.ActivatorCA = "activator-ca"
+	config.Network.ActivatorSAN = "activator-san"
 	newConfig := store.Load()
 
 	if newConfig.Autoscaler.MaxScaleUpRate == 100.0 {
 		t.Error("Autoscaler config is not immuable")
 	}
 	if newConfig.Deployment.ProgressDeadline == 3*time.Minute {
-		t.Error("Autoscaler config is not immuable")
+		t.Error("Deployment config is not immuable")
+	}
+	if newConfig.Network.ActivatorCA == "activator-ca" || newConfig.Network.ActivatorSAN == "activator-san" {
+		t.Error("Network config is not immuable")
 	}
 }

pkg/reconciler/autoscaling/config/testdata/config-network.yaml

@@ -0,0 +1 @@
+../../../../../config/core/configmaps/network.yaml

pkg/reconciler/autoscaling/hpa/hpa_test.go

@@ -45,6 +45,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	ktesting "k8s.io/client-go/testing"
 
+	netpkg "knative.dev/networking/pkg"
 	"knative.dev/networking/pkg/apis/networking"
 	nv1a1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
 	"knative.dev/pkg/configmap"
@@ -93,6 +94,13 @@ func TestControllerCanReconcile(t *testing.T) {
 			Data: map[string]string{
 				deployment.QueueSidecarImageKey: "motorbike-sidecar",
 			},
+		},
+		&corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: system.Namespace(),
+				Name:      netpkg.ConfigName,
+			},
+			Data: map[string]string{},
 		}))
 
 	waitInformers, err := RunAndSyncInformers(ctx, infs...)

pkg/reconciler/autoscaling/kpa/kpa.go

@@ -124,14 +124,20 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pa *autoscalingv1alpha1.
 	}
 
 	mode := nv1alpha1.SKSOperationModeServe
+	switch {
+	// When activator CA is enabled, force activator always in path.
+	// TODO: This is a temporary state and to be fixed.
+	// See also issues/11906 and issues/12797.
+	case len(config.FromContext(ctx).Network.ActivatorCA) > 0:
+		mode = nv1alpha1.SKSOperationModeProxy
 	// We put activator in the serving path in the following cases:
 	// 1. The revision is scaled to 0:
 	//   a. want == 0
 	//   b. want == -1 && PA is inactive (Autoscaler has no previous knowledge of
 	//			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() {
+	case want == 0 || decider.Status.ExcessBurstCapacity < 0 || want == scaleUnknown && pa.Status.IsInactive():
 		mode = nv1alpha1.SKSOperationModeProxy
 	}
 

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...)