Skip to content

Commit

Permalink
fix(certificaterequest): ignore empty issuer group
Browse files Browse the repository at this point in the history
A CertificateRequest's reference to an Issuer can have an empty Group,
which we were considering as equivalent to matching, then later
rejecting the CertificateRequest when it was for an unknown issuer.
While setting the group is optional in the schema, our documentation has
always included the group in the issuer reference. The rejection was
racing with other controllers, including the built-in controllers, and
preventing them from issuing certificates.

This changeset updates CertificateRequestController to ignore
CertificateRequests where the issuer group is unset, preventing the
race with controllers for other issuers. We'll continue to reject
CertificateRequests for unknown kinds if the group matches our group.

Bug: #149
Fixes: d8c1b77 ("feat: add ClusterOriginIssuer")
  • Loading branch information
terinjokes committed Nov 1, 2024
1 parent ec06da1 commit 2ed2a72
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkgs/controllers/certificaterequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type CertificateRequestController struct {
func (r *CertificateRequestController) Reconcile(ctx context.Context, cr *certmanager.CertificateRequest) (reconcile.Result, error) {
log := r.Log.WithValues("namespace", cr.Namespace, "certificaterequest", cr.Name)

if cr.Spec.IssuerRef.Group != "" && cr.Spec.IssuerRef.Group != v1.GroupVersion.Group {
if cr.Spec.IssuerRef.Group != v1.GroupVersion.Group {
log.V(4).Info("resource does not specify an issuerRef group name that we are responsible for", "group", cr.Spec.IssuerRef.Group)

return reconcile.Result{}, nil
Expand Down
51 changes: 51 additions & 0 deletions pkgs/controllers/certificaterequest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,57 @@ func TestCertificateRequestReconcile(t *testing.T) {
Name: "foobar",
},
},
{
name: "ignores CertificateRequests with empty Issuer group reference",
objects: []runtime.Object{
cmgen.CertificateRequest("foobar",
cmgen.SetCertificateRequestNamespace("default"),
cmgen.SetCertificateRequestDuration(&metav1.Duration{Duration: 7 * 24 * time.Hour}),
cmgen.SetCertificateRequestCSR(golden.Get(t, "csr.golden")),
cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
Name: "foobar",
Kind: "StepIssuer", // 👋 hello friends!
}),
),
&v1.ClusterOriginIssuer{
ObjectMeta: metav1.ObjectMeta{
Name: "foobar",
},
Spec: v1.OriginIssuerSpec{
RequestType: v1.RequestTypeOriginECC,
Auth: v1.OriginIssuerAuthentication{
TokenRef: &v1.SecretKeySelector{
Name: "token-issuer",
Key: "token",
},
},
},
Status: v1.OriginIssuerStatus{
Conditions: []v1.OriginIssuerCondition{
{
Type: v1.ConditionReady,
Status: v1.ConditionTrue,
},
},
},
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "token-issuer",
Namespace: "super-secret",
},
Data: map[string][]byte{
"token": []byte("api-token"),
},
},
},
recorder: RecorderMust(t, "testdata/working"),
expected: cmapi.CertificateRequestStatus{},
namespaceName: types.NamespacedName{
Namespace: "default",
Name: "foobar",
},
},
{
name: "OriginIssuer without authentication",
objects: []runtime.Object{
Expand Down

0 comments on commit 2ed2a72

Please sign in to comment.