check for renewing status

[KauzClay]

Fixes # knative-extensions/net-certmanager#416
Related to # knative-extensions/net-certmanager#482

Proposed Changes

• Trying to implement item 3 in this comment: Renewing the certificate doesn't work as expected and it will call user ksvc for 30 days  knative-extensions/net-certmanager#416 (comment)

Release Note

Fixes issue where certificates would not get renewed when using auto-tls.

[codecov]

Codecov Report

Base: 86.22% // Head: 86.14% // Decreases project coverage by -0.08% ⚠️

Coverage data is based on head (02a1c3f) compared to base (b4d7a28).
Patch coverage: 46.66% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13666      +/-   ##
==========================================
- Coverage   86.22%   86.14%   -0.08%     
==========================================
  Files         197      197              
  Lines       14783    14790       +7     
==========================================
- Hits        12746    12741       -5     
- Misses       1735     1745      +10     
- Partials      302      304       +2     

Impacted Files	Coverage Δ
pkg/reconciler/route/route.go	79.70% <46.66%> (-1.53%)	⬇️
pkg/reconciler/revision/resolve.go	77.50% <0.00%> (-7.50%)	⬇️
pkg/reconciler/route/resources/ingress.go	94.80% <0.00%> (-0.20%)	⬇️
pkg/queue/sharedmain/main.go	0.61% <0.00%> (-0.01%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[KauzClay]

For visibility, I've been testing this by changing the cmCert spec to add a RenewBefore field.

Since I'm using letsencrypt, the default lifetime of a cert is 90 days, and you can't really change it. But you can edit the RenewBefore field to force a renewal.

Because of some letsencrypt + certmanager stuff I don't understand, the notBefore time is actually an hour before the cert is created. So if you set it to renew 1hr2mins after it was ready, you can force a renewal every 2 minutes. If you do exactly an hour, it gets kinda stuck in a renewal loop.

anway, the cmCert spec then looks something like this:

cert := &cmv1.Certificate{
		ObjectMeta: metav1.ObjectMeta{
			Name:            knCert.Name,
			Namespace:       knCert.Namespace,
			OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(knCert)},
			Annotations:     knCert.GetAnnotations(),
			Labels:          knCert.GetLabels(),
		},
		Spec: cmv1.CertificateSpec{
			CommonName: commonName,
			SecretName: knCert.Spec.SecretName,
			DNSNames:   dnsNames,
			IssuerRef:  *cmConfig.IssuerRef,
			RenewBefore: &metav1.Duration{
				Duration: 129538 * time.Minute,
			},
			SecretTemplate: &cmv1.CertificateSecretTemplate{
				Labels: map[string]string{
					networking.CertificateUIDLabelKey: string(knCert.GetUID()),
				}},
		},
	}

Unfortunately, this doesn't seem to create the new http01 challenge endpoints. So I can't fully validate if the renewal change actually works. Still trying to understand why that is.

UPDATE: I asked around and learned that LetsEncrypt will cache challenge authorizations for 30 days, see here. So even though I can force a renew, no challenge gets created since it is in the cache.

UPDATE 2: So, you can clear this cache by deleting the letsencrypt issuer + secret and recreating the issuer, as explained here. I was able to do this, and could see the certificate get renewed, and see that the new HTTP01 Challenges were added to the kcert status.

[dprotaso]

We should add a unit test.

Similarly, we might be able to add an e2e test that users our self-signed cluster issuer

[dprotaso]

Going into the else flow will mark the route not ready - which we don't want since it can still serve traffic.

[KauzClay]

@dprotaso I added the unit test

Regarding this though:

Similarly, we might be able to add an e2e test that users our self-signed cluster issuer

I don't know of a way to make a self signed issuer to http01 challenges. The selfsigned issuer definition doesn't have a place to define solvers from what I can tell:

❯ k explain clusterissuer.spec.selfSigned
W0213 14:08:10.927054   69927 gcp.go:119] WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.26+; use gcloud instead.
To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
KIND:     ClusterIssuer
VERSION:  cert-manager.io/v1

RESOURCE: selfSigned <Object>

DESCRIPTION:
     SelfSigned configures this issuer to 'self sign' certificates using the
     private key used to create the CertificateRequest object.

FIELDS:
   crlDistributionPoints	<[]string>
     The CRL distribution points is an X.509 v3 certificate extension which
     identifies the location of the CRL from which the revocation of this
     certificate can be checked. If not set certificate will be issued without
     CDP. Values are strings.

[dprotaso]

Leaving the hold in case you want to address the nit - feel free to drop it

/lgtm
/approve
/hold

[dprotaso]

nit: You might be able to simplify this test by lumping it into the following table test

serving/pkg/reconciler/route/table_test.go

Line 2344 in a5e0f22

func TestReconcileEnableAutoTLS(t *testing.T) {

Those table tests test a single Reconcile call and will assert create/update etc. occurs

[KauzClay]

ah okay, I'll give that a try

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, KauzClay

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 [dprotaso]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dprotaso]

/lgtm
/hold cancel

[KauzClay]

/test istio-latest-no-mesh