cert-manager / approver-policy

approver-policy is a cert-manager approver that allows users to define policies that restrict what certificates can be requested.
https://cert-manager.io/docs/policy/approval/approver-policy/
Apache License 2.0
66 stars 23 forks source link

Add CertificateRequest username to CEL Validator with serviceaccount functions #514

Open jamesglennan opened 1 week ago

jamesglennan commented 1 week ago

Addresses https://github.com/cert-manager/approver-policy/issues/506

This PR is to add the username associated with a certificate request into the CEL Validator, so it is possible to approve or deny certificate requests based on who/what requested them. This is useful specifically for validating SPIFFE SVIDs where the URI field is in the format of spiffe://trust.domain/ns/serviceAccountNamespace/sa/serviceAccountName

cert-manager-prow[bot] commented 1 week ago

Hi @jamesglennan. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
erikgb commented 1 week ago

A summary of our discussion of this on Slack:

@erikgb thinks we have to work a bit more on the UX of this CEL extension:

Something like isServiceAccount(cr.username) && self.matches('spiffe://trust.domain/ns/' + cr.namespace + '/sa/' + serviceAccount(cr.username).getName()) could be a reasonable use-case for this new feature.

SgtCoDFish commented 1 week ago

/ok-to-test

(I don't think tests will run on a draft PR, but at least now they'll run when this is marked ready for review!)

EDIT: Oh, looks like the tests will run! Good to know!

SgtCoDFish commented 1 week ago

I think this looks like a great start. I had a brief review - this isn't really my area of expertise but it looks how I'd expect this to look! 👍

cert-manager-prow[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: erikgb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/cert-manager/approver-policy/blob/main/OWNERS)~~ [erikgb] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment