From ff800307374aff81d69b16f7e2d13c78a542c845 Mon Sep 17 00:00:00 2001 From: irbekrm Date: Thu, 5 Jan 2023 18:15:19 +0000 Subject: [PATCH] Log error if CA source is in a namespace that is not in scope cainjector will still watch cluster-scoped resources such as CRDs, so it can get references to Secrets or Certificates in namespaces that are out of scope Signed-off-by: irbekrm --- pkg/controller/cainjector/controller.go | 10 ++++++++- pkg/controller/cainjector/setup.go | 9 +++++--- pkg/controller/cainjector/sources.go | 28 +++++++++++++++++++------ 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/pkg/controller/cainjector/controller.go b/pkg/controller/cainjector/controller.go index 1bcfc4ec3..44c063afb 100644 --- a/pkg/controller/cainjector/controller.go +++ b/pkg/controller/cainjector/controller.go @@ -106,6 +106,9 @@ type genericInjectReconciler struct { log logr.Logger client.Client + // if set, the reconciler is namespace scoped + namespace string + resourceName string // just used for logging } @@ -157,11 +160,16 @@ func (r *genericInjectReconciler) Reconcile(_ context.Context, req ctrl.Request) return ctrl.Result{}, nil } - caData, err := dataSource.ReadCA(ctx, log, metaObj) + caData, err := dataSource.ReadCA(ctx, log, metaObj, r.namespace) + if apierrors.IsForbidden(err) { + log.V(logf.InfoLevel).Info("cainjector was forbidden to retrieve the ca data source") + return ctrl.Result{}, nil + } if err != nil { log.Error(err, "failed to read CA from data source") return ctrl.Result{}, err } + if caData == nil { log.V(logf.InfoLevel).Info("could not find any ca data in data source for target") return ctrl.Result{}, nil diff --git a/pkg/controller/cainjector/setup.go b/pkg/controller/cainjector/setup.go index 52f200640..87892a749 100644 --- a/pkg/controller/cainjector/setup.go +++ b/pkg/controller/cainjector/setup.go @@ -77,10 +77,10 @@ var ( // registerAllInjectors registers all injectors and based on the // graduation state of the injector decides how to log no kind/resource match errors -func registerAllInjectors(ctx context.Context, groupName string, mgr ctrl.Manager, sources []caDataSource, client client.Client, ca cache.Cache) error { +func registerAllInjectors(ctx context.Context, groupName string, mgr ctrl.Manager, sources []caDataSource, client client.Client, ca cache.Cache, namespace string) error { controllers := make([]controller.Controller, len(injectorSetups)) for i, setup := range injectorSetups { - controller, err := newGenericInjectionController(ctx, groupName, mgr, setup, sources, ca, client) + controller, err := newGenericInjectionController(ctx, groupName, mgr, setup, sources, ca, client, namespace) if err != nil { if !meta.IsNoMatchError(err) || !setup.injector.IsAlpha() { return err @@ -126,7 +126,7 @@ func registerAllInjectors(ctx context.Context, groupName string, mgr ctrl.Manage // * https://github.com/kubernetes-sigs/controller-runtime/issues/764 func newGenericInjectionController(ctx context.Context, groupName string, mgr ctrl.Manager, setup injectorSetup, sources []caDataSource, ca cache.Cache, - client client.Client) (controller.Controller, error) { + client client.Client, namespace string) (controller.Controller, error) { log := ctrl.Log.WithName(groupName).WithName(setup.resourceName) typ := setup.injector.NewTarget().AsObject() @@ -140,6 +140,7 @@ func newGenericInjectionController(ctx context.Context, groupName string, mgr ct log: log.WithName("generic-inject-reconciler"), resourceName: setup.resourceName, injector: setup.injector, + namespace: namespace, }, LogConstructor: func(request *reconcile.Request) logr.Logger { return log }, }) @@ -194,6 +195,7 @@ func RegisterCertificateBased(ctx context.Context, mgr ctrl.Manager, namespace s }, client, cache, + namespace, ) } @@ -217,6 +219,7 @@ func RegisterSecretBased(ctx context.Context, mgr ctrl.Manager, namespace string }, client, cache, + namespace, ) } diff --git a/pkg/controller/cainjector/sources.go b/pkg/controller/cainjector/sources.go index 044d667af..8eb2e8080 100644 --- a/pkg/controller/cainjector/sources.go +++ b/pkg/controller/cainjector/sources.go @@ -18,11 +18,11 @@ package cainjector import ( "context" - - logf "github.com/cert-manager/cert-manager/pkg/logs" + "fmt" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -34,6 +34,7 @@ import ( cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + logf "github.com/cert-manager/cert-manager/pkg/logs" ) // caDataSource knows how to extract CA data given a provided InjectTarget. @@ -52,7 +53,7 @@ type caDataSource interface { // In this case, the caller should not retry the operation. // It is up to the ReadCA implementation to inform the user why the CA // failed to read. - ReadCA(ctx context.Context, log logr.Logger, metaObj metav1.Object) (ca []byte, err error) + ReadCA(ctx context.Context, log logr.Logger, metaObj metav1.Object, namespace string) (ca []byte, err error) // ApplyTo applies any required watchers to the given controller. ApplyTo(ctx context.Context, mgr ctrl.Manager, setup injectorSetup, controller controller.Controller, ca cache.Cache) error @@ -69,7 +70,7 @@ func (c *kubeconfigDataSource) Configured(log logr.Logger, metaObj metav1.Object return metaObj.GetAnnotations()[cmapi.WantInjectAPIServerCAAnnotation] == "true" } -func (c *kubeconfigDataSource) ReadCA(ctx context.Context, log logr.Logger, metaObj metav1.Object) (ca []byte, err error) { +func (c *kubeconfigDataSource) ReadCA(ctx context.Context, log logr.Logger, metaObj metav1.Object, namespace string) (ca []byte, err error) { return c.apiserverCABundle, nil } @@ -99,15 +100,22 @@ func (c *certificateDataSource) Configured(log logr.Logger, metaObj metav1.Objec return true } -func (c *certificateDataSource) ReadCA(ctx context.Context, log logr.Logger, metaObj metav1.Object) (ca []byte, err error) { +func (c *certificateDataSource) ReadCA(ctx context.Context, log logr.Logger, metaObj metav1.Object, namespace string) (ca []byte, err error) { certNameRaw := metaObj.GetAnnotations()[cmapi.WantInjectAnnotation] certName := splitNamespacedName(certNameRaw) log = log.WithValues("certificate", certName) if certName.Namespace == "" { log.Error(nil, "invalid certificate name; needs a namespace/ prefix") + // TODO: should an error be returned here to prevent the caller from proceeding? // don't return an error, requeuing won't help till this is changed return nil, nil } + if namespace != "" && certName.Namespace != namespace { + err := fmt.Errorf("cannot read CA data from Certificate in namespace %s, cainjector is scoped to namespace %s", certName.Namespace, namespace) + forbidenErr := apierrors.NewForbidden(cmapi.Resource("certificates"), certName.Name, err) + log.Error(forbidenErr, "cannot read data source") + return nil, forbidenErr + } var cert cmapi.Certificate if err := c.client.Get(ctx, certName, &cert); err != nil { @@ -185,16 +193,24 @@ func (c *secretDataSource) Configured(log logr.Logger, metaObj metav1.Object) bo return true } -func (c *secretDataSource) ReadCA(ctx context.Context, log logr.Logger, metaObj metav1.Object) ([]byte, error) { +func (c *secretDataSource) ReadCA(ctx context.Context, log logr.Logger, metaObj metav1.Object, namespace string) ([]byte, error) { secretNameRaw := metaObj.GetAnnotations()[cmapi.WantInjectFromSecretAnnotation] secretName := splitNamespacedName(secretNameRaw) log = log.WithValues("secret", secretName) if secretName.Namespace == "" { log.Error(nil, "invalid certificate name") + // TODO: should we return error here to prevent the caller from proceeding? // don't return an error, requeuing won't help till this is changed return nil, nil } + if namespace != "" && secretName.Namespace != namespace { + err := fmt.Errorf("cannot read CA data from Secret in namespace %s, cainjector is scoped to namespace %s", secretName.Namespace, namespace) + forbidenErr := apierrors.NewForbidden(cmapi.Resource("certificates"), secretName.Name, err) + log.Error(forbidenErr, "cannot read data source") + return nil, forbidenErr + } + // grab the associated secret var secret corev1.Secret if err := c.client.Get(ctx, secretName, &secret); err != nil {