Add the ability to use a DNSimple User API token (#26)

* Allow for the DNSimple account ID to be specified via the dnssimple.accountID value

* Allow for the DNSimple account ID to be manually specified via the DNSIMPLE_ACCOUNT_ID environment variable

Fix incorrect variable specification in main.go

* Allow for the DNSimple account ID to be manually specified via the DNSIMPLE_ACCOUNT_ID environment variable

Fix missing return of nil in main.go

* Fix incorrect double-declaration of env section in deployment.yaml

* Fix incorrect logic when handling DNSIMPLE_ACCOUNT_ID environment variable

* The client.Zones.GetZone check isn't needed in the getExistingRecord function and it prevents User API tokens from being used for authentication - only Account API tokens would work

* Incorporate changes from https://github.com/puzzle/cert-manager-webhook-dnsimple/pull/29 so that the DNSimple accountID may be obtained from the Issuer config if a DNSimple User API token is being used

* Remove unused variable DnsimpleAccountId

* Fix deploy/dnsimple/templates/deployment.yaml - DNSIMPLE_ACCOUNT_ID environment variable doesn't do anything anymore so we can delete it

* Pass the .Values.dnsimple.accountID value down to the staging and production ClusterIssuer configs

* Update README.md with documentation on the dnsimple.accountID parameter

* The ClusterIssuer configs must quote the accountID value so that it is interpreted as a string and not as a number

* Fix indentation level of imagePullSecret in deployment.yaml

README.md

@@ -10,7 +10,7 @@ A [cert-manager][2] ACME DNS01 solver webhook for [DNSimple][1].
 
 ## Quickstart
 
-Take note of your DNSimple API token from the account settings in the automation tab. Run the following commands replacing the API token placeholders and email address:
+Take note of your DNSimple API token from the account settings in the automation tab. Run the following commands replacing the API token / account ID placeholders and email address:
 
 ```bash
 $ helm repo add neoskop https://charts.neoskop.dev
@@ -18,6 +18,7 @@ $ helm install cert-manager-webhook-dnsimple \
     --namespace cert-manager \
     --dry-run \
     --set dnsimple.token='<DNSIMPLE_API_TOKEN>' \
+    --set dnsimple.accountID='<DNSIMPLE_ACCOUNT_ID>' # Only needed if using a User API token \
     --set clusterIssuer.production.enabled=true \
     --set clusterIssuer.staging.enabled=true \
     --set clusterIssuer.email=email@example.com \
@@ -52,6 +53,7 @@ The Helm chart accepts the following values:
 | name                               | required | description                                     | default value                           |
 | ---------------------------------- | -------- | ----------------------------------------------- | --------------------------------------- |
 | `dnsimple.token`                   | ✔️       | DNSimple API Token                              | _empty_                                 |
+| `dnsimple.accountID`               |          | DNSimple Account ID (required for User tokens)  | _empty_                                 |
 | `clusterIssuer.email`              |          | LetsEncrypt Admin Email                         | `name@example.com`                      |
 | `clusterIssuer.production.enabled` |          | Create a production `ClusterIssuer`             | `false`                                 |
 | `clusterIssuer.staging.enabled`    |          | Create a staging `ClusterIssuer`                | `false`                                 |

deploy/dnsimple/templates/deployment.yaml

@@ -24,14 +24,14 @@ spec:
         release: {{ .Release.Name }}
     spec:
       serviceAccountName: {{ include "dnsimple-webhook.fullname" . }}
-      containers:
-        - name: {{ .Chart.Name }}
-          image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
-          imagePullPolicy: {{ .Values.image.pullPolicy }}
       {{- if .Values.image.pullSecret }}
       imagePullSecrets:
         - name: {{ .Values.image.pullSecret }}
       {{- end }}
+      containers:
+        - name: {{ .Chart.Name }}
+          image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
+          imagePullPolicy: {{ .Values.image.pullPolicy }}
           args:
             - --tls-cert-file=/tls/tls.crt
             - --tls-private-key-file=/tls/tls.key

deploy/dnsimple/templates/production.cluster-issuer.yaml

@@ -21,6 +21,7 @@ spec:
             tokenSecretRef:
               key: token
               name: {{ include "dnsimple-webhook.tokenSecretName" . }}
+            accountID: {{ .Values.dnsimple.accountID | quote }}
           groupName: {{ .Values.groupName }}
           solverName: dnsimple
 {{- end -}}

deploy/dnsimple/templates/staging.cluster-issuer.yaml

@@ -21,6 +21,7 @@ spec:
             tokenSecretRef:
               key: token
               name: {{ include "dnsimple-webhook.tokenSecretName" . }}
+            accountID: {{ .Values.dnsimple.accountID | quote }}
           groupName: {{ .Values.groupName }}
           solverName: dnsimple
 {{- end -}}

deploy/dnsimple/values.yaml

@@ -13,6 +13,7 @@ certManager:
 # logLevel: 3
 dnsimple:
   token: ""
+  # accountID:
   # existingTokenSecret: false
   # tokenSecretName:
 clusterIssuer:

main.go

@@ -73,6 +73,7 @@ type dnsimpleDNSProviderConfig struct {
 	// `issuer.spec.acme.dns01.providers.webhook.config` field.
 
 	TokenSecretRef cmmeta.SecretKeySelector `json:"tokenSecretRef"`
+	AccountID      *string                  `json:"accountID"`
 }
 
 // Name is used as the name for this DNS solver when referencing it on the ACME
@@ -85,19 +86,19 @@ func (c *dnsimpleDNSProviderSolver) Name() string {
 	return "dnsimple"
 }
 
-func (c *dnsimpleDNSProviderSolver) getClient(cfg *dnsimpleDNSProviderConfig, namespace string) (*dnsimple.Client, error) {
+func (c *dnsimpleDNSProviderSolver) getClient(cfg *dnsimpleDNSProviderConfig, namespace string) (client *dnsimple.Client, accountID string, err error) {
 	secretName := cfg.TokenSecretRef.LocalObjectReference.Name
 	klog.V(6).Infof("Try to load secret `%s` with key `%s`", secretName, cfg.TokenSecretRef.Key)
 	sec, err := c.client.CoreV1().Secrets(namespace).Get(context.Background(), secretName, metav1.GetOptions{})
 
 	if err != nil {
-		return nil, fmt.Errorf("unable to get secret `%s`; %v", secretName, err)
+		return nil, "", fmt.Errorf("unable to get secret `%s`; %v", secretName, err)
 	}
 
 	secBytes, ok := sec.Data[cfg.TokenSecretRef.Key]
 
 	if !ok {
-		return nil, fmt.Errorf("Key %q not found in secret \"%s/%s\"", cfg.TokenSecretRef.Key, cfg.TokenSecretRef.LocalObjectReference.Name, namespace)
+		return nil, "", fmt.Errorf("Key %q not found in secret \"%s/%s\"", cfg.TokenSecretRef.Key, cfg.TokenSecretRef.LocalObjectReference.Name, namespace)
 	}
 
 	apiKey := string(secBytes)
@@ -105,9 +106,27 @@ func (c *dnsimpleDNSProviderSolver) getClient(cfg *dnsimpleDNSProviderConfig, na
 	tc := dnsimple.StaticTokenHTTPClient(context.Background(), apiKey)
 
 	// new client
-	client := dnsimple.NewClient(tc)
+	client = dnsimple.NewClient(tc)
 	client.SetUserAgent("cert-manager-webhook-dnsimple")
-	return client, nil
+
+	// if account id is configured explicitly we use it
+	if cfg.AccountID != nil {
+		return client, *cfg.AccountID, nil
+	}
+
+	// resolve account id if not configured
+	whoamiResponse, err := client.Identity.Whoami(context.Background())
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to lookup account ID: %w", err)
+	}
+
+	// if whoami is called with a user token it does not contain account data
+	if whoamiResponse.Data == nil || whoamiResponse.Data.Account == nil {
+		return nil, "", fmt.Errorf("failed to lookup account ID. data missing. you are most likely using a user token without configuring an account ID in your issuer config.")
+	}
+
+	accountID = strconv.FormatInt(whoamiResponse.Data.Account.ID, 10)
+	return client, accountID, nil
 }
 
 func (c *dnsimpleDNSProviderSolver) getDomainAndEntry(ch *v1alpha1.ChallengeRequest) (string, string) {
@@ -119,14 +138,9 @@ func (c *dnsimpleDNSProviderSolver) getDomainAndEntry(ch *v1alpha1.ChallengeRequ
 }
 
 func (c *dnsimpleDNSProviderSolver) getExistingRecord(cfg *dnsimpleDNSProviderConfig, client *dnsimple.Client, accountID string, zoneName string, entry string, key string) (*dnsimple.ZoneRecord, error) {
-	zone, err := client.Zones.GetZone(context.Background(), accountID, zoneName)
-
-	if err != nil {
-		return nil, fmt.Errorf("unable to get zone: %s", err)
-	}
 
 	// Look for existing TXT records.
-	records, err := client.Zones.ListRecords(context.Background(), accountID, zone.Data.Name, &dnsimple.ZoneRecordListOptions{Type: dnsimple.String("TXT"), Name: dnsimple.String(entry)})
+	records, err := client.Zones.ListRecords(context.Background(), accountID, zoneName, &dnsimple.ZoneRecordListOptions{Type: dnsimple.String("TXT"), Name: dnsimple.String(entry)})
 
 	if err != nil {
 		return nil, fmt.Errorf("unable to get resource records: %s", err)
@@ -173,15 +187,6 @@ func (c *dnsimpleDNSProviderSolver) deleteRecord(cfg *dnsimpleDNSProviderConfig,
 	return createdRecord.Data, nil
 }
 
-func Whoami(client *dnsimple.Client) (string, error) {
-	whoamiResponse, err := client.Identity.Whoami(context.Background())
-	if err != nil {
-		return "", err
-	}
-
-	return strconv.FormatInt(whoamiResponse.Data.Account.ID, 10), nil
-}
-
 // Present is responsible for actually presenting the DNS record with the
 // DNS provider.
 // This method should tolerate being called multiple times with the same value.
@@ -193,17 +198,12 @@ func (c *dnsimpleDNSProviderSolver) Present(ch *v1alpha1.ChallengeRequest) error
 		return err
 	}
 
-	client, err := c.getClient(&cfg, ch.ResourceNamespace)
+	client, accountID, err := c.getClient(&cfg, ch.ResourceNamespace)
 
 	if err != nil {
 		return fmt.Errorf("unable to get client: %s", err)
 	}
 
-	accountID, err := Whoami(client)
-	if err != nil {
-		return fmt.Errorf("unable to fetch account ID: %s", err)
-	}
-
 	entry, domain := c.getDomainAndEntry(ch)
 	klog.V(6).Infof("present for entry=%s, domain=%s", entry, domain)
 
@@ -242,17 +242,12 @@ func (c *dnsimpleDNSProviderSolver) CleanUp(ch *v1alpha1.ChallengeRequest) error
 		return err
 	}
 
-	client, err := c.getClient(&cfg, ch.ResourceNamespace)
+	client, accountID, err := c.getClient(&cfg, ch.ResourceNamespace)
 
 	if err != nil {
 		return fmt.Errorf("unable to get client: %s", err)
 	}
 
-	accountID, err := Whoami(client)
-	if err != nil {
-		return fmt.Errorf("unable to fetch account ID: %s", err)
-	}
-
 	entry, domain := c.getDomainAndEntry(ch)
 	klog.V(6).Infof("present for entry=%s, domain=%s", entry, domain)