search

cert-manager / csi-lib

A library for building CSI drivers that request certificates from cert-manager
Apache License 2.0
14 stars 13 forks source link

Expect PEM data to be returned by SignRequestFunc instead of DER #6

Closed munnerz closed 3 years ago

munnerz commented 3 years ago

This PR changes the public/exported interface (that consumers will implement) for the library to shift the responsibility for encoding to PEM to the implementor of the library.

This is to allow implementors to add e.g. trailing PEM comments (which are permitted by both the PEM spec and the Certificate(Signing)Request API). These may be used for any purpose, including (but not limited to) attaching additional attestation information that does not fit within the x509 document.

JoshVanL commented 3 years ago

Looks good to me, this is what I would expect as a developer.

/lgtm

munnerz commented 3 years ago

@jtweaver this is a breaking change to our Golang API and will require any implementations of the driver to update their code to encode the CSR bytes as PEM before returning (in SignRequest) FYI

munnerz commented 3 years ago

Wake up Tide, please? 🙄 🤖

SgtCoDFish commented 3 years ago

here goes nothing:

/hold

SgtCoDFish commented 3 years ago

/unhold

EDIT: well, that didn't work :)

jetstack-bot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoshVanL, munnerz

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/csi-lib/blob/main/OWNERS)~~ [JoshVanL,munnerz] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
munnerz commented 3 years ago

/hold

munnerz commented 3 years ago

/unhold

munnerz commented 3 years ago

/test all