diff --git a/cmd/cainjector/app/controller.go b/cmd/cainjector/app/controller.go index a242bfc1b..85d870c69 100644 --- a/cmd/cainjector/app/controller.go +++ b/cmd/cainjector/app/controller.go @@ -23,6 +23,7 @@ import ( "net/http" "time" + corev1 "k8s.io/api/core/v1" apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -81,6 +82,46 @@ func Run(opts *config.CAInjectorConfiguration, ctx context.Context) error { ReaderFailOnMissingInformer: true, DefaultNamespaces: defaultNamespaces, }, + Client: client.Options{ + Cache: &client.CacheOptions{ + // Why do we disable the cache for v1.Secret? + // + // 1. To reduce memory use of cainjector, by disabling + // in-memory cache of Secret resources. + // 2. To reduce the load on the K8S API server when + // cainjector starts up, caused by the initial listing of + // Secret resources in the cluster. + // + // Clusters may contain many and / or large Secret + // resources. + // For example OpenShift clusters may have thousands of + // ServiceAccounts and each of these has a Secret with the + // associated token. + // Or where helm is used, there will be large Secret + // resources containing the configuration of each Helm + // deployment. + // + // Ordinarily, the controller-runtime client would implicitly + // initialize a client-go cache which would list every + // Secret, including the entire data of every Secret. + // This initial list operation can place enormous load on + // the K8S API server. + // + // The problem can be alleviated by disabling the implicit cache: + // * Here in the client CacheOptions and, + // * in NewControllerManagedBy.Watches, by supplying the + // builder.OnlyMetadata option. + // + // The disadvantage is that this will cause *increased* + // ongoing load on the K8S API server later, because the + // reconciler for each injectable will GET the source Secret + // directly from the K8S API server every time the + // injectable is reconciled. + DisableFor: []client.Object{ + &corev1.Secret{}, + }, + }, + }, LeaderElection: opts.LeaderElectionConfig.Enabled, LeaderElectionNamespace: opts.LeaderElectionConfig.Namespace, LeaderElectionID: "cert-manager-cainjector-leader-election", diff --git a/pkg/controller/cainjector/indexers.go b/pkg/controller/cainjector/indexers.go index cefb46dc2..b0cc0a5b5 100644 --- a/pkg/controller/cainjector/indexers.go +++ b/pkg/controller/cainjector/indexers.go @@ -20,8 +20,8 @@ import ( "context" "github.com/go-logr/logr" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -48,7 +48,7 @@ const ( func certFromSecretToInjectableMapFuncBuilder(cl client.Reader, log logr.Logger, config setup) handler.MapFunc { return func(ctx context.Context, obj client.Object) []ctrl.Request { secretName := types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()} - certName := owningCertForSecret(obj.(*corev1.Secret)) + certName := owningCertForSecret(obj.(*metav1.PartialObjectMetadata)) if certName == nil { return nil } diff --git a/pkg/controller/cainjector/reconciler.go b/pkg/controller/cainjector/reconciler.go index 2421749eb..fa4d9271b 100644 --- a/pkg/controller/cainjector/reconciler.go +++ b/pkg/controller/cainjector/reconciler.go @@ -22,7 +22,6 @@ import ( "strings" "github.com/go-logr/logr" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -163,16 +162,25 @@ func dropNotFound(err error) error { return err } -// owningCertForSecret gets the name of the owning certificate for a -// given secret, returning nil if no such object exists. -func owningCertForSecret(secret *corev1.Secret) *types.NamespacedName { - val, ok := secret.Annotations[certmanager.CertificateNameKey] +// owningCertForSecret gets the name of the owning certificate for a given +// secret, returning nil if the supplied secret does not have a +// `cert-manager.io/certificate-name` annotation. +// The secret may be a v1.Secret or a v1.PartialObjectMetadata. +// +// NOTE: "owning" here does not mean [ownerReference][1], because +// cert-manager does not set the ownerReference of the Secret, +// unless the [`--enable-certificate-owner-ref` flag is true][2]. +// +// [1]: https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/ +// [2]: https://cert-manager.io/docs/cli/controller/ +func owningCertForSecret(secret client.Object) *types.NamespacedName { + val, ok := secret.GetAnnotations()[certmanager.CertificateNameKey] if !ok { return nil } return &types.NamespacedName{ Name: val, - Namespace: secret.Namespace, + Namespace: secret.GetNamespace(), } } diff --git a/pkg/controller/cainjector/setup.go b/pkg/controller/cainjector/setup.go index 2b51a325e..bf23396bf 100644 --- a/pkg/controller/cainjector/setup.go +++ b/pkg/controller/cainjector/setup.go @@ -165,7 +165,20 @@ func RegisterAllInjectors(ctx context.Context, mgr ctrl.Manager, opts SetupOptio // injectables is here where we define which // objects' events should trigger a reconcile. builder.WithPredicates(predicates)). - Watches(new(corev1.Secret), handler.EnqueueRequestsFromMapFunc(secretForInjectableMapFuncBuilder(mgr.GetClient(), log, setup))) + Watches( + new(corev1.Secret), + handler.EnqueueRequestsFromMapFunc(secretForInjectableMapFuncBuilder(mgr.GetClient(), log, setup)), + // Why do we use builder.OnlyMetadata? + // + // 1. To reduce memory use of cainjector, by only caching the + // metadata of Secrets, not the data. + // 2. To reduce the load on the K8S API server, by only listing + // the metadata of the Secrets when cainjector starts up. + // + // This configuration works in conjunction with the `DisableFor` + // option in the manager client.Options and client.CacheOptions. + builder.OnlyMetadata, + ) if opts.EnableCertificatesDataSource { // Index injectable with a new field. If the injectable's CA is // to be sourced from a Certificate's Secret, the field's value will be the @@ -176,10 +189,15 @@ func RegisterAllInjectors(ctx context.Context, mgr ctrl.Manager, opts SetupOptio err := fmt.Errorf("error making injectable indexable by inject-ca-from path: %w", err) return err } - b.Watches(new(corev1.Secret), handler.EnqueueRequestsFromMapFunc( - certFromSecretToInjectableMapFuncBuilder(mgr.GetClient(), log, setup))). - Watches(new(cmapi.Certificate), - handler.EnqueueRequestsFromMapFunc(certToInjectableMapFuncBuilder(mgr.GetClient(), log, setup))) + b.Watches( + new(corev1.Secret), + handler.EnqueueRequestsFromMapFunc(certFromSecretToInjectableMapFuncBuilder(mgr.GetClient(), log, setup)), + // See "Why do we use builder.OnlyMetadata?" above. + builder.OnlyMetadata, + ).Watches( + new(cmapi.Certificate), + handler.EnqueueRequestsFromMapFunc(certToInjectableMapFuncBuilder(mgr.GetClient(), log, setup)), + ) } if err := b.Complete(r); err != nil { return fmt.Errorf("error registering controller for %s: %w", setup.objType.GetName(), err) diff --git a/pkg/controller/cainjector/sources.go b/pkg/controller/cainjector/sources.go index f84f4eb07..9cf9a1af9 100644 --- a/pkg/controller/cainjector/sources.go +++ b/pkg/controller/cainjector/sources.go @@ -117,6 +117,13 @@ func (c *certificateDataSource) ReadCA(ctx context.Context, log logr.Logger, met // don't requeue if we're just not found, we'll get called when the secret gets created return nil, dropNotFound(err) } + // Only use Secrets that have been created by this Certificate. + // The Secret must have a `cert-manager.io/certificate-name` annotation + // value matching the name of this Certificate.. + // NOTE: "owner" is not the `ownerReference`, because cert-manager does not + // usually set the ownerReference of the Secret. + // TODO: The logged warning below is misleading because it contains the + // ownerReference, which is not the reason for ignoring the Secret. owner := owningCertForSecret(&secret) if owner == nil || *owner != certName { log.V(logf.WarnLevel).Info("refusing to target secret not owned by certificate", "owner", metav1.GetControllerOf(&secret))