diff --git a/pkg/issuer/acme/acme.go b/pkg/issuer/acme/acme.go index 300066146..131db4079 100644 --- a/pkg/issuer/acme/acme.go +++ b/pkg/issuer/acme/acme.go @@ -71,6 +71,10 @@ type solver interface { // needs to create any new resources, it can set the appropriate owner // reference Present(ctx context.Context, crt *v1alpha1.Certificate, ch v1alpha1.ACMEOrderChallenge) error + + // Check should return Error only if propagation check cannot be performed. + // It MUST return `false, nil` if can contact all relevant services and all is + // doing is waiting for propagation Check(ch v1alpha1.ACMEOrderChallenge) (bool, error) CleanUp(ctx context.Context, crt *v1alpha1.Certificate, ch v1alpha1.ACMEOrderChallenge) error } diff --git a/pkg/issuer/acme/dns/util/wait.go b/pkg/issuer/acme/dns/util/wait.go index 5583bd8ed..739fd5eb8 100644 --- a/pkg/issuer/acme/dns/util/wait.go +++ b/pkg/issuer/acme/dns/util/wait.go @@ -86,7 +86,8 @@ func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, erro return false, err } - if r.Rcode != dns.RcodeSuccess { + // NXDomain response is not really an error, just waiting for propagation to happen + if !(r.Rcode == dns.RcodeSuccess || r.Rcode == dns.RcodeNameError) { return false, fmt.Errorf("NS %s returned %s for %s", ns, dns.RcodeToString[r.Rcode], fqdn) } @@ -102,7 +103,7 @@ func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, erro } if !found { - return false, fmt.Errorf("nameserver %q did not return the expected TXT record for domain %q", ns, fqdn) + return false, nil } } diff --git a/pkg/issuer/acme/prepare.go b/pkg/issuer/acme/prepare.go index a5c117f0e..72ffaa44b 100644 --- a/pkg/issuer/acme/prepare.go +++ b/pkg/issuer/acme/prepare.go @@ -170,11 +170,9 @@ func (a *Acme) presentOrder(ctx context.Context, cl client.Interface, crt *v1alp // presentChallenge will process a challenge by talking to the acme server and // obtaining up to date status information. -// If the challenge is still in a pending state, it will 'present' the -// challenge using the appropriate solver. -// It will then run the 'Check' function in order to determine whether the -// challenge has propegated (e.g. to DNS resolvers, or to the ingress -// controller). +// If the challenge is still in a pending state, it will first check propagation +// status of a challenge from previous attempt, and if missing it will 'present' the +// new challenge using the appropriate solver. // If the check fails, an error will be returned. // Otherwise, it will return nil. func (a *Acme) presentChallenge(ctx context.Context, cl client.Interface, crt *v1alpha1.Certificate, ch v1alpha1.ACMEOrderChallenge) error { @@ -202,20 +200,26 @@ func (a *Acme) presentChallenge(ctx context.Context, cl client.Interface, crt *v return err } + ok, err := solver.Check(ch) + if err != nil { + return err + } + + if ok { + return nil + } + + // TODO: make sure that solver.Present is noop if challenge + // is already present and all we do is waiting for propagation, + // otherwise it is spamming with errors which are not really erros + // as we are just waiting for propagation err = solver.Present(ctx, crt, ch) if err != nil { return err } - ok, err := solver.Check(ch) - if err != nil { - return err - } - if !ok { - return fmt.Errorf("%s self check failed for domain %q", ch.Type, ch.Domain) - } - - return nil + // Error is only way to indicate that we are ready to ACME Accept challenge + return fmt.Errorf("% challenge was created for domain %q, waiting for propagation", ch.Type, ch.Domain) } func (a *Acme) cleanupLastOrder(ctx context.Context, crt *v1alpha1.Certificate) error {