diff --git a/chart/templates/mutating-webhook.yaml b/chart/templates/mutating-webhook.yaml index 1e6890b5..7b407f1c 100644 --- a/chart/templates/mutating-webhook.yaml +++ b/chart/templates/mutating-webhook.yaml @@ -7,7 +7,6 @@ webhooks: - admissionReviewVersions: - v1 clientConfig: - caBundle: "" service: name: skyhook-operator-webhook-service namespace: {{ .Release.Namespace }} @@ -35,7 +34,6 @@ webhooks: - admissionReviewVersions: - v1 clientConfig: - caBundle: "" service: name: skyhook-operator-webhook-service namespace: {{ .Release.Namespace }} diff --git a/chart/templates/validating-webhook.yaml b/chart/templates/validating-webhook.yaml index 6081dcfb..bdd45d16 100644 --- a/chart/templates/validating-webhook.yaml +++ b/chart/templates/validating-webhook.yaml @@ -7,7 +7,6 @@ webhooks: - admissionReviewVersions: - v1 clientConfig: - caBundle: "" service: name: skyhook-operator-webhook-service namespace: {{ .Release.Namespace }} @@ -34,7 +33,6 @@ webhooks: - admissionReviewVersions: - v1 clientConfig: - caBundle: "" service: name: skyhook-operator-webhook-service namespace: {{ .Release.Namespace }} diff --git a/operator/internal/controller/webhook_controller.go b/operator/internal/controller/webhook_controller.go index 140c1f24..ffecff31 100644 --- a/operator/internal/controller/webhook_controller.go +++ b/operator/internal/controller/webhook_controller.go @@ -27,6 +27,7 @@ import ( "time" "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/NVIDIA/skyhook/operator/api/v1alpha1" @@ -127,9 +128,29 @@ func (r *WebhookController) SetupWithManager(mgr ctrl.Manager) error { For(&corev1.Secret{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { return obj.GetNamespace() == r.namespace && obj.GetName() == r.opts.SecretName }))). + // Watch webhook configurations so the leader detects external changes (e.g. Helm upgrade + // resetting caBundle) and fixes them immediately instead of waiting for the 24h requeue. + Watches(&admissionregistrationv1.ValidatingWebhookConfiguration{}, + handler.EnqueueRequestsFromMapFunc(r.webhookConfigToSecret), + builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { + return obj.GetName() == validatingWebhookConfigName + }))). + Watches(&admissionregistrationv1.MutatingWebhookConfiguration{}, + handler.EnqueueRequestsFromMapFunc(r.webhookConfigToSecret), + builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { + return obj.GetName() == mutatingWebhookConfigName + }))). Complete(r) } +// webhookConfigToSecret maps webhook config change events back to the cert Secret, +// so the existing Reconcile() can verify and fix the caBundle. +func (r *WebhookController) webhookConfigToSecret(_ context.Context, _ client.Object) []reconcile.Request { + return []reconcile.Request{{ + NamespacedName: types.NamespacedName{Name: r.opts.SecretName, Namespace: r.namespace}, + }} +} + // permissions //+kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations;mutatingwebhookconfigurations,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete @@ -480,8 +501,8 @@ func deploymentPolicyMutatingRules() []admissionregistrationv1.RuleWithOperation func validatingWebhookNeedsUpdate(webhook *admissionregistrationv1.ValidatingWebhook, caBundle []byte, expectedRules []admissionregistrationv1.RuleWithOperations) bool { needUpdate := false - // Check if CABundle needs to be set - if len(webhook.ClientConfig.CABundle) == 0 { + // Check if CABundle needs to be set or updated (catches both empty and stale values) + if !bytes.Equal(webhook.ClientConfig.CABundle, caBundle) { webhook.ClientConfig.CABundle = caBundle needUpdate = true } @@ -499,8 +520,8 @@ func validatingWebhookNeedsUpdate(webhook *admissionregistrationv1.ValidatingWeb func mutatingWebhookNeedsUpdate(webhook *admissionregistrationv1.MutatingWebhook, caBundle []byte, expectedRules []admissionregistrationv1.RuleWithOperations) bool { needUpdate := false - // Check if CABundle needs to be set - if len(webhook.ClientConfig.CABundle) == 0 { + // Check if CABundle needs to be set or updated (catches both empty and stale values) + if !bytes.Equal(webhook.ClientConfig.CABundle, caBundle) { webhook.ClientConfig.CABundle = caBundle needUpdate = true } diff --git a/operator/internal/controller/webhook_controller_test.go b/operator/internal/controller/webhook_controller_test.go index 370924a2..0747ff09 100644 --- a/operator/internal/controller/webhook_controller_test.go +++ b/operator/internal/controller/webhook_controller_test.go @@ -293,6 +293,32 @@ var _ = Describe("WebhookController", Ordered, func() { Expect(needsUpdate).To(BeTrue(), "should detect empty CABundle") Expect(webhook.ClientConfig.CABundle).To(Equal(caBundle), "CABundle should be updated") }) + + It("should update CABundle when stale (non-empty but wrong)", func() { + expectedRules := skyhookRules() + correctCA := []byte("correct-ca") + staleCA := []byte("stale-ca-from-previous-cert") + + validatingWebhook := admissionregistrationv1.ValidatingWebhook{ + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: staleCA, + }, + Rules: expectedRules, + } + needsUpdate := validatingWebhookNeedsUpdate(&validatingWebhook, correctCA, expectedRules) + Expect(needsUpdate).To(BeTrue(), "should detect stale validating CABundle") + Expect(validatingWebhook.ClientConfig.CABundle).To(Equal(correctCA)) + + mutatingWebhook := admissionregistrationv1.MutatingWebhook{ + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: staleCA, + }, + Rules: expectedRules, + } + needsUpdate = mutatingWebhookNeedsUpdate(&mutatingWebhook, correctCA, expectedRules) + Expect(needsUpdate).To(BeTrue(), "should detect stale mutating CABundle") + Expect(mutatingWebhook.ClientConfig.CABundle).To(Equal(correctCA)) + }) }) Describe("Disk and Secret-to-Disk Sync Logic", func() {