go back to using short prefix on domain (#551)

* ran hack/update-deps.sh as well

go.mod

@@ -6,8 +6,6 @@ require (
 	github.com/cert-manager/cert-manager v1.8.0
 	github.com/ghodss/yaml v1.0.0
 	github.com/google/go-cmp v0.5.9
-	github.com/google/uuid v1.3.0
-	github.com/martinlindhe/base36 v1.1.1
 	go.uber.org/zap v1.19.1
 	k8s.io/api v0.26.5
 	k8s.io/apimachinery v0.26.5
@@ -40,6 +38,7 @@ require (
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
+	github.com/google/uuid v1.3.0 // indirect
 	github.com/gorilla/websocket v1.4.2 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.11.3 // indirect
 	github.com/hashicorp/golang-lru v0.5.4 // indirect

go.sum

@@ -387,8 +387,6 @@ github.com/mailru/easyjson v0.7.0/go.mod h1:KAzv3t3aY1NaHWoQz1+4F1ccyAH66Jk7yos7
 github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
 github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
-github.com/martinlindhe/base36 v1.1.1 h1:1F1MZ5MGghBXDZ2KJ3QfxmiydlWOGB8HCEtkap5NkVg=
-github.com/martinlindhe/base36 v1.1.1/go.mod h1:vMS8PaZ5e/jV9LwFKlm0YLnXl/hpOihiBxKkIoc3g08=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
 github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=

pkg/reconciler/certificate/certificate.go

@@ -123,10 +123,10 @@ func (c *Reconciler) reconcile(ctx context.Context, knCert *v1alpha1.Certificate
 	switch {
 	case cmCertReadyCondition == nil:
 		knCert.Status.MarkNotReady(noCMConditionReason, noCMConditionMessage)
-		return c.setHTTP01Challenges(knCert, cmCert)
+		return c.setHTTP01Challenges(ctx, knCert, cmCert)
 	case cmCertReadyCondition.Status == cmmeta.ConditionUnknown:
 		knCert.Status.MarkNotReady(cmCertReadyCondition.Reason, cmCertReadyCondition.Message)
-		return c.setHTTP01Challenges(knCert, cmCert)
+		return c.setHTTP01Challenges(ctx, knCert, cmCert)
 	case cmCertReadyCondition.Status == cmmeta.ConditionTrue:
 		if cmCert.Status.RenewalTime != nil && time.Now().After(cmCert.Status.RenewalTime.Time) {
 			// add a temporary renewing state when cm certificate is being renewed
@@ -139,7 +139,7 @@ func (c *Reconciler) reconcile(ctx context.Context, knCert *v1alpha1.Certificate
 				Status: corev1.ConditionTrue,
 			}
 			certificateCondSet.Manage(&knCert.Status).SetCondition(renewCondition)
-			return c.setHTTP01Challenges(knCert, cmCert)
+			return c.setHTTP01Challenges(ctx, knCert, cmCert)
 		}
 		// remove renew condition if exists
 		certificateCondSet.Manage(&knCert.Status).ClearCondition(renewingEvent)
@@ -151,7 +151,7 @@ func (c *Reconciler) reconcile(ctx context.Context, knCert *v1alpha1.Certificate
 		} else {
 			knCert.Status.MarkFailed(cmCertReadyCondition.Reason, cmCertReadyCondition.Message)
 		}
-		return c.setHTTP01Challenges(knCert, cmCert)
+		return c.setHTTP01Challenges(ctx, knCert, cmCert)
 	}
 	return nil
 }
@@ -190,7 +190,8 @@ func (c *Reconciler) reconcileCMCertificate(ctx context.Context, knCert *v1alpha
 	return cmCert, nil
 }
 
-func (c *Reconciler) setHTTP01Challenges(knCert *v1alpha1.Certificate, cmCert *cmv1.Certificate) error {
+func (c *Reconciler) setHTTP01Challenges(ctx context.Context, knCert *v1alpha1.Certificate, cmCert *cmv1.Certificate) error {
+	logger := logging.FromContext(ctx)
 	if isHTTP, err := c.isHTTPChallenge(cmCert); err != nil {
 		return err
 	} else if !isHTTP {
@@ -213,8 +214,14 @@ func (c *Reconciler) setHTTP01Challenges(knCert *v1alpha1.Certificate, cmCert *c
 			return fmt.Errorf("failed to list services: %w", err)
 		}
 		if len(svcs) == 0 {
-			//If the cert is renewing, it could be possible that this isn't an error. Should this change depending on the case?
-			return fmt.Errorf("no challenge solver service for domain %s; selector=%v", dnsName, selector)
+			if dnsName == resources.Prefix+knCert.Spec.Domain {
+				logger.Info("No challenge service found for shortened commonname, could be cached? continuing")
+				continue
+			} else {
+				//If the cert is renewing, it could be possible that this isn't an error. Should this change depending on the case?
+				return fmt.Errorf("no challenge solver service for domain %s; selector=%v", dnsName, selector)
+
+			}
 		}
 
 		for _, svc := range svcs {

pkg/reconciler/certificate/certificate_test.go

@@ -62,7 +62,8 @@ import (
 const generation = 23132
 
 var (
-	correctDNSNames   = []string{"k.example.com", "correct-dns1.example.com", "correct-dns2.example.com"}
+	correctDNSNames   = []string{"correct-dns1.example.com", "correct-dns2.example.com"}
+	shortenedDNSNames = []string{"k.example.com", "reallyreallyreallyreallyreallyreallylongname.namespace.example.com"}
 	incorrectDNSNames = []string{"incorrect-dns.example.com"}
 	exampleDomain     = "example.com"
 	notAfter          = &metav1.Time{
@@ -95,8 +96,9 @@ var (
 		},
 	}
 
-	externalCert, _ = resources.MakeCertManagerCertificate(certmanagerConfig(), knCert("knCert", "foo"))
-	internalCert, _ = resources.MakeCertManagerCertificate(certmanagerConfig(), withClusterLocalVisibility(knCert("knCert", "foo")))
+	externalCert, _                  = resources.MakeCertManagerCertificate(certmanagerConfig(), knCert("knCert", "foo"))
+	internalCert, _                  = resources.MakeCertManagerCertificate(certmanagerConfig(), withClusterLocalVisibility(knCert("knCert", "foo")))
+	externalCertShortenedDNSNames, _ = resources.MakeCertManagerCertificate(certmanagerConfig(), knCertShortenedDNSNames("knCert", "foo"))
 )
 
 func TestNewController(t *testing.T) {
@@ -332,7 +334,7 @@ func TestReconcile(t *testing.T) {
 		},
 		WantErr: true,
 		WantEvents: []string{
-			"Warning InternalError error creating cert-manager certificate: cannot create valid length CommonName: (hello.ns.reallyreallyreallyreallyreallyreallyreallylong.domainname) still longer than 63 characters, cannot shorten",
+			"Warning InternalError error creating cert-manager certificate: CommonName (reallyreallyreallyreallyreallyreallyreallyreallylong.domainname)(length: 63) too long, prepending short prefix of (k.)(length: 2) will be longer than 64 bytes",
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: knCertDomainTooLong("knCert", "foo",
@@ -345,7 +347,7 @@ func TestReconcile(t *testing.T) {
 								Status:   corev1.ConditionFalse,
 								Severity: apis.ConditionSeverityError,
 								Reason:   "CommonName Too Long",
-								Message:  "error creating cert-manager certificate: cannot create valid length CommonName: (hello.ns.reallyreallyreallyreallyreallyreallyreallylong.domainname) still longer than 63 characters, cannot shorten",
+								Message:  "error creating cert-manager certificate: CommonName (reallyreallyreallyreallyreallyreallyreallyreallylong.domainname)(length: 63) too long, prepending short prefix of (k.)(length: 2) will be longer than 64 bytes",
 							},
 						},
 					},
@@ -555,7 +557,7 @@ func TestReconcile_HTTP01Challenges(t *testing.T) {
 		},
 		WantEvents: []string{
 			Eventf(corev1.EventTypeNormal, "Created", "Created Cert-Manager Certificate %s/%s", "foo", "knCert"),
-			Eventf(corev1.EventTypeWarning, "InternalError", "no challenge solver service for domain %s; selector=acme.cert-manager.io/http-domain=574162163", correctDNSNames[0]),
+			Eventf(corev1.EventTypeWarning, "InternalError", "no challenge solver service for domain %s; selector=acme.cert-manager.io/http-domain=1930889501", correctDNSNames[0]),
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: knCertWithStatus("knCert", "foo",
@@ -578,10 +580,8 @@ func TestReconcile_HTTP01Challenges(t *testing.T) {
 		Objects: []runtime.Object{
 			cmSolverService(correctDNSNames[0], "foo"),
 			cmSolverService(correctDNSNames[1], "foo"),
-			cmSolverService(correctDNSNames[2], "foo"),
 			cmChallenge(correctDNSNames[0], "foo"),
 			cmChallenge(correctDNSNames[1], "foo"),
-			cmChallenge(correctDNSNames[2], "foo"),
 			cmCert("knCert", "foo", correctDNSNames),
 			knCert("knCert", "foo"),
 			http01Issuer,
@@ -605,14 +605,6 @@ func TestReconcile_HTTP01Challenges(t *testing.T) {
 						},
 						ServiceName:      "cm-solver-" + correctDNSNames[1],
 						ServiceNamespace: "foo",
-					}, {
-						URL: &apis.URL{
-							Scheme: "http",
-							Host:   correctDNSNames[2],
-							Path:   "/.well-known/acme-challenge/cm-challenge-token",
-						},
-						ServiceName:      "cm-solver-" + correctDNSNames[2],
-						ServiceNamespace: "foo",
 					}},
 					Status: duckv1.Status{
 						ObservedGeneration: generation,
@@ -632,10 +624,8 @@ func TestReconcile_HTTP01Challenges(t *testing.T) {
 		Objects: []runtime.Object{
 			cmSolverService(correctDNSNames[0], "foo"),
 			cmSolverService(correctDNSNames[1], "foo"),
-			cmSolverService(correctDNSNames[2], "foo"),
 			cmChallenge(correctDNSNames[0], "foo"),
 			cmChallenge(correctDNSNames[1], "foo"),
-			cmChallenge(correctDNSNames[2], "foo"),
 			cmCertWithStatus("knCert", "foo", correctDNSNames, []cmv1.CertificateCondition{{
 				Type:   cmv1.CertificateConditionReady,
 				Status: cmmeta.ConditionFalse,
@@ -663,14 +653,6 @@ func TestReconcile_HTTP01Challenges(t *testing.T) {
 						},
 						ServiceName:      "cm-solver-" + correctDNSNames[1],
 						ServiceNamespace: "foo",
-					}, {
-						URL: &apis.URL{
-							Scheme: "http",
-							Host:   correctDNSNames[2],
-							Path:   "/.well-known/acme-challenge/cm-challenge-token",
-						},
-						ServiceName:      "cm-solver-" + correctDNSNames[2],
-						ServiceNamespace: "foo",
 					}},
 					Status: duckv1.Status{
 						ObservedGeneration: generation,
@@ -683,6 +665,49 @@ func TestReconcile_HTTP01Challenges(t *testing.T) {
 					},
 				}),
 		}},
+	}, {
+		//It is possible for a challenge to not be created for a k.{{Domain}} dnsname, since it may have already been created in a previous Kservice
+		Name: "set Status.HTTP01Challenges on Knative certificate when shortened domain with prefix (k.) is reused",
+		Key:  "foo/knCert",
+		Objects: []runtime.Object{
+			cmSolverService(shortenedDNSNames[1], "foo"),
+			cmChallenge(shortenedDNSNames[1], "foo"),
+			cmCert("knCert", "foo", shortenedDNSNames),
+			knCertShortenedDNSNames("knCert", "foo"),
+			http01Issuer,
+		},
+		WantStatusUpdates: []clientgotesting.UpdateActionImpl{
+			{
+				Object: knCertShortenedDNSNamesWithStatus("knCert", "foo",
+					&v1alpha1.CertificateStatus{
+						HTTP01Challenges: []v1alpha1.HTTP01Challenge{{
+							URL: &apis.URL{
+								Scheme: "http",
+								Host:   shortenedDNSNames[1],
+								Path:   "/.well-known/acme-challenge/cm-challenge-token",
+							},
+							ServiceName:      "cm-solver-" + shortenedDNSNames[1],
+							ServiceNamespace: "foo",
+						}},
+						Status: duckv1.Status{
+							ObservedGeneration: generation,
+							Conditions: duckv1.Conditions{{
+								Type:     v1alpha1.CertificateConditionReady,
+								Status:   corev1.ConditionUnknown,
+								Severity: apis.ConditionSeverityError,
+								Reason:   noCMConditionReason,
+								Message:  noCMConditionMessage,
+							}},
+						},
+					}),
+			},
+		},
+		WantUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: externalCertShortenedDNSNames,
+		}},
+		WantEvents: []string{
+			Eventf(corev1.EventTypeNormal, "Updated", "Updated Spec for Cert-Manager Certificate %s/%s", "foo", "knCert"),
+		},
 	}}
 
 	table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {
@@ -733,6 +758,12 @@ func knCert(name, namespace string) *v1alpha1.Certificate {
 	return knCertWithStatus(name, namespace, &v1alpha1.CertificateStatus{})
 }
 
+func knCertShortenedDNSNames(name, namespace string) *v1alpha1.Certificate {
+	cert := knCertWithStatus(name, namespace, &v1alpha1.CertificateStatus{})
+	cert.Spec.DNSNames = shortenedDNSNames
+	return cert
+}
+
 func knCertDomainTooLong(name, namespace string, status *v1alpha1.CertificateStatus, gen int) *v1alpha1.Certificate {
 	return &v1alpha1.Certificate{
 		ObjectMeta: metav1.ObjectMeta{
@@ -744,8 +775,8 @@ func knCertDomainTooLong(name, namespace string, status *v1alpha1.CertificateSta
 			},
 		},
 		Spec: v1alpha1.CertificateSpec{
-			DNSNames:   []string{"hello.ns.reallyreallyreallyreallyreallyreallyreallylong.domainname"},
-			Domain:     "reallyreallyreallyreallyreallyreallyreallylong.domainname",
+			DNSNames:   []string{"hello.ns.reallyreallyreallyreallyreallyreallyreallyreallylong.domainname"},
+			Domain:     "reallyreallyreallyreallyreallyreallyreallyreallylong.domainname",
 			SecretName: "secret0",
 		},
 		Status: *status,
@@ -756,6 +787,12 @@ func knCertWithStatus(name, namespace string, status *v1alpha1.CertificateStatus
 	return knCertWithStatusAndGeneration(name, namespace, status, generation)
 }
 
+func knCertShortenedDNSNamesWithStatus(name, namespace string, status *v1alpha1.CertificateStatus) *v1alpha1.Certificate {
+	cert := knCertWithStatus(name, namespace, status)
+	cert.Spec.DNSNames = shortenedDNSNames
+	return cert
+}
+
 func knCertWithStatusAndGeneration(name, namespace string, status *v1alpha1.CertificateStatus, gen int) *v1alpha1.Certificate {
 	return &v1alpha1.Certificate{
 		ObjectMeta: metav1.ObjectMeta{