diff --git a/pkg/issuer/acme/dns/dns.go b/pkg/issuer/acme/dns/dns.go index a9ebd5914..6234c3102 100644 --- a/pkg/issuer/acme/dns/dns.go +++ b/pkg/issuer/acme/dns/dns.go @@ -70,40 +70,35 @@ type Solver struct { } // Present performs the work to configure DNS to resolve a DNS01 challenge. -func (s *Solver) Present(ctx context.Context, issuer v1alpha1.GenericIssuer, _ *v1alpha1.Certificate, ch v1alpha1.ACMEOrderChallenge) error { - if ch.SolverConfig.DNS01 == nil { +func (s *Solver) Present(ctx context.Context, issuer v1alpha1.GenericIssuer, ch *v1alpha1.Challenge) error { + if ch.Spec.Config.DNS01 == nil { return fmt.Errorf("challenge dns config must be specified") } - providerName := ch.SolverConfig.DNS01.Provider - if providerName == "" { - return fmt.Errorf("dns01 challenge provider name must be set") - } - - slv, err := s.solverForIssuerProvider(issuer, providerName) + slv, err := s.solverForChallenge(issuer, ch) if err != nil { return err } - glog.Infof("Presenting DNS01 challenge for domain %q", ch.Domain) - return slv.Present(ch.Domain, ch.Token, ch.Key) + glog.Infof("Presenting DNS01 challenge for domain %q", ch.Spec.DNSName) + return slv.Present(ch.Spec.DNSName, ch.Spec.Token, ch.Spec.Key) } // Check verifies that the DNS records for the ACME challenge have propagated. -func (s *Solver) Check(ch v1alpha1.ACMEOrderChallenge) (bool, error) { - fqdn, value, ttl, err := util.DNS01Record(ch.Domain, ch.Key, s.DNS01Nameservers) +func (s *Solver) Check(ch *v1alpha1.Challenge) (bool, error) { + fqdn, value, ttl, err := util.DNS01Record(ch.Spec.DNSName, ch.Spec.Key, s.DNS01Nameservers) if err != nil { return false, err } - glog.Infof("Checking DNS propagation for %q using name servers: %v", ch.Domain, s.DNS01Nameservers) + glog.Infof("Checking DNS propagation for %q using name servers: %v", ch.Spec.DNSName, s.Context.DNS01Nameservers) - ok, err := util.PreCheckDNS(fqdn, value, s.DNS01Nameservers) + ok, err := util.PreCheckDNS(fqdn, value, s.Context.DNS01Nameservers) if err != nil { return false, err } if !ok { - glog.Infof("DNS record for %q not yet propagated", ch.Domain) + glog.Infof("DNS record for %q not yet propagated", ch.Spec.DNSName) return false, nil } @@ -116,34 +111,31 @@ func (s *Solver) Check(ch v1alpha1.ACMEOrderChallenge) (bool, error) { // CleanUp removes DNS records which are no longer needed after // certificate issuance. -func (s *Solver) CleanUp(ctx context.Context, issuer v1alpha1.GenericIssuer, _ *v1alpha1.Certificate, ch v1alpha1.ACMEOrderChallenge) error { - if ch.SolverConfig.DNS01 == nil { +func (s *Solver) CleanUp(ctx context.Context, issuer v1alpha1.GenericIssuer, ch *v1alpha1.Challenge) error { + if ch.Spec.Config.DNS01 == nil { return fmt.Errorf("challenge dns config must be specified") } - providerName := ch.SolverConfig.DNS01.Provider - if providerName == "" { - return fmt.Errorf("dns01 challenge provider name must be set") - } - - slv, err := s.solverForIssuerProvider(issuer, providerName) + slv, err := s.solverForChallenge(issuer, ch) if err != nil { return err } - return slv.CleanUp(ch.Domain, ch.Token, ch.Key) + return slv.CleanUp(ch.Spec.DNSName, ch.Spec.Token, ch.Spec.Key) } -// solverForIssuerProvider returns a Solver for the given providerName. +// solverForChallenge returns a Solver for the given providerName. // The providerName is the name of an ACME DNS-01 challenge provider as // specified on the Issuer resource for the Solver. -// -// This method is exported so that only the provider name is required in order -// to obtain an instance of a Solver. This is useful when cleaning up old -// challenges after the ACME challenge configuration on the Certificate has -// been removed by the user. -func (s *Solver) solverForIssuerProvider(issuer v1alpha1.GenericIssuer, providerName string) (solver, error) { +func (s *Solver) solverForChallenge(issuer v1alpha1.GenericIssuer, ch *v1alpha1.Challenge) (solver, error) { resourceNamespace := s.ResourceNamespace(issuer) + canUseAmbientCredentials := s.CanUseAmbientCredentials(issuer) + + providerName := ch.Spec.Config.DNS01.Provider + if providerName == "" { + return nil, fmt.Errorf("dns01 challenge provider name must be set") + } + providerConfig, err := issuer.GetSpec().ACME.DNS01.Provider(providerName) if err != nil { return nil, err @@ -234,7 +226,7 @@ func (s *Solver) solverForIssuerProvider(issuer v1alpha1.GenericIssuer, provider strings.TrimSpace(secretAccessKey), providerConfig.Route53.HostedZoneID, providerConfig.Route53.Region, - s.CanUseAmbientCredentials(issuer), + canUseAmbientCredentials, s.DNS01Nameservers, ) if err != nil { diff --git a/pkg/issuer/acme/dns/dns_test.go b/pkg/issuer/acme/dns/dns_test.go index ebabc20b6..f62e720da 100644 --- a/pkg/issuer/acme/dns/dns_test.go +++ b/pkg/issuer/acme/dns/dns_test.go @@ -50,18 +50,6 @@ func newIssuer(name, namespace string, configs []v1alpha1.ACMEIssuerDNS01Provide } } -func newCertificate(name, namespace, cn string, dnsNames []string, configs []v1alpha1.DomainSolverConfig) *v1alpha1.Certificate { - return &v1alpha1.Certificate{ - Spec: v1alpha1.CertificateSpec{ - CommonName: cn, - DNSNames: dnsNames, - ACME: &v1alpha1.ACMECertificateConfig{ - Config: configs, - }, - }, - } -} - func newSecret(name, namespace string, data map[string][]byte) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -102,10 +90,12 @@ func TestSolverFor(t *testing.T) { }, }, }), - Challenge: v1alpha1.ACMEOrderChallenge{ - SolverConfig: v1alpha1.SolverConfig{ - DNS01: &v1alpha1.DNS01SolverConfig{ - Provider: "fake-cloudflare", + Challenge: &v1alpha1.Challenge{ + Spec: v1alpha1.ChallengeSpec{ + Config: v1alpha1.SolverConfig{ + DNS01: &v1alpha1.DNS01SolverConfig{ + Provider: "fake-cloudflare", + }, }, }, }, @@ -130,10 +120,12 @@ func TestSolverFor(t *testing.T) { }, }), // don't include any secrets in the lister - Challenge: v1alpha1.ACMEOrderChallenge{ - SolverConfig: v1alpha1.SolverConfig{ - DNS01: &v1alpha1.DNS01SolverConfig{ - Provider: "fake-cloudflare", + Challenge: &v1alpha1.Challenge{ + Spec: v1alpha1.ChallengeSpec{ + Config: v1alpha1.SolverConfig{ + DNS01: &v1alpha1.DNS01SolverConfig{ + Provider: "fake-cloudflare", + }, }, }, }, @@ -164,10 +156,12 @@ func TestSolverFor(t *testing.T) { }, }, }), - Challenge: v1alpha1.ACMEOrderChallenge{ - SolverConfig: v1alpha1.SolverConfig{ - DNS01: &v1alpha1.DNS01SolverConfig{ - Provider: "fake-cloudflare", + Challenge: &v1alpha1.Challenge{ + Spec: v1alpha1.ChallengeSpec{ + Config: v1alpha1.SolverConfig{ + DNS01: &v1alpha1.DNS01SolverConfig{ + Provider: "fake-cloudflare", + }, }, }, }, @@ -198,10 +192,12 @@ func TestSolverFor(t *testing.T) { }, }, }), - Challenge: v1alpha1.ACMEOrderChallenge{ - SolverConfig: v1alpha1.SolverConfig{ - DNS01: &v1alpha1.DNS01SolverConfig{ - Provider: "fake-cloudflare-oops", + Challenge: &v1alpha1.Challenge{ + Spec: v1alpha1.ChallengeSpec{ + Config: v1alpha1.SolverConfig{ + DNS01: &v1alpha1.DNS01SolverConfig{ + Provider: "fake-cloudflare-oops", + }, }, }, }, @@ -249,7 +245,7 @@ func TestSolverFor(t *testing.T) { test.Setup(t) defer test.Finish(t) s := test.Solver - dnsSolver, err := s.solverForIssuerProvider(test.Issuer, test.Challenge.SolverConfig.DNS01.Provider) + dnsSolver, err := s.solverForChallenge(test.Issuer, test.Challenge) if err != nil && !test.expectErr { t.Errorf("expected solverFor to not error, but got: %s", err.Error()) return @@ -290,10 +286,12 @@ func TestRoute53TrimCreds(t *testing.T) { }, }, }), - Challenge: v1alpha1.ACMEOrderChallenge{ - SolverConfig: v1alpha1.SolverConfig{ - DNS01: &v1alpha1.DNS01SolverConfig{ - Provider: "fake-route53", + Challenge: &v1alpha1.Challenge{ + Spec: v1alpha1.ChallengeSpec{ + Config: v1alpha1.SolverConfig{ + DNS01: &v1alpha1.DNS01SolverConfig{ + Provider: "fake-route53", + }, }, }, }, @@ -304,7 +302,7 @@ func TestRoute53TrimCreds(t *testing.T) { defer f.Finish(t) s := f.Solver - _, err := s.solverForIssuerProvider(f.Issuer, f.Challenge.SolverConfig.DNS01.Provider) + _, err := s.solverForChallenge(f.Issuer, f.Challenge) if err != nil { t.Fatalf("expected solverFor to not error, but got: %s", err) } @@ -349,10 +347,12 @@ func TestRoute53AmbientCreds(t *testing.T) { }, }), dnsProviders: newFakeDNSProviders(), - Challenge: v1alpha1.ACMEOrderChallenge{ - SolverConfig: v1alpha1.SolverConfig{ - DNS01: &v1alpha1.DNS01SolverConfig{ - Provider: "fake-route53", + Challenge: &v1alpha1.Challenge{ + Spec: v1alpha1.ChallengeSpec{ + Config: v1alpha1.SolverConfig{ + DNS01: &v1alpha1.DNS01SolverConfig{ + Provider: "fake-route53", + }, }, }, }, @@ -382,10 +382,12 @@ func TestRoute53AmbientCreds(t *testing.T) { }, }), dnsProviders: newFakeDNSProviders(), - Challenge: v1alpha1.ACMEOrderChallenge{ - SolverConfig: v1alpha1.SolverConfig{ - DNS01: &v1alpha1.DNS01SolverConfig{ - Provider: "fake-route53", + Challenge: &v1alpha1.Challenge{ + Spec: v1alpha1.ChallengeSpec{ + Config: v1alpha1.SolverConfig{ + DNS01: &v1alpha1.DNS01SolverConfig{ + Provider: "fake-route53", + }, }, }, }, @@ -404,7 +406,7 @@ func TestRoute53AmbientCreds(t *testing.T) { f.Setup(t) defer f.Finish(t) s := f.Solver - _, err := s.solverForIssuerProvider(f.Issuer, f.Challenge.SolverConfig.DNS01.Provider) + _, err := s.solverForChallenge(f.Issuer, f.Challenge) if !reflect.DeepEqual(tt.out.expectedErr, err) { t.Fatalf("expected error %v, got error %v", tt.out.expectedErr, err) } diff --git a/pkg/issuer/acme/dns/util_test.go b/pkg/issuer/acme/dns/util_test.go index b9786f498..841dbd680 100644 --- a/pkg/issuer/acme/dns/util_test.go +++ b/pkg/issuer/acme/dns/util_test.go @@ -47,10 +47,8 @@ type solverFixture struct { // Issuer to be passed to functions on the Solver (a default will be used if nil) Issuer v1alpha1.GenericIssuer - // Certificate resource to use during tests - Certificate *v1alpha1.Certificate // Challenge resource to use during tests - Challenge v1alpha1.ACMEOrderChallenge + Challenge *v1alpha1.Challenge dnsProviders *fakeDNSProviders