Add filtering support for secret informer in ingress controller (#920)

* add filtering support for secret informer

* updates

* use the new key

* revert githubflow

* use networking with new key

* minor updates

* updates

* cleanup tests

cmd/controller/main.go

@@ -18,8 +18,10 @@ package main
 
 import (
  "istio.io/api/networking/v1beta1"
+ "knative.dev/net-istio/pkg/reconciler/informerfiltering"
  "knative.dev/net-istio/pkg/reconciler/ingress"
  "knative.dev/net-istio/pkg/reconciler/serverlessservice"
+ "knative.dev/pkg/signals"
 
  // This defines the shared main for injected controllers.
  "knative.dev/pkg/injection/sharedmain"
@@ -31,5 +33,6 @@ func main() {
  v1beta1.VirtualServiceUnmarshaler.AllowUnknownFields = true
  v1beta1.GatewayUnmarshaler.AllowUnknownFields = true
 
- sharedmain.Main("net-istio-controller", ingress.NewController, serverlessservice.NewController)
+ ctx := informerfiltering.GetContextWithFilteringLabelSelector(signals.NewContext())
+ sharedmain.MainWithContext(ctx, "net-istio-controller", ingress.NewController, serverlessservice.NewController)
 }

config/500-controller.yaml

@@ -64,6 +64,8 @@ spec:
  value: config-logging
  - name: CONFIG_OBSERVABILITY_NAME
  value: config-observability
+ - name: ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UID
+ value: "false"
 
  # TODO(https://github.com/knative/pkg/pull/953): Remove stackdriver specific config
  - name: METRICS_DOMAIN

pkg/reconciler/informerfiltering/util.go

@@ -0,0 +1,47 @@
+/*
+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.
+*/
+
+package informerfiltering
+
+import (
+ "context"
+ "os"
+ "strconv"
+
+ "knative.dev/networking/pkg/apis/networking"
+ filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
+)
+
+const EnableSecretInformerFilteringByCertUIDEnv = "ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UID"
+
+// ShouldFilterByCertificateUID allows to choose whether to apply filtering on certificate related secrets
+// when list by informers in this component. If not set or set to false no filtering is applied and instead informers
+// will get any secret available in the cluster which may lead to mem issues in large clusters.
+func ShouldFilterByCertificateUID() bool {
+ if enable := os.Getenv(EnableSecretInformerFilteringByCertUIDEnv); enable != "" {
+ b, _ := strconv.ParseBool(enable)
+ return b
+ }
+ return false
+}
+
+// GetContextWithFilteringLabelSelector returns the passed context with the proper label key selector added to it.
+func GetContextWithFilteringLabelSelector(ctx context.Context) context.Context {
+ if ShouldFilterByCertificateUID() {
+ return filteredFactory.WithSelectors(ctx, networking.CertificateUIDLabelKey)
+ }
+ return filteredFactory.WithSelectors(ctx, "") // Allow all
+}

pkg/reconciler/ingress/controller.go

@@ -20,6 +20,7 @@ import (
  "context"
 
  "go.uber.org/zap"
+ v1 "k8s.io/client-go/informers/core/v1"
  istioclient "knative.dev/net-istio/pkg/client/istio/injection/client"
  gatewayinformer "knative.dev/net-istio/pkg/client/istio/injection/informers/networking/v1alpha3/gateway"
  virtualserviceinformer "knative.dev/net-istio/pkg/client/istio/injection/informers/networking/v1alpha3/virtualservice"
@@ -33,8 +34,9 @@ import (
  kubeclient "knative.dev/pkg/client/injection/kube/client"
  endpointsinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/endpoints"
  podinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/pod"
- secretinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/secret"
+ secretfilteredinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/secret/filtered"
  serviceinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/service"
+ filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
  "knative.dev/pkg/configmap"
  "knative.dev/pkg/controller"
  "knative.dev/pkg/logging"
@@ -81,7 +83,7 @@ func newControllerWithOptions(
  logger := logging.FromContext(ctx)
  virtualServiceInformer := virtualserviceinformer.Get(ctx)
  gatewayInformer := gatewayinformer.Get(ctx)
- secretInformer := secretinformer.Get(ctx)
+ secretInformer := getSecretInformer(ctx)
  serviceInformer := serviceinformer.Get(ctx)
  ingressInformer := ingressinformer.Get(ctx)
 
@@ -179,3 +181,8 @@ func combineFunc(functions ...func(interface{})) func(interface{}) {
  }
  }
 }
+
+func getSecretInformer(ctx context.Context) v1.SecretInformer {
+ untyped := ctx.Value(filteredFactory.LabelKey{}) // This should always be not nil and have exactly one selector
+ return secretfilteredinformer.Get(ctx, untyped.([]string)[0])
+}

pkg/reconciler/ingress/ingress_test.go

@@ -38,8 +38,10 @@ import (
  fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake"
  _ "knative.dev/pkg/client/injection/kube/informers/core/v1/endpoints/fake"
  _ "knative.dev/pkg/client/injection/kube/informers/core/v1/pod/fake"
- _ "knative.dev/pkg/client/injection/kube/informers/core/v1/secret/fake"
+ _ "knative.dev/pkg/client/injection/kube/informers/core/v1/secret/filtered/fake"
  _ "knative.dev/pkg/client/injection/kube/informers/core/v1/service/fake"
+ filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
+ _ "knative.dev/pkg/client/injection/kube/informers/factory/filtered/fake"
 
  proto "github.com/gogo/protobuf/proto"
  "github.com/google/go-cmp/cmp"
@@ -981,8 +983,9 @@ func TestReconcile_EnableAutoTLS(t *testing.T) {
 
  // The secret copy under istio-system.
  targetSecret("istio-system", targetSecretName, map[string]string{
- networking.OriginSecretNameLabelKey: "secret0",
- networking.OriginSecretNamespaceLabelKey: "knative-serving",
+ "networking.internal.knative.dev/certificate-uid": "",
+ networking.OriginSecretNameLabelKey: "secret0",
+ networking.OriginSecretNamespaceLabelKey: "knative-serving",
  }),
  },
  WantPatches: []clientgotesting.PatchActionImpl{
@@ -1573,7 +1576,9 @@ func newTestSetup(t *testing.T, configs ...*corev1.ConfigMap) (
  *controller.Impl,
  *configmap.ManualWatcher) {
 
- ctx, cancel, informers := SetupFakeContextWithCancel(t)
+ ctx, cancel, informers := SetupFakeContextWithCancel(t, func(ctx context.Context) context.Context {
+ return filteredFactory.WithSelectors(ctx, networking.CertificateUIDLabelKey)
+ })
  configMapWatcher := &configmap.ManualWatcher{Namespace: system.Namespace()}
 
  controller := newControllerWithOptions(ctx,

pkg/reconciler/ingress/resources/secret.go

@@ -97,6 +97,8 @@ func targetWildcardSecretName(originSecretName, originSecretNamespace string) st
 }
 
 func makeSecret(originSecret *corev1.Secret, name, namespace string, labels map[string]string) *corev1.Secret {
+ labels[networking.CertificateUIDLabelKey] = originSecret.Labels[networking.CertificateUIDLabelKey] // propagate label for informer use
+
  return &corev1.Secret{
  ObjectMeta: metav1.ObjectMeta{
  Name: name,

pkg/reconciler/ingress/resources/secret_test.go

@@ -159,8 +159,9 @@ func TestMakeSecrets(t *testing.T) {
  // the ns of Istio gateway service.
  Namespace: "istio-system",
  Labels: map[string]string{
- networking.OriginSecretNameLabelKey: "test-secret",
- networking.OriginSecretNamespaceLabelKey: "knative-serving",
+ "networking.internal.knative.dev/certificate-uid": "",
+ networking.OriginSecretNameLabelKey: "test-secret",
+ networking.OriginSecretNamespaceLabelKey: "knative-serving",
  },
  },
  Data: map[string][]byte{
@@ -230,7 +231,7 @@ func TestMakeWildcardSecrets(t *testing.T) {
  // Expected secret should be in istio-system which is
  // the ns of Istio gateway service.
  Namespace: "istio-system",
- Labels: map[string]string{},
+ Labels: map[string]string{"networking.internal.knative.dev/certificate-uid": ""},
  },
  Data: map[string][]byte{
  "test-data": []byte("abcd"),