From 5111838dfee333a07d734cab3296dfef2938e8a2 Mon Sep 17 00:00:00 2001 From: Sean Hobbs Date: Tue, 29 Nov 2022 15:13:38 -0500 Subject: [PATCH 1/3] Bound the agent created webhook config to the fleet-system namespace. --- pkg/webhook/webhook.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index e0d7198a8..e88537e96 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" @@ -180,6 +181,7 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, } + bindWebhookConfigToFleetSystem(ctx, w.mgr.GetClient(), &whCfg) if err := w.mgr.GetClient().Create(ctx, &whCfg); err != nil { if !apierrors.IsAlreadyExists(err) { return err @@ -386,3 +388,22 @@ func genCertAndKeyFile(certData, keyData []byte, certDir string) error { klog.V(2).InfoS("successfully generate certificate and key files") return nil } + +// bindWebhookConfigToFleetSystem sets the OwnerReference of the webhook config to a cluster scoped Fleet resource. +func bindWebhookConfigToFleetSystem(ctx context.Context, k8Client client.Client, validatingWebhookConfig *admv1.ValidatingWebhookConfiguration) { + var fleetNs corev1.Namespace + err := k8Client.Get(ctx, client.ObjectKey{Name: "fleet-system"}, &fleetNs) + if err != nil { + klog.ErrorS(err, "error while binding ValidatingWebhookConfig to fleet-system namespace") + } + + ownerRef := metav1.OwnerReference{ + APIVersion: fleetNs.GroupVersionKind().GroupVersion().String(), + Kind: fleetNs.Kind, + Name: fleetNs.GetName(), + UID: fleetNs.GetUID(), + BlockOwnerDeletion: pointer.Bool(false), + } + + validatingWebhookConfig.OwnerReferences = []metav1.OwnerReference{ownerRef} +} From 8e4a6cb7dfb57916fa52ffb39308fbdf6263512e Mon Sep 17 00:00:00 2001 From: Sean Hobbs Date: Tue, 29 Nov 2022 15:31:00 -0500 Subject: [PATCH 2/3] Error handling. --- pkg/webhook/webhook.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index e88537e96..b4120c7d6 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -181,7 +181,10 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, } - bindWebhookConfigToFleetSystem(ctx, w.mgr.GetClient(), &whCfg) + if err := bindWebhookConfigToFleetSystem(ctx, w.mgr.GetClient(), &whCfg); err != nil { + return err + } + if err := w.mgr.GetClient().Create(ctx, &whCfg); err != nil { if !apierrors.IsAlreadyExists(err) { return err @@ -390,11 +393,10 @@ func genCertAndKeyFile(certData, keyData []byte, certDir string) error { } // bindWebhookConfigToFleetSystem sets the OwnerReference of the webhook config to a cluster scoped Fleet resource. -func bindWebhookConfigToFleetSystem(ctx context.Context, k8Client client.Client, validatingWebhookConfig *admv1.ValidatingWebhookConfiguration) { +func bindWebhookConfigToFleetSystem(ctx context.Context, k8Client client.Client, validatingWebhookConfig *admv1.ValidatingWebhookConfiguration) error { var fleetNs corev1.Namespace - err := k8Client.Get(ctx, client.ObjectKey{Name: "fleet-system"}, &fleetNs) - if err != nil { - klog.ErrorS(err, "error while binding ValidatingWebhookConfig to fleet-system namespace") + if err := k8Client.Get(ctx, client.ObjectKey{Name: "fleet-system"}, &fleetNs); err != nil { + return err } ownerRef := metav1.OwnerReference{ @@ -406,4 +408,5 @@ func bindWebhookConfigToFleetSystem(ctx context.Context, k8Client client.Client, } validatingWebhookConfig.OwnerReferences = []metav1.OwnerReference{ownerRef} + return nil } From 9f6ba7d2e25833a93255282d32e5e177c4295b96 Mon Sep 17 00:00:00 2001 From: Sean Hobbs Date: Fri, 2 Dec 2022 09:56:57 -0800 Subject: [PATCH 3/3] Added comments. --- pkg/webhook/webhook.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index b4120c7d6..ea38fa539 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -181,6 +181,8 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, } + // We need to ensure this webhook configuration is garbage collected if Fleet is uninstalled from the cluster. + // Since the fleet-system namespace is a prerequisite for core Fleet components, we bind to this namespace. if err := bindWebhookConfigToFleetSystem(ctx, w.mgr.GetClient(), &whCfg); err != nil { return err } @@ -392,7 +394,7 @@ func genCertAndKeyFile(certData, keyData []byte, certDir string) error { return nil } -// bindWebhookConfigToFleetSystem sets the OwnerReference of the webhook config to a cluster scoped Fleet resource. +// bindWebhookConfigToFleetSystem sets the OwnerReference of the argued ValidatingWebhookConfiguration to the cluster scoped fleet-system namespace. func bindWebhookConfigToFleetSystem(ctx context.Context, k8Client client.Client, validatingWebhookConfig *admv1.ValidatingWebhookConfiguration) error { var fleetNs corev1.Namespace if err := k8Client.Get(ctx, client.ObjectKey{Name: "fleet-system"}, &fleetNs); err != nil {