Skip to content

Commit

Permalink
Apply suggested spelling improvements
Browse files Browse the repository at this point in the history
Co-authored-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
  • Loading branch information
erikgb and jsoref committed Sep 20, 2023
1 parent d918aea commit b185ab4
Showing 1 changed file with 11 additions and 11 deletions.
22 changes: 11 additions & 11 deletions design/20230726-cel-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

<!-- toc -->
- [Release Signoff Checklist](#release-signoff-checklist)
Expand Down Expand Up @@ -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.

Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit b185ab4

Please sign in to comment.