Apply feedback

Author: jpbetz

keps/sig-api-machinery/3488-cel-admission-control/README.md

@@ -1024,21 +1024,9 @@ this enhancement.
 
 #### Enforcement Actions
 
-For parity with admission webhooks, a validating policy may also emit audit
-annotations and warnings:
-
-- [Audit
+`ValidatingAdmissionPolicy` may declare [Audit
   annotations](https://github.com/kubernetes/kubernetes/blob/97bbf07d3f3f20332912ee411fdf75ce84425e28/staging/src/k8s.io/api/admission/v1/types.go#L142)
-  are key/value pairs included in the audit event for an admission request. The
-  audit annotation key supplied by the policy definition author will be prefixed
-  with the name of the `ValidatingAdmissionPolicy` and policy binding, e.g.:
-  `mypolicy.mygroup.example.com/mybinding.mygroup.example.com/<annotation-key>`.
-- [Warnings](https://kubernetes.io/blog/2020/09/03/warnings/#admission-webhooks)
-  are string messages that are returned to API clients. Warning are returned for
-  both requests that are accepted and requests that are rejected.
-
-`ValidatingAdmissionPolicy` may declare audit annotations in the policy
-definition. E.g.:
+  in the policy definition. E.g.:
 
 ```yaml
 apiVersion: admissionregistration.k8s.io/v1alpha1
@@ -1049,39 +1037,50 @@ spec:
   validations:
     - expression: <expression>
   auditAnnotations:
-    - includeWhen: <expression> # optional field
-      key: "my-audit-key"
+    - key: "my-audit-key"
       valueExpression: <expression that evaluates to a string>
 ```
 
-Additionally, `ValidatingAdmissionPolicyBinding` resource may control how
-admission is enforced. This is performed using a single field. E.g.:
+The published annotation key will be of the form `<ValidatingPolicyDefinition
+name>/<auditAnnotation key>` and will be validated as a
+[QualifiedName]https://github.com/kubernetes/kubernetes/blob/dfa4143086bf504c6c72d5eee8a2210b8ed41b9a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go#L43).
+
+The validation rule will be: `len(key) < QualifierName.maxLength - len(policy
+name) - 1` to accommodate the `<ValidatingPolicyDefinition
+name>/<auditAnnotation key>` audit annotation key format.
+
+If `valueExpression` returns a string, the audit annotation is published.  If
+`valueExpression` returns null, the audit annotation is omitted. No other return
+types will be supported.
+
+`ValidatingAdmissionPolicyBinding` resources may control how admission is
+enforced. This is performed using a single field. E.g.:
 
 ```yaml
 apiVersion: admissionregistration.k8s.io/v1alpha1
 kind: ValidatingAdmissionPolicyBinding
 ...
 spec:
-  enforcement: warn # optional field
+  validationActions: [warn, audit] # optional field
 ```
 
+The enum options will be:
+
 - `deny`: Validation failures result in a denied request. (default beahvior if
   field is unset)
-- `warn`: Validation failures are reported as warnings to the client.
-- `silent`: Validation failures are not reported to clients. Audit annotations
-          for any failed validations are still included in audit events (see
-          below).
-- (To disable audit annotations, delete the binding)
+- `warn`: Validation failures are reported as warnings to the client. (xref: [Admisssion Webhook Warnings](https://kubernetes.io/blog/2020/09/03/warnings/#admission-webhooks))
+- `audit`: Validation failures are published as audit events (see below Audit
+  Annotations section for details).
 
 Systems that need to aggregate validation failures may implement an [audit
 webhook
 backend](https://kubernetes.io/docs/tasks/debug/debug-cluster/audit/#webhook-backend). See
 below "Audit Events" for details.
 
-For singleton policies, the enforcement field will be set on the policy definition.
+For singleton policies, the `validationActions` field will be set on the policy definition.
 
-Metrics will include enforcement so that cluster administrators can monitor the
-validation failures of a binding before setting enforcement to `deny`.
+Metrics will include validation action so that cluster administrators can monitor the
+validation failures of a binding before setting `validationActions` to `deny`.
 
 Supported use cases:
 
@@ -1102,10 +1101,18 @@ Supported use cases:
   enforcement point, higher level constructs can map to this enforcement point
   as needed.
 
+
+Future work:
+
+ValidatingAdmissionPolicy resources might, in the future, add a `warnings` field
+adjacent to the `validations` and `auditAnnotations` fields to declare
+expressions only ever result in warnings. This would allow
+ValidatingAdmissionPolicy authors to declare a expression as non-enforcing
+regardless of `validationActions`.
+
 #### Audit Events
 
-All audit event keys are prefixed by
-`<ValidatingPolicyDefinition name>/<ValidatingPolicyDefinitionBinding name>/`.
+All audit event keys are prefixed by `<ValidatingPolicyDefinition name>/`.
 
 At Metadata audit level or higher, when a validating admission binding fails any
 validation expression, details are included in the audit annotations
@@ -1117,7 +1124,7 @@ for the audit event under the key `validation_failures`. E.g.:
     "kind": "Event",
     "apiVersion": "audit.k8s.io/v1",
     "annotations": {
-        "mypolicy.mygroup.example.com/mybinding.mygroup.example.com/validation_failure": "{\"expression\": 1, \"message\": \"x must be greater than y\", \"enforcement\": \"deny\"}"
+        "mypolicy.mygroup.example.com/validation_failure": "{\"expression\": 1, \"message\": \"x must be greater than y\", \"enforcement\": \"deny\", \"binding\": \"mybinding.mygroup.example.com\"}"
         # other annotations
         ...
     }
@@ -1135,7 +1142,7 @@ are included with the key provided. E.g.:
     "kind": "Event",
     "apiVersion": "audit.k8s.io/v1",
     "annotations": {
-        "mypolicy.mygroup.example.com/mybinding.mygroup.example.com/myauditkey": "my audit value"
+        "mypolicy.mygroup.example.com/myauditkey": "my audit value"
         # other annotations
         ...
     }