Add back Webhook validation by NGF

bjee19 · bjee19 · commit 72a716f1342f · 2023-11-07T15:48:03.000-08:00
diff --git a/design/resource-validation.md b/design/resource-validation.md
@@ -91,9 +91,36 @@ Design a validation mechanism for Gateway API resources.
 
 ## Design
 
+We will introduce two validation methods to be run by NGF control plane:
+
+1. Re-run of the Gateway API webhook validation
+2. NGF-specific field validation
+
+### Re-run of Webhook Validation
+
+Before processing a resource, NGF will validate it using the functions from
+the [validation package](https://github.com/kubernetes-sigs/gateway-api/tree/b241afc88e68c952cc0a59a5c72a51358dc2bada/apis/v1beta1/validation)
+from the Gateway API. This will ensure that the webhook validation cannot be bypassed (it can be bypassed if the webhook
+is not installed, misconfigured, or running a different version), and it will allow us to avoid repeating the same
+validation in our code.
+
+If a resource is invalid:
+
+- NGF will not process it -- it will treat it as if the resource didn't exist. This also means that if the resource was
+  updated from a valid to an invalid state, NGF will also ignore any previous valid state. For example, it will remove
+  the generation configuration for an HTTPRoute resource.
+- NGF will report the validation error as a
+  Warning [Event](https://kubernetes.io/docs/reference/kubernetes-api/cluster-resources/event-v1/)
+  for that resource. The Event message will describe the error and explain that the resource was ignored. We chose to
+  report an Event instead of updating the status, because to update the status, NGF first needs to look inside the
+  resource to determine whether it belongs to it or not. However, since the webhook validation applies to all parts of
+  the spec of resource, it means NGF has to look inside the invalid resource and parse potentially invalid parts. To
+  avoid that, NGF will report an Event. The owner of the resource will be able to see the Event.
+- NGF will also report the validation error in the NGF logs.
+
 ### NGF-specific validation
 
-NGF will run NGF-specific validation written in go.
+After re-running the webhook validation, NGF will run NGF-specific validation written in go.
 
 NGF-specific validation will:
 
@@ -105,7 +132,7 @@ NGF-specific validation will not include:
 
 - *All* validation done by CRDs. NGF will only repeat the validation that addresses (1) and (2) in the list above with
   extra rules required by NGINX but missing in the CRDs. For example, NGF will not ensure the limits of field values.
-- The validation done by the webhook.
+- The validation done by the webhook (because it is done in the previous step).
 
 If a resource is invalid, NGF will report the error in its status.
 
@@ -119,6 +146,7 @@ following methods in order of their appearance in the table.
 | Name                         | Type                       | Component             | Scope                   | Feedback loop for errors                                                         | Can be bypassed?               |
 |------------------------------|----------------------------|-----------------------|-------------------------|----------------------------------------------------------------------------------|--------------------------------|
 | CRD validation               | OpenAPI and CEL validation | Kubernetes API server | Structure, field values | Kubernetes API server returns any errors a response for an API call.             | Yes, if the CRDs are modified. |
+| Re-run of webhook validation | Go code                    | NGF control plane     | Field values            | Errors are reported as Event for the resource.                                   | No                             |
 | NGF-specific validation      | Go code                    | NGF control plane     | Field values            | Errors are reported in the status of a resource after its creation/modification. | No                             |
 
 
@@ -128,6 +156,7 @@ following methods in order of their appearance in the table.
 |------------------------------|---------|-----------------------|-------------------------|----------------------------------------------------------------------------------|--------------------------------------------------------------------------------------|
 | CRD validation               | OpenAPI | Kubernetes API server | Structure, field values | Kubernetes API server returns any errors a response for an API call.             | Yes, if the CRDs are modified.                                                       |
 | Webhook validation           | Go code | Gateway API webhook   | Field values            | Kubernetes API server returns any errors a response for an API call.             | Yes, if the webhook is not installed, misconfigured, or running a different version. |
+| Re-run of webhook validation | Go code | NGF control plane     | Field values            | Errors are reported as Event for the resource.                                   | No                                                                                   |
 | NGF-specific validation      | Go code | NGF control plane     | Field values            | Errors are reported in the status of a resource after its creation/modification. | No                                                                                   |
 
 
@@ -153,6 +182,14 @@ We will not introduce any NGF webhook in the cluster (it adds operational comple
 source of potential downtime -- a webhook failure disables CRUD operations on the relevant resources) unless we find
 good reasons for that.
 
+### Upgrades
+
+Since NGF will use the validation package from the Gateway API project, when a new release happens, we will need to
+upgrade the dependency and release a new version of NGF, provided that the validation code changed. However, if it did
+not change, we do not need to release a new version. Note: other things from a new Gateway API release might prompt us
+to release a new version like supporting a new field. See also
+[GEP-922](https://gateway-api.sigs.k8s.io/geps/gep-922/#).
+
 ### Reliability
 
 NGF processes two kinds of transactions:
diff --git a/docs/resource-validation.md b/docs/resource-validation.md
@@ -19,13 +19,15 @@ A Gateway API resource (a new resource or an update for the existing one) is val
 
 1. OpenAPI schema validation by the Kubernetes API server.
 2. CEL validation by the Kubernetes API server.
-3. Validation by NGF.
+3. Webhook validation by NGF.
+4. Validation by NGF.
 
 ### For Kubernetes 1.23 and 1.24
 
 1. OpenAPI schema validation by the Kubernetes API server.
 2. Webhook validation by the Gateway API webhook.
-3. Validation by NGF.
+3. Webhook validation by NGF.
+4. Validation by NGF.
 
 To confirm that a resource is valid and accepted by NGF, check that the `Accepted` condition in the resource status
 has the Status field set to `True`. For example, in a status of a valid HTTPRoute, if NGF accepts a parentRef,
@@ -69,7 +71,7 @@ The HTTPRoute "coffee" is invalid: spec.hostnames[0]: Invalid value: "cafe.!@#$%
 ```
 
 > While unlikely, bypassing this validation step is possible if the Gateway API CRDs are modified to remove the validation.
-> If this happens, Step 3 will reject any invalid values (from NGINX perspective).
+> If this happens, Step 4 will reject any invalid values (from NGINX perspective).
 
 ### Step 2 - For Kubernetes 1.25+ - CEL Validation by Kubernetes API Server
 
@@ -104,7 +106,34 @@ kubectl apply -f some-gateway.yaml
 Error from server: error when creating "some-gateway.yaml": admission webhook "validate.gateway.networking.k8s.io" denied the request: spec.listeners[1].hostname: Forbidden: should be empty for protocol TCP
 ```
 
-### Step 3 - Validation by NGF
+> Bypassing this validation step is possible if the webhook is not running in the cluster.
+> If this happens, Step 3 will reject the invalid values.
+
+### Step 3 - Webhook validation by NGF
+To ensure that the resources are validated with the webhook validation rules, even if the webhook is not running,
+NGF performs the same validation. However, NGF performs the validation *after* the Kubernetes API server accepts
+the resource.
+
+Below is an example of how NGF rejects an invalid resource (a Gateway resource with a TCP listener that configures a
+hostname) with a Kubernetes event:
+
+```shell
+kubectl describe gateway some-gateway
+```
+
+```text
+. . .
+Events:
+  Type     Reason    Age   From                            Message
+  ----     ------    ----  ----                            -------
+  Warning  Rejected  6s    nginx-gateway-fabric-nginx  the resource failed webhook validation, however the Gateway API webhook failed to reject it with the error; make sure the webhook is installed and running correctly; validation error: spec.listeners[1].hostname: Forbidden: should be empty for protocol TCP; NGF will delete any existing NGINX configuration that corresponds to the resource
+```
+
+> This validation step always runs and cannot be bypassed.
+> NGF will ignore any resources that fail the webhook validation, like in the example above.
+> If the resource previously existed, NGF will remove any existing NGINX configuration for that resource.
+
+### Step 4 - Validation by NGF
 
 This step catches the following cases of invalid values:
 
