issuer/acme/dns: refactor provider construction

Previously, each provider's package-level 'New' function was being
called.

That made mocking it out for a different function that records data or
returns different output quite difficult.

This PR introduces an additional layer of abstraction in the form of
effectively a vtable struct for the dns providers. It's defaulted to the
same package-level constructors as before, but unit tests in the dns
package can easily override it.

A new test for the previously-introduced route53 trimming behavior is
also added.
This commit is contained in:
Euan Kemp 2018-03-14 01:04:22 -07:00
parent 8aefbb1470
commit 78b1b8d69d
2 changed files with 141 additions and 13 deletions

View File

@ -28,11 +28,25 @@ type solver interface {
Timeout() (timeout, interval time.Duration)
}
// dnsProviderConstructors defines how each provider may be constructed.
// It is useful for mocking out a given provider since an alternate set of
// constructors may be set.
type dnsProviderConstructors struct {
cloudDNS func(project string, serviceAccount []byte) (*clouddns.DNSProvider, error)
cloudFlare func(email, apikey string) (*cloudflare.DNSProvider, error)
route53 func(accessKey, secretKey, hostedZoneID, region string) (*route53.DNSProvider, error)
azureDNS func(clientID, clientSecret, subscriptionID, tenentID, resourceGroupName, hostedZoneName string) (*azuredns.DNSProvider, error)
}
// Solver is a solver for the acme dns01 challenge.
// Given a Certificate object, it determines the correct DNS provider based on
// the certificate, and configures it based on the referenced issuer.
type Solver struct {
issuer v1alpha1.GenericIssuer
client kubernetes.Interface
secretLister corev1listers.SecretLister
resourceNamespace string
issuer v1alpha1.GenericIssuer
client kubernetes.Interface
secretLister corev1listers.SecretLister
dnsProviderConstructors dnsProviderConstructors
resourceNamespace string
}
func (s *Solver) Present(ctx context.Context, crt *v1alpha1.Certificate, domain, token, key string) error {
@ -124,7 +138,7 @@ func (s *Solver) solverFor(crt *v1alpha1.Certificate, domain string) (solver, er
}
saBytes := saSecret.Data[providerConfig.CloudDNS.ServiceAccount.Key]
impl, err = clouddns.NewDNSProviderServiceAccountBytes(providerConfig.CloudDNS.Project, saBytes)
impl, err = s.dnsProviderConstructors.cloudDNS(providerConfig.CloudDNS.Project, saBytes)
if err != nil {
return nil, fmt.Errorf("error instantiating google clouddns challenge solver: %s", err.Error())
}
@ -137,7 +151,7 @@ func (s *Solver) solverFor(crt *v1alpha1.Certificate, domain string) (solver, er
email := providerConfig.Cloudflare.Email
apiKey := string(apiKeySecret.Data[providerConfig.Cloudflare.APIKey.Key])
impl, err = cloudflare.NewDNSProviderCredentials(email, apiKey)
impl, err = s.dnsProviderConstructors.cloudFlare(email, apiKey)
if err != nil {
return nil, fmt.Errorf("error instantiating cloudflare challenge solver: %s", err.Error())
}
@ -152,7 +166,7 @@ func (s *Solver) solverFor(crt *v1alpha1.Certificate, domain string) (solver, er
return nil, fmt.Errorf("error getting route53 secret access key: key '%s' not found in secret", providerConfig.Route53.SecretAccessKey.Key)
}
impl, err = route53.NewDNSProviderAccessKey(
impl, err = s.dnsProviderConstructors.route53(
strings.TrimSpace(providerConfig.Route53.AccessKeyID),
strings.TrimSpace(string(secretAccessKeyBytes)),
providerConfig.Route53.HostedZoneID,
@ -172,7 +186,7 @@ func (s *Solver) solverFor(crt *v1alpha1.Certificate, domain string) (solver, er
return nil, fmt.Errorf("error getting azure dns client secret: key '%s' not found in secret", providerConfig.AzureDNS.ClientSecret.Key)
}
impl, err = azuredns.NewDNSProviderCredentials(
impl, err = s.dnsProviderConstructors.azureDNS(
providerConfig.AzureDNS.ClientID,
string(clientSecretBytes),
providerConfig.AzureDNS.SubscriptionID,
@ -188,5 +202,16 @@ func (s *Solver) solverFor(crt *v1alpha1.Certificate, domain string) (solver, er
}
func NewSolver(issuer v1alpha1.GenericIssuer, client kubernetes.Interface, secretLister corev1listers.SecretLister, resourceNamespace string) *Solver {
return &Solver{issuer, client, secretLister, resourceNamespace}
return &Solver{
issuer,
client,
secretLister,
dnsProviderConstructors{
clouddns.NewDNSProviderServiceAccountBytes,
cloudflare.NewDNSProviderCredentials,
route53.NewDNSProviderAccessKey,
azuredns.NewDNSProviderCredentials,
},
resourceNamespace,
}
}

View File

@ -1,6 +1,7 @@
package dns
import (
"errors"
"reflect"
"testing"
@ -11,7 +12,10 @@ import (
kubefake "k8s.io/client-go/kubernetes/fake"
"github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1"
"github.com/jetstack/cert-manager/pkg/issuer/acme/dns/azuredns"
"github.com/jetstack/cert-manager/pkg/issuer/acme/dns/clouddns"
"github.com/jetstack/cert-manager/pkg/issuer/acme/dns/cloudflare"
"github.com/jetstack/cert-manager/pkg/issuer/acme/dns/route53"
)
type fixture struct {
@ -29,6 +33,50 @@ type fixture struct {
// certificate used in the test
Certificate *v1alpha1.Certificate
DNSProviders *fakeDNSProviders
}
type fakeDNSProviderCall struct {
name string
args []interface{}
}
type fakeDNSProviders struct {
constructors dnsProviderConstructors
calls []fakeDNSProviderCall
}
func (f *fakeDNSProviders) call(name string, args ...interface{}) {
f.calls = append(f.calls, fakeDNSProviderCall{name: name, args: args})
}
func newFakeDNSProviders() *fakeDNSProviders {
f := &fakeDNSProviders{
calls: []fakeDNSProviderCall{},
}
f.constructors = dnsProviderConstructors{
cloudDNS: func(project string, serviceAccount []byte) (*clouddns.DNSProvider, error) {
f.call("clouddns", project, serviceAccount)
return nil, nil
},
cloudFlare: func(email, apikey string) (*cloudflare.DNSProvider, error) {
f.call("cloudflare", email, apikey)
if email == "" || apikey == "" {
return nil, errors.New("invalid email or apikey")
}
return nil, nil
},
route53: func(accessKey, secretKey, hostedZoneID, region string) (*route53.DNSProvider, error) {
f.call("route53", accessKey, secretKey, hostedZoneID, region)
return nil, nil
},
azureDNS: func(clientID, clientSecret, subscriptionID, tenentID, resourceGroupName, hostedZoneName string) (*azuredns.DNSProvider, error) {
f.call("azuredns", clientID, clientSecret, subscriptionID, tenentID, resourceGroupName, hostedZoneName)
return nil, nil
},
}
return f
}
func (f *fixture) solver() *Solver {
@ -41,11 +89,16 @@ func (f *fixture) solver() *Solver {
stopCh := make(chan struct{})
defer close(stopCh)
sharedInformerFactory.Start(stopCh)
dnsProvider := f.DNSProviders
if dnsProvider == nil {
dnsProvider = newFakeDNSProviders()
}
return &Solver{
issuer: f.Issuer,
client: kubeClient,
secretLister: secretsLister,
resourceNamespace: f.ResourceNamespace,
issuer: f.Issuer,
client: kubeClient,
secretLister: secretsLister,
dnsProviderConstructors: dnsProvider.constructors,
resourceNamespace: f.ResourceNamespace,
}
}
@ -276,3 +329,53 @@ func TestSolverFor(t *testing.T) {
t.Run(name, testFn(test))
}
}
func TestRoute53TrimCreds(t *testing.T) {
f := &fixture{
Issuer: newIssuer("test", "default", []v1alpha1.ACMEIssuerDNS01Provider{
{
Name: "fake-route53",
Route53: &v1alpha1.ACMEIssuerDNS01ProviderRoute53{
AccessKeyID: " test_with_spaces ",
Region: "us-west-2",
SecretAccessKey: v1alpha1.SecretKeySelector{
LocalObjectReference: v1alpha1.LocalObjectReference{
Name: "route53",
},
Key: "secret",
},
},
},
}),
SecretLister: []*corev1.Secret{newSecret("route53", "default", map[string][]byte{
"secret": []byte("AKIENDINNEWLINE \n"),
})},
ResourceNamespace: "default",
Certificate: newCertificate("test", "default", "example.com", nil, []v1alpha1.ACMECertificateDomainConfig{
{
Domains: []string{"example.com"},
DNS01: &v1alpha1.ACMECertificateDNS01Config{
Provider: "fake-route53",
},
},
}),
DNSProviders: newFakeDNSProviders(),
}
s := f.solver()
_, err := s.solverFor(f.Certificate, "example.com")
if err != nil {
t.Fatalf("expected solverFor to not error, but got: %s", err)
}
expectedR53Call := []fakeDNSProviderCall{
{
name: "route53",
args: []interface{}{"test_with_spaces", "AKIENDINNEWLINE", "", "us-west-2"},
},
}
if !reflect.DeepEqual(expectedR53Call, f.DNSProviders.calls) {
t.Fatalf("expected %+v == %+v", expectedR53Call, f.DNSProviders.calls)
}
}