fix: put webhook disable option back (#1439)

kwok/charts/README.md

@@ -60,5 +60,6 @@ For full Karpenter documentation please checkout [https://karpenter.sh](https://
 | terminationGracePeriodSeconds | string | `nil` | Override the default termination grace period for the pod. |
 | tolerations | list | `[{"key":"CriticalAddonsOnly","operator":"Exists"}]` | Tolerations to allow the pod to be scheduled to nodes with taints. |
 | topologySpreadConstraints | list | `[{"maxSkew":1,"topologyKey":"topology.kubernetes.io/zone","whenUnsatisfiable":"ScheduleAnyway"}]` | Topology spread constraints to increase the controller resilience by distributing pods across the cluster zones. If an explicit label selector is not provided one will be created from the pod selector labels. |
+| webhook.enabled | bool | `true` | Whether to enable the webhooks and webhook permissions. |
 | webhook.port | int | `8443` | The container port to use for the webhook. |
 

kwok/charts/templates/deployment.yaml

@@ -76,8 +76,16 @@ spec:
               value: "1.19.0-0"
             - name: KARPENTER_SERVICE
               value: {{ include "karpenter.fullname" . }}
+          {{- if .Values.webhook.enabled }}
             - name: WEBHOOK_PORT
               value: "{{ .Values.webhook.port }}"
+            - name: DISABLE_WEBHOOK
+              value: "false"
+          {{- end }}
+          {{- if not .Values.webhook.enabled }}
+            - name: DISABLE_WEBHOOK
+              value: "true" 
+          {{- end }}
           {{- with .Values.logLevel }}
             - name: LOG_LEVEL
               value: "{{ . }}"
@@ -120,9 +128,11 @@ spec:
             - name: http
               containerPort: {{ .Values.controller.healthProbe.port }}
               protocol: TCP
+          {{- if .Values.webhook.enabled }}
             - name: https-webhook
               containerPort: {{ .Values.webhook.port }}
               protocol: TCP
+          {{- end }}
           livenessProbe:
             initialDelaySeconds: 30
             timeoutSeconds: 30

kwok/charts/templates/role.yaml

@@ -14,15 +14,19 @@ rules:
   - apiGroups: ["coordination.k8s.io"]
     resources: ["leases"]
     verbs: ["get", "watch"]
+{{- if .Values.webhook.enabled }}
   - apiGroups: [""]
     resources: ["configmaps", "secrets"]
     verbs: ["get", "list", "watch"]
+{{- end}}
   # Write
+{{- if .Values.webhook.enabled }}
   - apiGroups: [""]
     resources: ["secrets"]
     verbs: ["update"]
     resourceNames:
       - "{{ include "karpenter.fullname" . }}-cert"
+{{- end }}
   - apiGroups: ["coordination.k8s.io"]
     resources: ["leases"]
     verbs: ["patch", "update"]

kwok/charts/templates/secret-webhook-cert.yaml

@@ -1,3 +1,4 @@
+{{- if .Values.webhook.enabled }}
 apiVersion: v1
 kind: Secret
 metadata:
@@ -9,4 +10,5 @@ metadata:
   annotations:
     {{- toYaml . | nindent 4 }}
   {{- end }}
-# data: {} # Injected by karpenter-webhook
+# data: {} # Injected by karpenter-webhook
+{{- end }}

kwok/charts/templates/service.yaml

@@ -16,9 +16,11 @@ spec:
       port: {{ .Values.controller.metrics.port }}
       targetPort: http-metrics
       protocol: TCP
+  {{- if .Values.webhook.enabled }}
     - name: https-webhook
       port: {{ .Values.webhook.port }}
       targetPort: https-webhook
       protocol: TCP
+  {{- end }}
   selector:
     {{- include "karpenter.selectorLabels" . | nindent 4 }}

kwok/charts/values.yaml

@@ -123,6 +123,8 @@ controller:
     # -- The container port to use for http health probe.
     port: 8081
 webhook:
+  # -- Whether to enable the webhooks and webhook permissions.
+  enabled: true
   # -- The container port to use for the webhook.
   port: 8443
 # -- Global log level, defaults to 'info'

pkg/operator/operator.go

@@ -241,9 +241,11 @@ func (o *Operator) WithControllers(ctx context.Context, controllers ...controlle
 }
 
 func (o *Operator) WithWebhooks(ctx context.Context, ctors ...knativeinjection.ControllerConstructor) *Operator {
-	o.webhooks = append(o.webhooks, ctors...)
-	lo.Must0(o.Manager.AddReadyzCheck("webhooks", webhooks.HealthProbe(ctx)))
-	lo.Must0(o.Manager.AddHealthzCheck("webhooks", webhooks.HealthProbe(ctx)))
+	if !options.FromContext(ctx).DisableWebhook {
+		o.webhooks = append(o.webhooks, ctors...)
+		lo.Must0(o.Manager.AddReadyzCheck("webhooks", webhooks.HealthProbe(ctx)))
+		lo.Must0(o.Manager.AddHealthzCheck("webhooks", webhooks.HealthProbe(ctx)))
+	}
 	return o
 }
 
@@ -254,16 +256,20 @@ func (o *Operator) Start(ctx context.Context, cp cloudprovider.CloudProvider) {
 		defer wg.Done()
 		lo.Must0(o.Manager.Start(ctx))
 	}()
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
-		// Taking the first supported NodeClass to be the default NodeClass
-		gvk := lo.Map(cp.GetSupportedNodeClasses(), func(nc status.Object, _ int) schema.GroupVersionKind {
-			return object.GVK(nc)
-		})
-		ctx = injection.WithNodeClasses(ctx, gvk)
-		ctx = injection.WithClient(ctx, o.GetClient())
-		webhooks.Start(ctx, o.GetConfig(), o.webhooks...)
-	}()
+	if options.FromContext(ctx).DisableWebhook {
+		log.FromContext(ctx).Info("conversion webhooks are disabled")
+	} else {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			// Taking the first supported NodeClass to be the default NodeClass
+			gvk := lo.Map(cp.GetSupportedNodeClasses(), func(nc status.Object, _ int) schema.GroupVersionKind {
+				return object.GVK(nc)
+			})
+			ctx = injection.WithNodeClasses(ctx, gvk)
+			ctx = injection.WithClient(ctx, o.GetClient())
+			webhooks.Start(ctx, o.GetConfig(), o.webhooks...)
+		}()
+	}
 	wg.Wait()
 }

