Remove SetCertificateRequestConditionError#188
Remove SetCertificateRequestConditionError#188inteon wants to merge 5 commits intocert-manager:mainfrom
Conversation
5564e5d to
7360a53
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
7360a53 to
33b083f
Compare
c906759 to
ee11618
Compare
SgtCoDFish
left a comment
There was a problem hiding this comment.
Got a few comments on this one - I see what you're aiming for.
On a broader note, while I recognise we're v0 with issuer-lib it definitely feels harder to stomach breaking changes in this repo now. There are multiple issuers working today which import this lib: https://pkg.go.dev/github.com/cert-manager/issuer-lib@v0.9.0/controllers?tab=importedby
The dev UX of us making a breaking change really blows, even if we've warned people that it could happen. If we changed the import path (by pushing a new major version for every breaking change) it'd be a lot less painful for downstream consumers.
I'd be supportive of marking issuer-lib as v1 without making any breaking changes, and then immediately working on v2 which could include breaking changes. I don't think that hurts anyone!
I totally agree with Ash. Pre-v1 projects are just too much of a pain, and there is no reason to cause that pain, especially since it would cost almost no effort to cut a v1 now. Regardless of whether the project is v1 material or not, let's avoid needlessly causing pain. Cloudflare folks will thank us. Note that lots of people are developing custom issuers behind closed curtains. We probably only see 1% of the users of this project. |
e89ad9e to
a0aefe2
Compare
a0aefe2 to
cc6c051
Compare
cc6c051 to
e81f61a
Compare
| // | ||
| // All other fields of the condition are ignored and will be set by the | ||
| // controller automatically. | ||
| func WithExtraConditions(conditions ...metav1.Condition) interface { |
There was a problem hiding this comment.
could you add a code snippet showing how an implementor would use this? that would greatly help me see how it's meant to be used
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
…signing async Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
274a803 to
19dd427
Compare
Instead of returning SetCertificateRequestConditionError,
we now have 3 return values for the sign function.we now return a SignResult value.Using the
WithExtraConditionsoption, we can set the custom conditions on a CertificateRequest/ CertificateSigningRequest resource.