From b185ab4a9f4e12476e99cbb580f134e6084c6d53 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Wed, 20 Sep 2023 20:57:41 +0200 Subject: [PATCH] Apply suggested spelling improvements Co-authored-by: Josh Soref <2119212+jsoref@users.noreply.github.com> Signed-off-by: Erik Godding Boye --- design/20230726-cel-policy.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/design/20230726-cel-policy.md b/design/20230726-cel-policy.md index 15f2aefd..43f32333 100644 --- a/design/20230726-cel-policy.md +++ b/design/20230726-cel-policy.md @@ -2,7 +2,7 @@ This template is adapted from Kubernetes Enchancements KEP template https://raw.githubusercontent.com/kubernetes/enhancements/a86942e8ba802d0035ec7d4a9c992f03bca7dce9/keps/NNNN-kep-template/README.md --> -# CEL validation rules to approve or deny CertificateRequest +# CEL validation rules to approve or deny a CertificateRequest - [Release Signoff Checklist](#release-signoff-checklist) @@ -52,7 +52,7 @@ A good summary is probably around a paragraph in length. --> While approver-policy allows policies to include namespace selectors, this does not scale when attempting to -enforce CSR attributes that must be a function of the CertificateRequest namespace (or other CertificateRequest fields). +enforce CSR attributes that must be a function of the CertificateRequest's namespace (or other CertificateRequest fields). This proposal suggests an extension to the CertificateRequestPolicy API allowing policy authors to specify approval rules using [CEL in Kubernetes](https://kubernetes.io/docs/reference/using-api/cel/) using variables with information from the CertificateRequest. @@ -72,7 +72,7 @@ List specific goals. What is this proposal trying to achieve? How will we know that this has succeeded? --> -- allow policy authors to express `allowed` CSR attribute value(s) to be a function of CertificateRequest fields using CEL expressions +- allow policy authors to express `allowed` CSR attribute value(s) to be a function of CertificateRequest's fields using CEL expressions - include variables with information from the CertificateRequest and CSR attribute(s) in the CEL context provided to expressions - validate CEL expressions in CertificateRequestPolicy on admission - extensions to the CertificateRequestPolicy API should be backwards compatible @@ -100,21 +100,21 @@ The current CertificateRequestPolicy API allows a policy author to specify `allo CertificateRequest, and will deny the request if it contains an attribute which is not present in the `allowed` block. To configure valid CSR attribute values, the API provides fields for allowed `values` (array; any element matching the attribute is sufficient), -`value` (must match the attribute) or an array of literals/enums (in `usages`) depending on the request attribute. +`value` (must match the attribute) or an array of literals/enums (in `usages`) depending on the requested attribute. Allowed string fields accept wildcards `"*"` within its value(s). This proposal suggests to add support for specifying allowed string fields using CEL expressions, similar to the existing wildcard support. While CEL expressions could eventually replace the current CSR attribute validation API - including the -wildcard support, the main improvement would be support for policies that specifies an allowed attribute value that depends +wildcard support, the main improvement would be support for policies that specify an allowed attribute value that depends on CertificateRequest fields, e.g. `namespace`. If implemented, this new feature would close [#62](https://github.com/cert-manager/approver-policy/issues/62). An [experimental approver-policy plugin](https://github.com/erikgb/cel-approver-policy-plugin) providing some of the requested features exists, and the proposal (including alternatives) is inspired by this plugin. -The main downside of using a plugin for this, in addition increased maintenance/complexity, +The main downside of using a plugin for this, in addition to increased maintenance/complexity, is that the approver-policy rules are still in play. -Which means that approver rules are somehow duplicated in the `allowed` block and in the plugin configuration and -should be avoided. +Which means that approver rules are somehow duplicated in the `allowed` block and in the plugin configuration but +this should be avoided as maintaining things in multiple places is error prone. ### User Stories (Optional) @@ -330,9 +330,9 @@ not need to be as detailed as the proposal, but should include enough information to express the idea and why it was not acceptable. --> -### CEL expressions with similar semantics as values +### CEL expressions with semantics similar to values -_Note: This was the original proposal, after some consideration it is considered inferior to the current proposal, +_Note: This was the original proposal, after some consideration it was considered inferior to the current proposal, mainly because we think we should align this CEL API extension more towards how Kubernetes supports CEL._ The proposal is to add sibling properties to `values` and `value` properties inside the `allowed` block. The new @@ -381,7 +381,7 @@ This alternative was suggested by @inteon on Slack. The idea here is to provide the policy author with a CEL context variable containing the decoded CSR replacing the `self` variable in the proposed design. Since the expressions now get access to the full CSR, the expressions will have to be "promoted" in the CertificateRequestPolicy API. The expression will have to return a bool indicating approved or -denied. An example policy with this alternative API might look like (example); +denied. An example policy with this alternative API might look like (example): ```yaml spec: