From 39713768148865049fa41471c429457b4457e09d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Mon, 3 Oct 2022 13:23:02 +0200 Subject: [PATCH] refactor: introduce webhook controller (#4749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: introduce webhook controller Signed-off-by: Charles-Edouard Brétéché * fix linter issues Signed-off-by: Charles-Edouard Brétéché * fix linter Signed-off-by: Charles-Edouard Brétéché * fix imports Signed-off-by: Charles-Edouard Brétéché * merge main Signed-off-by: Charles-Edouard Brétéché * merge main Signed-off-by: Charles-Edouard Brétéché * fix linter Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- cmd/kyverno/main.go | 24 ++- pkg/controllers/certmanager/controller.go | 21 +-- pkg/controllers/webhook/controller.go | 182 ++++++++++++++++++++++ pkg/controllers/webhook/log.go | 7 + pkg/utils/controller/utils.go | 13 +- pkg/webhookconfig/registration.go | 45 ------ 6 files changed, 230 insertions(+), 62 deletions(-) create mode 100644 pkg/controllers/webhook/controller.go create mode 100644 pkg/controllers/webhook/log.go diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index c837318c93..a0dd05bfe1 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -28,6 +28,7 @@ import ( aggregatereportcontroller "github.com/kyverno/kyverno/pkg/controllers/report/aggregate" backgroundscancontroller "github.com/kyverno/kyverno/pkg/controllers/report/background" resourcereportcontroller "github.com/kyverno/kyverno/pkg/controllers/report/resource" + webhookcontroller "github.com/kyverno/kyverno/pkg/controllers/webhook" "github.com/kyverno/kyverno/pkg/cosign" event "github.com/kyverno/kyverno/pkg/event" "github.com/kyverno/kyverno/pkg/leaderelection" @@ -48,6 +49,8 @@ import ( webhooksresource "github.com/kyverno/kyverno/pkg/webhooks/resource" webhookgenerate "github.com/kyverno/kyverno/pkg/webhooks/updaterequest" _ "go.uber.org/automaxprocs" // #nosec + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" kubeinformers "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" metadataclient "k8s.io/client-go/metadata" @@ -390,12 +393,30 @@ func main() { logger.Error(err, "failed to initialize CertRenewer") os.Exit(1) } - certManager, err := certmanager.NewController(kubeKyvernoInformer.Core().V1().Secrets(), certRenewer, webhookCfg.UpdateWebhooksCaBundle) + certManager, err := certmanager.NewController(kubeKyvernoInformer.Core().V1().Secrets(), certRenewer) if err != nil { logger.Error(err, "failed to initialize CertManager") os.Exit(1) } + webhookController := webhookcontroller.NewController( + metrics.ObjectClient[*corev1.Secret]( + metrics.NamespacedClientQueryRecorder(metricsConfig, config.KyvernoNamespace(), "Secret", metrics.KubeClient), + kubeClient.CoreV1().Secrets(config.KyvernoNamespace()), + ), + metrics.ObjectClient[*admissionregistrationv1.MutatingWebhookConfiguration]( + metrics.ClusteredClientQueryRecorder(metricsConfig, "MutatingWebhookConfiguration", metrics.KubeClient), + kubeClient.AdmissionregistrationV1().MutatingWebhookConfigurations(), + ), + metrics.ObjectClient[*admissionregistrationv1.ValidatingWebhookConfiguration]( + metrics.ClusteredClientQueryRecorder(metricsConfig, "ValidatingWebhookConfiguration", metrics.KubeClient), + kubeClient.AdmissionregistrationV1().ValidatingWebhookConfigurations(), + ), + kubeKyvernoInformer.Core().V1().Secrets(), + kubeInformer.Admissionregistration().V1().MutatingWebhookConfigurations(), + kubeInformer.Admissionregistration().V1().ValidatingWebhookConfigurations(), + ) + // the webhook server runs across all instances openAPIController := startOpenAPIController(signalCtx, logger, dynamicClient) @@ -463,6 +484,7 @@ func main() { webhookCfg.UpdateWebhookChan <- true go certManager.Run(signalCtx, certmanager.Workers) go policyCtrl.Run(signalCtx, 2) + go webhookController.Run(signalCtx, webhookcontroller.Workers) reportControllers := setupReportControllers( backgroundScan, diff --git a/pkg/controllers/certmanager/controller.go b/pkg/controllers/certmanager/controller.go index 8d05d2636b..cdfc8481bf 100644 --- a/pkg/controllers/certmanager/controller.go +++ b/pkg/controllers/certmanager/controller.go @@ -29,18 +29,16 @@ type controller struct { renewer *tls.CertRenewer secretLister corev1listers.SecretLister // secretSynced returns true if the secret shared informer has synced at least once - secretSynced cache.InformerSynced - secretQueue chan bool - onSecretChanged func() error + secretSynced cache.InformerSynced + secretQueue chan bool } -func NewController(secretInformer corev1informers.SecretInformer, certRenewer *tls.CertRenewer, onSecretChanged func() error) (Controller, error) { +func NewController(secretInformer corev1informers.SecretInformer, certRenewer *tls.CertRenewer) (Controller, error) { manager := &controller{ - renewer: certRenewer, - secretLister: secretInformer.Lister(), - secretSynced: secretInformer.Informer().HasSynced, - secretQueue: make(chan bool, 1), - onSecretChanged: onSecretChanged, + renewer: certRenewer, + secretLister: secretInformer.Lister(), + secretSynced: secretInformer.Informer().HasSynced, + secretQueue: make(chan bool, 1), } secretInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: manager.addSecretFunc, @@ -102,11 +100,6 @@ func (m *controller) renewCertificates() error { if err := common.RetryFunc(time.Second, 5*time.Second, m.renewer.RenewCA, "failed to renew CA", logger)(); err != nil { return err } - if m.onSecretChanged != nil { - if err := common.RetryFunc(time.Second, 5*time.Second, m.onSecretChanged, "failed to renew CA", logger)(); err != nil { - return err - } - } if err := common.RetryFunc(time.Second, 5*time.Second, m.renewer.RenewTLS, "failed to renew TLS", logger)(); err != nil { return err } diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go new file mode 100644 index 0000000000..8707081a93 --- /dev/null +++ b/pkg/controllers/webhook/controller.go @@ -0,0 +1,182 @@ +package background + +import ( + "context" + + "github.com/go-logr/logr" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + "github.com/kyverno/kyverno/pkg/config" + "github.com/kyverno/kyverno/pkg/controllers" + "github.com/kyverno/kyverno/pkg/tls" + controllerutils "github.com/kyverno/kyverno/pkg/utils/controller" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + admissionregistrationv1informers "k8s.io/client-go/informers/admissionregistration/v1" + corev1informers "k8s.io/client-go/informers/core/v1" + admissionregistrationv1listers "k8s.io/client-go/listers/admissionregistration/v1" + corev1listers "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/util/workqueue" +) + +const ( + // Workers is the number of workers for this controller + Workers = 2 + maxRetries = 10 +) + +type controller struct { + // clients + secretClient controllerutils.GetClient[*corev1.Secret] + mwcClient controllerutils.UpdateClient[*admissionregistrationv1.MutatingWebhookConfiguration] + vwcClient controllerutils.UpdateClient[*admissionregistrationv1.ValidatingWebhookConfiguration] + + // listers + secretLister corev1listers.SecretLister + mwcLister admissionregistrationv1listers.MutatingWebhookConfigurationLister + vwcLister admissionregistrationv1listers.ValidatingWebhookConfigurationLister + + // queue + queue workqueue.RateLimitingInterface + mwcEnqueue controllerutils.EnqueueFunc + vwcEnqueue controllerutils.EnqueueFunc +} + +func NewController( + secretClient controllerutils.GetClient[*corev1.Secret], + mwcClient controllerutils.UpdateClient[*admissionregistrationv1.MutatingWebhookConfiguration], + vwcClient controllerutils.UpdateClient[*admissionregistrationv1.ValidatingWebhookConfiguration], + secretInformer corev1informers.SecretInformer, + mwcInformer admissionregistrationv1informers.MutatingWebhookConfigurationInformer, + vwcInformer admissionregistrationv1informers.ValidatingWebhookConfigurationInformer, +) controllers.Controller { + queue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), controllerName) + c := controller{ + secretClient: secretClient, + mwcClient: mwcClient, + vwcClient: vwcClient, + secretLister: secretInformer.Lister(), + mwcLister: mwcInformer.Lister(), + vwcLister: vwcInformer.Lister(), + queue: queue, + mwcEnqueue: controllerutils.AddDefaultEventHandlers(logger.V(3), mwcInformer.Informer(), queue), + vwcEnqueue: controllerutils.AddDefaultEventHandlers(logger.V(3), vwcInformer.Informer(), queue), + } + controllerutils.AddEventHandlers( + secretInformer.Informer(), + func(obj interface{}) { + if err := c.enqueue(obj.(*corev1.Secret)); err != nil { + logger.Error(err, "failed to enqueue") + } + }, + func(_, obj interface{}) { + if err := c.enqueue(obj.(*corev1.Secret)); err != nil { + logger.Error(err, "failed to enqueue") + } + }, + func(obj interface{}) { + if err := c.enqueue(obj.(*corev1.Secret)); err != nil { + logger.Error(err, "failed to enqueue") + } + }, + ) + return &c +} + +func (c *controller) Run(ctx context.Context, workers int) { + controllerutils.Run(ctx, controllerName, logger.V(3), c.queue, workers, maxRetries, c.reconcile) +} + +func (c *controller) enqueue(obj *corev1.Secret) error { + if obj.GetName() == tls.GenerateRootCASecretName() && obj.GetNamespace() == config.KyvernoNamespace() { + requirement, err := labels.NewRequirement("webhook.kyverno.io/managed-by", selection.Equals, []string{kyvernov1.ValueKyvernoApp}) + if err != nil { + // TODO: log error + return err + } + selector := labels.Everything().Add(*requirement) + mwcs, err := c.mwcLister.List(selector) + if err != nil { + return err + } + for _, mwc := range mwcs { + err = c.mwcEnqueue(mwc) + if err != nil { + return err + } + } + vwcs, err := c.vwcLister.List(selector) + if err != nil { + return err + } + for _, vwc := range vwcs { + err = c.vwcEnqueue(vwc) + if err != nil { + return err + } + } + } + return nil +} + +func (c *controller) reconcileMutatingWebhookConfiguration(ctx context.Context, logger logr.Logger, name string) error { + w, err := c.mwcLister.Get(name) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + labels := w.GetLabels() + if labels == nil || labels["webhook.kyverno.io/managed-by"] != kyvernov1.ValueKyvernoApp { + return nil + } + _, err = controllerutils.Update(ctx, w, c.mwcClient, func(w *admissionregistrationv1.MutatingWebhookConfiguration) error { + caData, err := tls.ReadRootCASecret(c.secretClient) + if err != nil { + return err + } + for i := range w.Webhooks { + w.Webhooks[i].ClientConfig.CABundle = caData + } + return nil + }) + return err +} + +func (c *controller) reconcileValidatingWebhookConfiguration(ctx context.Context, logger logr.Logger, name string) error { + w, err := c.vwcLister.Get(name) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + labels := w.GetLabels() + if labels == nil || labels["webhook.kyverno.io/managed-by"] != kyvernov1.ValueKyvernoApp { + return nil + } + _, err = controllerutils.Update(ctx, w, c.vwcClient, func(w *admissionregistrationv1.ValidatingWebhookConfiguration) error { + caData, err := tls.ReadRootCASecret(c.secretClient) + if err != nil { + return err + } + for i := range w.Webhooks { + w.Webhooks[i].ClientConfig.CABundle = caData + } + return nil + }) + return err +} + +func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, namespace, name string) error { + if err := c.reconcileMutatingWebhookConfiguration(ctx, logger, name); err != nil { + return err + } + if err := c.reconcileValidatingWebhookConfiguration(ctx, logger, name); err != nil { + return err + } + return nil +} diff --git a/pkg/controllers/webhook/log.go b/pkg/controllers/webhook/log.go new file mode 100644 index 0000000000..d5e8400f33 --- /dev/null +++ b/pkg/controllers/webhook/log.go @@ -0,0 +1,7 @@ +package background + +import "sigs.k8s.io/controller-runtime/pkg/log" + +const controllerName = "webhook-ca-controller" + +var logger = log.Log.WithName(controllerName) diff --git a/pkg/utils/controller/utils.go b/pkg/utils/controller/utils.go index 4c3663da7f..e590b10da7 100644 --- a/pkg/utils/controller/utils.go +++ b/pkg/utils/controller/utils.go @@ -126,10 +126,19 @@ func CreateOrUpdate[T any, R Object[T], G Getter[R], S Setter[R]](ctx context.Co } } -func Update[T any, R Object[T], S Setter[R]](ctx context.Context, setter S, obj R, build func(R) error) (R, error) { +type DeepCopy[T any] interface { + DeepCopy() T +} + +func Update[T interface { + metav1.Object + DeepCopy[T] +}, S UpdateClient[T]](ctx context.Context, obj T, setter S, build func(T) error, +) (T, error) { mutated := obj.DeepCopy() if err := build(mutated); err != nil { - return nil, err + var d T + return d, err } else { if reflect.DeepEqual(obj, mutated) { return mutated, nil diff --git a/pkg/webhookconfig/registration.go b/pkg/webhookconfig/registration.go index ed50b33dad..19e8adbaa5 100644 --- a/pkg/webhookconfig/registration.go +++ b/pkg/webhookconfig/registration.go @@ -236,51 +236,6 @@ func (wrc *Register) GetWebhookTimeOut() time.Duration { return time.Duration(wrc.timeoutSeconds) } -func (wrc *Register) UpdateWebhooksCaBundle() error { - selector := &metav1.LabelSelector{ - MatchLabels: map[string]string{ - managedByLabel: kyvernov1.ValueKyvernoApp, - }, - } - caData := wrc.readCaData() - m := wrc.kubeClient.AdmissionregistrationV1().MutatingWebhookConfigurations() - v := wrc.kubeClient.AdmissionregistrationV1().ValidatingWebhookConfigurations() - - wrc.metricsConfig.RecordClientQueries(metrics.ClientList, metrics.KubeClient, kindMutating, "") - if list, err := m.List(context.TODO(), metav1.ListOptions{LabelSelector: metav1.FormatLabelSelector(selector)}); err != nil { - return err - } else { - for _, item := range list.Items { - copy := item - for r := range copy.Webhooks { - copy.Webhooks[r].ClientConfig.CABundle = caData - } - if _, err := m.Update(context.TODO(), ©, metav1.UpdateOptions{}); err != nil { - wrc.metricsConfig.RecordClientQueries(metrics.ClientUpdate, metrics.KubeClient, kindMutating, "") - return err - } - } - } - - wrc.metricsConfig.RecordClientQueries(metrics.ClientList, metrics.KubeClient, kindValidating, "") - if list, err := v.List(context.TODO(), metav1.ListOptions{LabelSelector: metav1.FormatLabelSelector(selector)}); err != nil { - return err - } else { - for _, item := range list.Items { - copy := item - for r := range copy.Webhooks { - copy.Webhooks[r].ClientConfig.CABundle = caData - } - - wrc.metricsConfig.RecordClientQueries(metrics.ClientUpdate, metrics.KubeClient, kindValidating, "") - if _, err := v.Update(context.TODO(), ©, metav1.UpdateOptions{}); err != nil { - return err - } - } - } - return nil -} - // UpdateWebhookConfigurations updates resource webhook configurations dynamically // based on the UPDATEs of Kyverno ConfigMap defined in INIT_CONFIG env //