pkg/operator/options/options.go

@@ -47,6 +47,7 @@ type FeatureGates struct {
 // Options contains all CLI flags / env vars for karpenter-core. It adheres to the options.Injectable interface.
 type Options struct {
 	ServiceName          string
+	DisableWebhook       bool
 	WebhookPort          int
 	MetricsPort          int
 	WebhookMetricsPort   int
@@ -81,6 +82,7 @@ func (fs *FlagSet) BoolVarWithEnv(p *bool, name string, envVar string, val bool,
 
 func (o *Options) AddFlags(fs *FlagSet) {
 	fs.StringVar(&o.ServiceName, "karpenter-service", env.WithDefaultString("KARPENTER_SERVICE", ""), "The Karpenter Service name for the dynamic webhook certificate")
+	fs.BoolVarWithEnv(&o.DisableWebhook, "disable-webhook", "DISABLE_WEBHOOK", false, "Disable the conversion webhooks")
 	fs.IntVar(&o.WebhookPort, "webhook-port", env.WithDefaultInt("WEBHOOK_PORT", 8443), "The port the webhook endpoint binds to for validation and mutation of resources")
 	fs.IntVar(&o.MetricsPort, "metrics-port", env.WithDefaultInt("METRICS_PORT", 8000), "The port the metric endpoint binds to for operating metrics about the controller itself")
 	fs.IntVar(&o.WebhookMetricsPort, "webhook-metrics-port", env.WithDefaultInt("WEBHOOK_METRICS_PORT", 8001), "The port the webhook metric endpoing binds to for operating metrics about the webhook")

pkg/operator/options/suite_test.go

@@ -97,7 +97,7 @@ var _ = Describe("Options", func() {
 			Expect(err).To(BeNil())
 			expectOptionsMatch(opts, test.Options(test.OptionsFields{
 				ServiceName:          lo.ToPtr(""),
-				DisableWebhook:       lo.ToPtr(true),
+				DisableWebhook:       lo.ToPtr(false),
 				WebhookPort:          lo.ToPtr(8443),
 				MetricsPort:          lo.ToPtr(8000),
 				WebhookMetricsPort:   lo.ToPtr(8001),
@@ -120,6 +120,7 @@ var _ = Describe("Options", func() {
 			err := opts.Parse(
 				fs,
 				"--karpenter-service", "cli",
+				"--disable-webhook",
 				"--webhook-port", "0",
 				"--metrics-port", "0",
 				"--webhook-metrics-port", "0",
@@ -220,6 +221,7 @@ var _ = Describe("Options", func() {
 			err := opts.Parse(
 				fs,
 				"--karpenter-service", "cli",
+				"--disable-webhook",
 			)
 			Expect(err).To(BeNil())
 			expectOptionsMatch(opts, test.Options(test.OptionsFields{
@@ -244,6 +246,19 @@ var _ = Describe("Options", func() {
 		})
 	})
 
+	DescribeTable(
+		"should correctly parse boolean values",
+		func(arg string, expected bool) {
+			err := opts.Parse(fs, arg)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(opts.DisableWebhook).To(Equal(expected))
+		},
+		Entry("explicit true", "--disable-webhook=true", true),
+		Entry("explicit false", "--disable-webhook=false", false),
+		Entry("implicit true", "--disable-webhook", true),
+		Entry("implicit false", "", false),
+	)
+
 	Context("Validation", func() {
 		DescribeTable(
 			"should parse valid log levels successfully",
@@ -271,6 +286,7 @@ func expectOptionsMatch(optsA, optsB *options.Options) {
 	Expect(optsA).ToNot(BeNil())
 	Expect(optsB).ToNot(BeNil())
 	Expect(optsA.ServiceName).To(Equal(optsB.ServiceName))
+	Expect(optsA.DisableWebhook).To(Equal(optsB.DisableWebhook))
 	Expect(optsA.WebhookPort).To(Equal(optsB.WebhookPort))
 	Expect(optsA.MetricsPort).To(Equal(optsB.MetricsPort))
 	Expect(optsA.WebhookMetricsPort).To(Equal(optsB.WebhookMetricsPort))

pkg/test/options.go

@@ -59,6 +59,7 @@ func Options(overrides ...OptionsFields) *options.Options {
 
 	return &options.Options{
 		ServiceName:          lo.FromPtrOr(opts.ServiceName, ""),
+		DisableWebhook:       lo.FromPtrOr(opts.DisableWebhook, false),
 		WebhookPort:          lo.FromPtrOr(opts.WebhookPort, 8443),
 		MetricsPort:          lo.FromPtrOr(opts.MetricsPort, 8000),
 		WebhookMetricsPort:   lo.FromPtrOr(opts.WebhookMetricsPort, 8001),