Skip to content

Commit

Permalink
Check the ClusterDomainClaim before creating kcert. (#12085)
Browse files Browse the repository at this point in the history
I unfortunately ran afoul of a change in the default behavior where
`DomainMapping` stopped creating `ClusterDomainClaim` by default, and
I unfortunately had auto-TLS enabled, and before I could enable this
creation, my Let's Encrypt quota for the week had been exhausted.

It seems like the best course all around is to verify the domain has
been claimed before we direct `kcert` to try provisioning a certificate,
which will otherwise 404.

Co-authored-by: Matt Moore <[email protected]>
  • Loading branch information
knative-prow-robot and mattmoor authored Oct 7, 2021
1 parent 430a9ac commit ce57365
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 9 deletions.
10 changes: 5 additions & 5 deletions pkg/reconciler/domainmapping/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,6 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, dm *v1alpha1.DomainMappi
dm.Status.URL = url
dm.Status.Address = &duckv1.Addressable{URL: url}

tls, acmeChallenges, err := r.tls(ctx, dm)
if err != nil {
return err
}

// IngressClass can be set via annotations or in the config map.
ingressClass := dm.Annotations[networking.IngressClassAnnotationKey]
if ingressClass == "" {
Expand All @@ -111,6 +106,11 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, dm *v1alpha1.DomainMappi
return err
}

tls, acmeChallenges, err := r.tls(ctx, dm)
if err != nil {
return err
}

// Resolve the spec.Ref to a URI following the Addressable contract.
targetHost, targetBackendSvc, err := r.resolveRef(ctx, dm)
if err != nil {
Expand Down
6 changes: 2 additions & 4 deletions pkg/reconciler/domainmapping/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,6 @@ func TestReconcile(t *testing.T) {
withURL("http", "first-reconcile.com"),
withAddress("http", "first-reconcile.com"),
withInitDomainMappingConditions,
withTLSNotEnabled,
),
}},
WantCreates: []runtime.Object{
Expand Down Expand Up @@ -395,7 +394,6 @@ func TestReconcile(t *testing.T) {
withURL("http", "first-reconcile.com"),
withAddress("http", "first-reconcile.com"),
withInitDomainMappingConditions,
withTLSNotEnabled,
withDomainClaimNotOwned,
),
}},
Expand Down Expand Up @@ -710,7 +708,6 @@ func TestReconcileAutocreateClaimsDisabled(t *testing.T) {
withURL("http", "first-reconcile.com"),
withAddress("http", "first-reconcile.com"),
withInitDomainMappingConditions,
withTLSNotEnabled,
withDomainClaimNotOwned,
),
}},
Expand Down Expand Up @@ -772,7 +769,6 @@ func TestReconcileAutocreateClaimsDisabled(t *testing.T) {
withURL("http", "first-reconcile.com"),
withAddress("http", "first-reconcile.com"),
withInitDomainMappingConditions,
withTLSNotEnabled,
withDomainClaimNotOwned,
),
}},
Expand Down Expand Up @@ -951,6 +947,7 @@ func TestReconcileTLSEnabled(t *testing.T) {
withRef("default", "ready"),
withURL("http", "cert.not.owned.ru"),
withAddress("http", "cert.not.owned.ru"),
withDomainClaimed,
withCertificateNotOwned,
withInitDomainMappingConditions,
),
Expand Down Expand Up @@ -991,6 +988,7 @@ func TestReconcileTLSEnabled(t *testing.T) {
withRef("default", "ready"),
withURL("http", "cert.creation.failed.ly"),
withAddress("http", "cert.creation.failed.ly"),
withDomainClaimed,
withCertificateFail,
withInitDomainMappingConditions,
),
Expand Down

0 comments on commit ce57365

Please sign in to comment.