Split webhook validation (#10284)

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: Sam Heilbron <[email protected]>
Co-authored-by: Nadine Spies <[email protected]>
Co-authored-by: Jenny Shu <[email protected]>

Author: 5 people

.github/workflows/pr-kubernetes-tests.yaml

@@ -76,8 +76,9 @@ jobs:
           go-test-run-regex: '(^TestK8sGatewayIstio$$|^TestGlooGatewayEdgeGateway$$|^TestGlooctlIstioInjectEdgeApiGateway$$)'
 
         # October 10, 2024: 22 minutes
+        # TODO (sheidkamp) rebalance tests
         - cluster-name: 'cluster-five'
-          go-test-args: '-v -timeout=25m'
+          go-test-args: '-v -timeout=35m'
           go-test-run-regex: '^TestFullEnvoyValidation$$|^TestValidationStrict$$|^TestValidationAlwaysAccept$$|^TestTransformationValidationDisabled$$|^TestGloomtlsGatewayEdgeGateway$$|^TestWatchNamespaceSelector$$'
 
         # October 10, 2024: 12 minutes

changelog/v1.18.0-beta34/split-validating-webhook.yaml

@@ -0,0 +1,14 @@
+changelog:
+  - type: NEW_FEATURE
+    issueLink: https://github.com/solo-io/gloo/issues/10247
+    resolvesIssue: false
+    description: >-
+      Split the validating webhook to allow different failure policies for gloo/non-gloo resources.
+      The split out webhook for kubernetes resources shares all configuration with the existing webhook except for the failure policy,
+      which can be set with `gateway.validation.kubeCoreFailurePolicy`
+  - type: DEPENDENCY_BUMP
+    dependencyOwner: solo-io
+    dependencyRepo: k8s-utils
+    dependencyTag: v0.8.1
+    description: >-
+      Bump k8s-utils to v0.8.1 for updated `ConvertUnstructured` function

docs/content/reference/values.txt

@@ -569,7 +569,8 @@
 |gateway.validation.disableTransformationValidation|bool|false|set this to true to disable transformation validation. This may bring significant performance benefits if using many transformations, at the cost of possibly incorrect transformations being sent to Envoy. When using this value make sure to pre-validate transformations.|
 |gateway.validation.warnRouteShortCircuiting|bool|false|Write a warning to route resources if validation produced a route ordering warning (defaults to false). By setting to true, this means that Gloo Edge will start assigning warnings to resources that would result in route short-circuiting within a virtual host.|
 |gateway.validation.secretName|string|gateway-validation-certs|Name of the Kubernetes Secret containing TLS certificates used by the validation webhook server. This secret will be created by the certGen Job if the certGen Job is enabled.|
-|gateway.validation.failurePolicy|string|Ignore|failurePolicy defines how unrecognized errors from the Gateway validation endpoint are handled - allowed values are 'Ignore' or 'Fail'. Defaults to Ignore |
+|gateway.validation.failurePolicy|string|Ignore|Specify how to handle unrecognized errors for Gloo resources that are returned from the Gateway validation endpoint. Supported values are 'Ignore' or 'Fail'|
+|gateway.validation.kubeCoreFailurePolicy|string|Ignore|Specify how to handle unrecognized errors for Kubernetes core resources that are returned by the Gateway validation endpoint. Currently the [validation webhook](https://github.com/solo-io/gloo/blob/main/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml) is configured to handle errors for Kubernetes secrets and namespaces. Supported values are 'Ignore' or 'Fail'. If you set this value to 'Fail', you cannot modify these core resources if the 'gloo' service is unavailable.|
 |gateway.validation.webhook.enabled|bool|true|enable validation webhook (default true)|
 |gateway.validation.webhook.disableHelmHook|bool|false|do not create the webhook as helm hook (default false)|
 |gateway.validation.webhook.timeoutSeconds|int||the timeout for the webhook, defaults to 10|

go.mod

@@ -49,7 +49,7 @@ require (
 	github.com/sergi/go-diff v1.2.0
 	github.com/solo-io/go-list-licenses v0.1.4
 	github.com/solo-io/go-utils v0.27.1
-	github.com/solo-io/k8s-utils v0.8.0
+	github.com/solo-io/k8s-utils v0.8.1
 	github.com/solo-io/protoc-gen-ext v0.0.25
 	github.com/solo-io/protoc-gen-openapi v0.2.5
 	github.com/solo-io/skv2 v0.41.0

go.sum

@@ -2697,8 +2697,8 @@ github.com/solo-io/go-list-licenses v0.1.4/go.mod h1:x6LSp/NrYgVXwNum7ZOiaAYTpg6
 github.com/solo-io/go-utils v0.20.2/go.mod h1:6e8K1spnMWwlnJRSNp/J84GEyJbrcK4Gm7i+ehzCi8c=
 github.com/solo-io/go-utils v0.27.1 h1:14XwaKv21EaYYeUF2wFfPe3DPz2Gbm9sfenGv/aRIls=
 github.com/solo-io/go-utils v0.27.1/go.mod h1:cwbQIYO1/BeU4aPB0Yy8WzzS77dfVTZyCVqbA4YsRSY=
-github.com/solo-io/k8s-utils v0.8.0 h1:jXd4HFDgbPWxHi04QDFYwA37D1nYr9XJI3MVa75oCD8=
-github.com/solo-io/k8s-utils v0.8.0/go.mod h1:fOIFkh4+F45MmrUZEFx0pW75EvFYOR7v5/BIIQiSIwA=
+github.com/solo-io/k8s-utils v0.8.1 h1:Xqqze6RLWsHCYetbaiXDEnuhFRXyqw0azyogggK43H8=
+github.com/solo-io/k8s-utils v0.8.1/go.mod h1:fOIFkh4+F45MmrUZEFx0pW75EvFYOR7v5/BIIQiSIwA=
 github.com/solo-io/protoc-gen-ext v0.0.25 h1:UqNW/A4UqCO5aUFg7LYdV82tK0R2mqu7RFftYtT/Fu8=
 github.com/solo-io/protoc-gen-ext v0.0.25/go.mod h1:TZwUhbFtfd1fQQGBN6qWwtea0Fhi3V6DvGQnbqk3jf8=
 github.com/solo-io/protoc-gen-openapi v0.2.5 h1:l8YKsVks6JDFRzA9liYZIqauqpYRxHXnmHi4TjTIRf4=

install/helm/gloo/generate/values.go

@@ -479,7 +479,8 @@ type GatewayValidation struct {
 	DisableTransformationValidation  *bool    `json:"disableTransformationValidation,omitempty" desc:"set this to true to disable transformation validation. This may bring significant performance benefits if using many transformations, at the cost of possibly incorrect transformations being sent to Envoy. When using this value make sure to pre-validate transformations."`
 	WarnRouteShortCircuiting         *bool    `json:"warnRouteShortCircuiting,omitempty" desc:"Write a warning to route resources if validation produced a route ordering warning (defaults to false). By setting to true, this means that Gloo Edge will start assigning warnings to resources that would result in route short-circuiting within a virtual host."`
 	SecretName                       *string  `json:"secretName,omitempty" desc:"Name of the Kubernetes Secret containing TLS certificates used by the validation webhook server. This secret will be created by the certGen Job if the certGen Job is enabled."`
-	FailurePolicy                    *string  `json:"failurePolicy,omitempty" desc:"failurePolicy defines how unrecognized errors from the Gateway validation endpoint are handled - allowed values are 'Ignore' or 'Fail'. Defaults to Ignore "`
+	FailurePolicy                    *string  `json:"failurePolicy,omitempty" desc:"Specify how to handle unrecognized errors for Gloo resources that are returned from the Gateway validation endpoint. Supported values are 'Ignore' or 'Fail'"`
+	KubeCoreFailurePolicy            *string  `json:"kubeCoreFailurePolicy,omitempty" desc:"Specify how to handle unrecognized errors for Kubernetes core resources that are returned by the Gateway validation endpoint. Currently the [validation webhook](https://github.com/solo-io/gloo/blob/main/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml) is configured to handle errors for Kubernetes secrets and namespaces. Supported values are 'Ignore' or 'Fail'. If you set this value to 'Fail', you cannot modify these core resources if the 'gloo' service is unavailable."`
 	Webhook                          *Webhook `json:"webhook,omitempty" desc:"webhook specific configuration"`
 	ValidationServerGrpcMaxSizeBytes *int     `json:"validationServerGrpcMaxSizeBytes,omitempty" desc:"gRPC max message size in bytes for the gloo validation server"`
 	LivenessProbeEnabled             *bool    `json:"livenessProbeEnabled,omitempty" desc:"Set to true to enable a liveness probe for the gateway (default is false). You must also set the 'Probes' value to true."`

install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml

@@ -52,7 +52,36 @@ specific resources, we will manage the resources that the webhook receives via t
     apiGroups: ["gloo.solo.io"]
     apiVersions: ["v1"]
     resources: ["upstreams"]{{/* TODO(https://github.com/solo-io/gloo/issues/2797): Extend to all gloo resources */}}
-{{/* Can't use the include for this one because if the operations are empty, we need to drop the whole list element */}}
+  - operations: {{ include "gloo.webhookvalidation.operationsForResource" (list "ratelimitconfigs" .Values.gateway.validation.webhook.skipDeleteValidationResources) }}
+    apiGroups: ["ratelimit.solo.io"]
+    apiVersions: ["v1alpha1"]
+    resources: ["ratelimitconfigs"]
+  sideEffects: None
+  matchPolicy: Exact
+{{- if .Values.gateway.validation.webhook.timeoutSeconds }}
+  timeoutSeconds: {{ .Values.gateway.validation.webhook.timeoutSeconds }}
+{{- end }}
+  admissionReviewVersions:
+    - v1beta1 # v1beta1 still live in 1.22 https://github.com/kubernetes/api/blob/release-1.22/admission/v1beta1/types.go#L33
+{{- if .Values.gateway.validation.failurePolicy }}
+  failurePolicy: {{ .Values.gateway.validation.failurePolicy }}
+{{- end }} {{- /* if .Values.gateway.validation.failurePolicy */}}
+
+{{/* Webhook for core resources - only render if we need to */}}
+{{- if and 
+  (not (has "*" .Values.gateway.validation.webhook.skipDeleteValidationResources))
+  (or (not (has "secrets" .Values.gateway.validation.webhook.skipDeleteValidationResources))
+       (not (has "namespaces" .Values.gateway.validation.webhook.skipDeleteValidationResources)))
+}}
+- name: kube.{{ .Release.Namespace }}.svc  # must be a domain with at least three segments separated by dots
+  clientConfig:
+    service:
+      name: gloo
+      namespace: {{ .Release.Namespace }}
+      path: "/validation"
+    caBundle: "" # update manually or use certgen job or cert-manager's ca-injector
+  rules:
+{{- /* Can't use the include for this one because if the operations are empty, we need to drop the whole list element */}}
 {{- if and (not (has "*" .Values.gateway.validation.webhook.skipDeleteValidationResources)) (not (has "secrets" .Values.gateway.validation.webhook.skipDeleteValidationResources)) }}
   - operations: [ "DELETE" ]
     apiGroups: [""]{{/* We do not have internal secret CRDs. We want to validate the deletion of secrets such as TLS, so we add "" which refers to Kubernetes' core APIs. */}}
@@ -66,20 +95,18 @@ specific resources, we will manage the resources that the webhook receives via t
     apiVersions: ["v1"]
     resources: ["namespaces"]
 {{- end }}
-  - operations: {{ include "gloo.webhookvalidation.operationsForResource" (list "ratelimitconfigs" .Values.gateway.validation.webhook.skipDeleteValidationResources) }}
-    apiGroups: ["ratelimit.solo.io"]
-    apiVersions: ["v1alpha1"]
-    resources: ["ratelimitconfigs"]
   sideEffects: None
   matchPolicy: Exact
 {{- if .Values.gateway.validation.webhook.timeoutSeconds }}
   timeoutSeconds: {{ .Values.gateway.validation.webhook.timeoutSeconds }}
 {{- end }}
   admissionReviewVersions:
     - v1beta1 # v1beta1 still live in 1.22 https://github.com/kubernetes/api/blob/release-1.22/admission/v1beta1/types.go#L33
-{{- if .Values.gateway.validation.failurePolicy }}
-  failurePolicy: {{ .Values.gateway.validation.failurePolicy }}
-{{- end }} {{/* if .Values.gateway.validation.failurePolicy */}}
+{{- if .Values.gateway.validation.kubeCoreFailurePolicy }}
+  failurePolicy: {{ .Values.gateway.validation.kubeCoreFailurePolicy -}}
+{{- end }} {{/* if .Values.gateway.validation.kubeCoreFailurePolicy */}}
+{{- end }} {{/* render webhook if */}}
+
 {{- end }} {{/* if and .Values.gateway.enabled .Values.gateway.validation.enabled .Values.gateway.validation.webhook.enabled */}}
 {{- end }} {{/* define "gateway.validationWebhookSpec" */}}
 
@@ -88,6 +115,6 @@ specific resources, we will manage the resources that the webhook receives via t
 {{- if .Values.gateway.validation -}}
 {{- if .Values.gateway.validation.webhook -}}
 {{- $kubeResourceOverride = .Values.gateway.validation.webhook.kubeResourceOverride   -}}
-{{- end -}} {{/* if .Values.gateway.validation.webhook */}}
-{{- end -}} {{/* if .Values.gateway.validation */}}
+{{- end -}} {{/* if .Values.gateway.validation.webhook */ -}}
+{{- end -}} {{/* if .Values.gateway.validation */ -}}
 {{- include "gloo.util.merge" (list . $kubeResourceOverride "gateway.validationWebhookSpec") -}}

install/helm/gloo/values-template.yaml

@@ -131,6 +131,9 @@ gateway:
   validation:
     enabled: true
     failurePolicy: "Ignore"
+    # This is the recommended setting because if it set to "Fail" modifications to core resources such as secrets and namespace that are defined
+    # in the validating webhook will be blocked if the Gloo Service is not available.
+    kubeCoreFailurePolicy: "Ignore"
     secretName: gateway-validation-certs
     alwaysAcceptResources: true
     allowWarnings: true