From 3b270623fde069429fe8bd7d2cb5a6272a0780ff Mon Sep 17 00:00:00 2001 From: "rico.pahlisch" Date: Mon, 27 Aug 2018 08:53:00 +0200 Subject: [PATCH 1/3] enable clouddns meta auth Signed-off-by: Rico Pahlisch --- pkg/issuer/acme/dns/clouddns/clouddns.go | 21 ++++++++++++- pkg/issuer/acme/dns/clouddns/clouddns_test.go | 4 +-- pkg/issuer/acme/dns/dns.go | 30 ++++++++++--------- pkg/issuer/acme/dns/util_test.go | 4 +-- 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/pkg/issuer/acme/dns/clouddns/clouddns.go b/pkg/issuer/acme/dns/clouddns/clouddns.go index 6d0a2927e..f94312fb1 100644 --- a/pkg/issuer/acme/dns/clouddns/clouddns.go +++ b/pkg/issuer/acme/dns/clouddns/clouddns.go @@ -31,11 +31,30 @@ type DNSProvider struct { client *dns.Service } +func NewDNSProvider(project string, saFile string, saBytes []byte, dns01Nameservers []string, ambient bool) (*DNSProvider, error) { + if project == "" { + return nil, fmt.Errorf("Google Cloud project name missing") + } + if saFile == "" && len(saBytes) == 0 { + if !ambient { + return nil, fmt.Errorf("unable to construct clouddns provider: empty credentials; perhaps you meant to enable ambient credentials?") + } + return NewDNSProviderCredentials(project, dns01Nameservers) + } + if saFile != "" { + return NewDNSProviderServiceAccount(project, saFile, dns01Nameservers) + } + if len(saBytes) != 0 { + return NewDNSProviderServiceAccountBytes(project, saBytes, dns01Nameservers) + } + return nil, fmt.Errorf("Google Cloud project name missing") +} + // NewDNSProvider returns a DNSProvider instance configured for Google Cloud // DNS. Project name must be passed in the environment variable: GCE_PROJECT. // A Service Account file can be passed in the environment variable: // GCE_SERVICE_ACCOUNT_FILE -func NewDNSProvider(dns01Nameservers []string) (*DNSProvider, error) { +func NewDNSProviderEnvironment(dns01Nameservers []string) (*DNSProvider, error) { project := os.Getenv("GCE_PROJECT") if saFile, ok := os.LookupEnv("GCE_SERVICE_ACCOUNT_FILE"); ok { return NewDNSProviderServiceAccount(project, saFile, dns01Nameservers) diff --git a/pkg/issuer/acme/dns/clouddns/clouddns_test.go b/pkg/issuer/acme/dns/clouddns/clouddns_test.go index a936ae3f1..61a0715cb 100644 --- a/pkg/issuer/acme/dns/clouddns/clouddns_test.go +++ b/pkg/issuer/acme/dns/clouddns/clouddns_test.go @@ -55,14 +55,14 @@ func TestNewDNSProviderValidEnv(t *testing.T) { t.Skip("skipping live test (requires credentials)") } os.Setenv("GCE_PROJECT", "my-project") - _, err := NewDNSProvider(util.RecursiveNameservers) + _, err := NewDNSProviderEnvironment(util.RecursiveNameservers) assert.NoError(t, err) restoreGCloudEnv() } func TestNewDNSProviderMissingCredErr(t *testing.T) { os.Setenv("GCE_PROJECT", "") - _, err := NewDNSProvider(util.RecursiveNameservers) + _, err := NewDNSProviderEnvironment(util.RecursiveNameservers) assert.EqualError(t, err, "Google Cloud project name missing") restoreGCloudEnv() } diff --git a/pkg/issuer/acme/dns/dns.go b/pkg/issuer/acme/dns/dns.go index 59520f7b7..727086823 100644 --- a/pkg/issuer/acme/dns/dns.go +++ b/pkg/issuer/acme/dns/dns.go @@ -51,7 +51,7 @@ type solver interface { // 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, dns01Nameservers []string) (*clouddns.DNSProvider, error) + cloudDNS func(project string, serviceAccountFile string, serviceAccount []byte, dns01Nameservers []string, ambient bool) (*clouddns.DNSProvider, error) cloudFlare func(email, apikey string, dns01Nameservers []string) (*cloudflare.DNSProvider, error) route53 func(accessKey, secretKey, hostedZoneID, region string, ambient bool, dns01Nameservers []string) (*route53.DNSProvider, error) azureDNS func(clientID, clientSecret, subscriptionID, tenentID, resourceGroupName, hostedZoneName string, dns01Nameservers []string) (*azuredns.DNSProvider, error) @@ -171,19 +171,21 @@ func (s *Solver) solverForIssuerProvider(issuer v1alpha1.GenericIssuer, provider return nil, errors.Wrap(err, "error instantiating akamai challenge solver") } case providerConfig.CloudDNS != nil: - saSecret, err := s.secretLister.Secrets(resourceNamespace).Get(providerConfig.CloudDNS.ServiceAccount.Name) - if err != nil { - return nil, fmt.Errorf("error getting clouddns service account: %s", err) + + var key []byte + if providerConfig.CloudDNS.ServiceAccount.Name != "" { + saSecret, err := s.secretLister.Secrets(resourceNamespace).Get(providerConfig.CloudDNS.ServiceAccount.Name) + if err != nil { + return nil, fmt.Errorf("error getting clouddns service account: %s", err) + } + saKey := providerConfig.CloudDNS.ServiceAccount.Key + saBytes := saSecret.Data[saKey] + + if len(saBytes) == 0 { + return nil, fmt.Errorf("specfied key %q not found in secret %s/%s", saKey, saSecret.Namespace, saSecret.Name) + } } - - saKey := providerConfig.CloudDNS.ServiceAccount.Key - saBytes := saSecret.Data[saKey] - - if len(saBytes) == 0 { - return nil, fmt.Errorf("specfied key %q not found in secret %s/%s", saKey, saSecret.Namespace, saSecret.Name) - } - - impl, err = s.dnsProviderConstructors.cloudDNS(providerConfig.CloudDNS.Project, saBytes, s.DNS01Nameservers) + impl, err = s.dnsProviderConstructors.cloudDNS(providerConfig.CloudDNS.Project, "", key, s.DNS01Nameservers, s.CanUseAmbientCredentials(issuer)) if err != nil { return nil, fmt.Errorf("error instantiating google clouddns challenge solver: %s", err) } @@ -274,7 +276,7 @@ func NewSolver(ctx *controller.Context) *Solver { ctx, ctx.KubeSharedInformerFactory.Core().V1().Secrets().Lister(), dnsProviderConstructors{ - clouddns.NewDNSProviderServiceAccountBytes, + clouddns.NewDNSProvider, cloudflare.NewDNSProviderCredentials, route53.NewDNSProvider, azuredns.NewDNSProviderCredentials, diff --git a/pkg/issuer/acme/dns/util_test.go b/pkg/issuer/acme/dns/util_test.go index bb24980df..2027cfb0c 100644 --- a/pkg/issuer/acme/dns/util_test.go +++ b/pkg/issuer/acme/dns/util_test.go @@ -136,8 +136,8 @@ func newFakeDNSProviders() *fakeDNSProviders { calls: []fakeDNSProviderCall{}, } f.constructors = dnsProviderConstructors{ - cloudDNS: func(project string, serviceAccount []byte, dns01Nameservers []string) (*clouddns.DNSProvider, error) { - f.call("clouddns", project, serviceAccount, util.RecursiveNameservers) + cloudDNS: func(project string, serviceAccountFile string, serviceAccount []byte, dns01Nameservers []string, ambient bool) (*clouddns.DNSProvider, error) { + f.call("clouddns", project, serviceAccountFile, serviceAccount, util.RecursiveNameservers, ambient) return nil, nil }, cloudFlare: func(email, apikey string, dns01Nameservers []string) (*cloudflare.DNSProvider, error) { From 8c5c402d1e81e4047968975a398dc7af0ee0fb69 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Mon, 10 Sep 2018 13:20:25 +0100 Subject: [PATCH 2/3] Fix up bug preventing saBytes being used. Add comments. Signed-off-by: James Munnelly --- pkg/issuer/acme/dns/clouddns/clouddns.go | 15 ++++++++------- pkg/issuer/acme/dns/dns.go | 18 ++++++++++++------ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/pkg/issuer/acme/dns/clouddns/clouddns.go b/pkg/issuer/acme/dns/clouddns/clouddns.go index f94312fb1..597ec8e1b 100644 --- a/pkg/issuer/acme/dns/clouddns/clouddns.go +++ b/pkg/issuer/acme/dns/clouddns/clouddns.go @@ -31,26 +31,27 @@ type DNSProvider struct { client *dns.Service } -func NewDNSProvider(project string, saFile string, saBytes []byte, dns01Nameservers []string, ambient bool) (*DNSProvider, error) { +func NewDNSProvider(project string, saBytes []byte, dns01Nameservers []string, ambient bool) (*DNSProvider, error) { + // project is a required field if project == "" { return nil, fmt.Errorf("Google Cloud project name missing") } - if saFile == "" && len(saBytes) == 0 { + // if the service account bytes are not provided, we will attempt to instantiate + // with 'ambient credentials' (if they are allowed/enabled) + if len(saBytes) == 0 { if !ambient { return nil, fmt.Errorf("unable to construct clouddns provider: empty credentials; perhaps you meant to enable ambient credentials?") } return NewDNSProviderCredentials(project, dns01Nameservers) } - if saFile != "" { - return NewDNSProviderServiceAccount(project, saFile, dns01Nameservers) - } + // if service account data is provided, we instantiate using that if len(saBytes) != 0 { return NewDNSProviderServiceAccountBytes(project, saBytes, dns01Nameservers) } - return nil, fmt.Errorf("Google Cloud project name missing") + return nil, fmt.Errorf("missing Google Cloud DNS provider credentials") } -// NewDNSProvider returns a DNSProvider instance configured for Google Cloud +// NewDNSProviderEnvironment returns a DNSProvider instance configured for Google Cloud // DNS. Project name must be passed in the environment variable: GCE_PROJECT. // A Service Account file can be passed in the environment variable: // GCE_SERVICE_ACCOUNT_FILE diff --git a/pkg/issuer/acme/dns/dns.go b/pkg/issuer/acme/dns/dns.go index 727086823..0e8edbfda 100644 --- a/pkg/issuer/acme/dns/dns.go +++ b/pkg/issuer/acme/dns/dns.go @@ -51,7 +51,7 @@ type solver interface { // 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, serviceAccountFile string, serviceAccount []byte, dns01Nameservers []string, ambient bool) (*clouddns.DNSProvider, error) + cloudDNS func(project string, serviceAccount []byte, dns01Nameservers []string, ambient bool) (*clouddns.DNSProvider, error) cloudFlare func(email, apikey string, dns01Nameservers []string) (*cloudflare.DNSProvider, error) route53 func(accessKey, secretKey, hostedZoneID, region string, ambient bool, dns01Nameservers []string) (*route53.DNSProvider, error) azureDNS func(clientID, clientSecret, subscriptionID, tenentID, resourceGroupName, hostedZoneName string, dns01Nameservers []string) (*azuredns.DNSProvider, error) @@ -171,21 +171,27 @@ func (s *Solver) solverForIssuerProvider(issuer v1alpha1.GenericIssuer, provider return nil, errors.Wrap(err, "error instantiating akamai challenge solver") } case providerConfig.CloudDNS != nil: + var keyData []byte - var key []byte + // if the serviceAccount.name field is set, we will load credentials from + // that secret. + // If it is not set, we will attempt to instantiate the provider using + // ambient credentials (if enabled). if providerConfig.CloudDNS.ServiceAccount.Name != "" { saSecret, err := s.secretLister.Secrets(resourceNamespace).Get(providerConfig.CloudDNS.ServiceAccount.Name) if err != nil { return nil, fmt.Errorf("error getting clouddns service account: %s", err) } - saKey := providerConfig.CloudDNS.ServiceAccount.Key - saBytes := saSecret.Data[saKey] - if len(saBytes) == 0 { + saKey := providerConfig.CloudDNS.ServiceAccount.Key + keyData = saSecret.Data[saKey] + if len(keyData) == 0 { return nil, fmt.Errorf("specfied key %q not found in secret %s/%s", saKey, saSecret.Namespace, saSecret.Name) } } - impl, err = s.dnsProviderConstructors.cloudDNS(providerConfig.CloudDNS.Project, "", key, s.DNS01Nameservers, s.CanUseAmbientCredentials(issuer)) + + // attempt to construct the cloud dns provider + impl, err = s.dnsProviderConstructors.cloudDNS(providerConfig.CloudDNS.Project, keyData, s.DNS01Nameservers, s.CanUseAmbientCredentials(issuer)) if err != nil { return nil, fmt.Errorf("error instantiating google clouddns challenge solver: %s", err) } From ac08365928bd47a3e7350f3851ac1c852e825062 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Mon, 10 Sep 2018 13:25:33 +0100 Subject: [PATCH 3/3] Fix up test failure Signed-off-by: James Munnelly --- pkg/issuer/acme/dns/util_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/issuer/acme/dns/util_test.go b/pkg/issuer/acme/dns/util_test.go index 2027cfb0c..cbca248f1 100644 --- a/pkg/issuer/acme/dns/util_test.go +++ b/pkg/issuer/acme/dns/util_test.go @@ -136,8 +136,8 @@ func newFakeDNSProviders() *fakeDNSProviders { calls: []fakeDNSProviderCall{}, } f.constructors = dnsProviderConstructors{ - cloudDNS: func(project string, serviceAccountFile string, serviceAccount []byte, dns01Nameservers []string, ambient bool) (*clouddns.DNSProvider, error) { - f.call("clouddns", project, serviceAccountFile, serviceAccount, util.RecursiveNameservers, ambient) + cloudDNS: func(project string, serviceAccount []byte, dns01Nameservers []string, ambient bool) (*clouddns.DNSProvider, error) { + f.call("clouddns", project, serviceAccount, util.RecursiveNameservers, ambient) return nil, nil }, cloudFlare: func(email, apikey string, dns01Nameservers []string) (*cloudflare.DNSProvider, error) {