Improve error messages from hcloud-go

This improves the err messages for certain error codes returned from the hcloud api. As a sample: invalid_input errors now show which field contains which error. I decided to not implement this in the hcloud-go because we want to have a different outputs based on the specific project where the hcloud-go is used. The output here is optimized to show all errors with the minimal needed space (single log line) and also already in mind to have more of those special cases. While implementing this i also fixed a smaller bug in the firewall tests and replaced the deprecated scopelint. Signed-off-by: Lukas Kämmerling <lukas.kaemmerling@hetzner-cloud.de>
hetznercloud · Jun 21, 2021 · 1795d37 · 1795d37
1 parent 68a3d6a
commit 1795d37
Show file tree

Hide file tree

Showing 35 changed files with 271 additions and 221 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -21,7 +21,7 @@ linters:
     - misspell
     - prealloc
     - rowserrcheck
-    - scopelint
+    - exportloopref
     - staticcheck
     - structcheck
     - typecheck
@@ -35,7 +35,7 @@ issues:
     - path: _test\.go
       linters:
         - gosec
-        - scopelint
+        - exportloopref
         - errcheck
     - linters:
         - gosec

diff --git a/hcloud/provider.go b/hcloud/provider.go
@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/hetznercloud/terraform-provider-hcloud/internal/firewall"
+	"github.com/hetznercloud/terraform-provider-hcloud/internal/hcclient"
 
 	"github.com/hetznercloud/terraform-provider-hcloud/internal/snapshot"
 
@@ -118,7 +119,7 @@ func providerConfigure(_ context.Context, d *schema.ResourceData) (interface{},
 	if pollInterval, ok := d.GetOk("poll_interval"); ok {
 		pollInterval, err := time.ParseDuration(pollInterval.(string))
 		if err != nil {
-			return nil, diag.FromErr(err)
+			return nil, hcclient.ErrorToDiag(err)
 		}
 		opts = append(opts, hcloud.WithPollInterval(pollInterval))
 	}

diff --git a/internal/certificate/data_source.go b/internal/certificate/data_source.go
@@ -7,6 +7,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hetznercloud/hcloud-go/hcloud"
+	"github.com/hetznercloud/terraform-provider-hcloud/internal/hcclient"
 )
 
 // DataSourceType is the type name of the Hetzner Cloud Certificate resource.
@@ -76,7 +77,7 @@ func dataSourceHcloudCertificateRead(ctx context.Context, d *schema.ResourceData
 	if id, ok := d.GetOk("id"); ok {
 		cert, _, err := client.Certificate.GetByID(ctx, id.(int))
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if cert == nil {
 			return diag.Errorf("certificate not found: id: %d", id)
@@ -87,7 +88,7 @@ func dataSourceHcloudCertificateRead(ctx context.Context, d *schema.ResourceData
 	if name, ok := d.GetOk("name"); ok {
 		cert, _, err := client.Certificate.Get(ctx, name.(string))
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if cert == nil {
 			return diag.Errorf("certificate not found: name: %s", name)
@@ -104,13 +105,13 @@ func dataSourceHcloudCertificateRead(ctx context.Context, d *schema.ResourceData
 		}
 		allCertificates, err := client.Certificate.AllWithOpts(ctx, opts)
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if len(allCertificates) == 0 {
-			return diag.FromErr(fmt.Errorf("no Certificate found for selector %q", selector))
+			return hcclient.ErrorToDiag(fmt.Errorf("no Certificate found for selector %q", selector))
 		}
 		if len(allCertificates) > 1 {
-			return diag.FromErr(fmt.Errorf("more than one Certificate found for selector %q", selector))
+			return hcclient.ErrorToDiag(fmt.Errorf("more than one Certificate found for selector %q", selector))
 		}
 		setCertificateSchema(d, allCertificates[0])
 		return nil

diff --git a/internal/certificate/resource.go b/internal/certificate/resource.go
@@ -175,7 +175,7 @@ func createUploadedResource(ctx context.Context, d *schema.ResourceData, m inter
 
 	res, _, err := client.Certificate.Create(ctx, opts)
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 	d.SetId(strconv.Itoa(res.ID))
 	return readResource(ctx, d, m)
@@ -204,10 +204,10 @@ func createManagedResource(ctx context.Context, d *schema.ResourceData, m interf
 
 	res, _, err := c.Certificate.CreateCertificate(ctx, opts)
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 	if err := hcclient.WaitForAction(ctx, &c.Action, res.Action); err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 	d.SetId(strconv.Itoa(res.Certificate.ID))
 
@@ -222,7 +222,7 @@ func readResource(ctx context.Context, d *schema.ResourceData, m interface{}) di
 		if resourceCertificateNotFound(err, d) {
 			return nil
 		}
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 	if cert == nil {
 		d.SetId("")
@@ -261,7 +261,7 @@ func updateResource(ctx context.Context, d *schema.ResourceData, m interface{})
 
 	cert, _, err := client.Certificate.Get(ctx, d.Id())
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 	if cert == nil {
 		d.SetId("")
@@ -274,7 +274,7 @@ func updateResource(ctx context.Context, d *schema.ResourceData, m interface{})
 			Name: d.Get("name").(string),
 		}
 		if _, _, err := client.Certificate.Update(ctx, cert, opts); err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 	}
 	if d.HasChange("labels") {
@@ -285,7 +285,7 @@ func updateResource(ctx context.Context, d *schema.ResourceData, m interface{})
 			opts.Labels[k] = v.(string)
 		}
 		if _, _, err := client.Certificate.Update(ctx, cert, opts); err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 	}
 	d.Partial(false)
@@ -306,7 +306,7 @@ func deleteResource(ctx context.Context, d *schema.ResourceData, m interface{})
 			// certificate has already been deleted
 			return nil
 		}
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 	d.SetId("")
 	return nil

diff --git a/internal/datacenter/data_source.go b/internal/datacenter/data_source.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hetznercloud/hcloud-go/hcloud"
+	"github.com/hetznercloud/terraform-provider-hcloud/internal/hcclient"
 )
 
 const (
@@ -68,7 +69,7 @@ func dataSourceHcloudDatacenterRead(ctx context.Context, data *schema.ResourceDa
 	if id, ok := data.GetOk("id"); ok {
 		d, _, err := client.Datacenter.GetByID(ctx, id.(int))
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if d == nil {
 			return diag.Errorf("no datacenter found with id %d", id)
@@ -79,7 +80,7 @@ func dataSourceHcloudDatacenterRead(ctx context.Context, data *schema.ResourceDa
 	if name, ok := data.GetOk("name"); ok {
 		d, _, err := client.Datacenter.GetByName(ctx, name.(string))
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if d == nil {
 			return diag.Errorf("no datacenter found with name %v", name)
@@ -148,7 +149,7 @@ func dataSourceHcloudDatacentersRead(ctx context.Context, d *schema.ResourceData
 	client := m.(*hcloud.Client)
 	dcs, err := client.Datacenter.All(ctx)
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 
 	names := make([]string, 0, len(dcs))

diff --git a/internal/e2etests/firewall/resource_test.go b/internal/e2etests/firewall/resource_test.go
@@ -76,6 +76,7 @@ func TestFirewallResource_Basic(t *testing.T) {
 						fmt.Sprintf("basic-firewall--%d", tmplMan.RandInt)),
 					resource.TestCheckResourceAttr(res.TFID(), "rule.#", "3"),
 					testsupport.LiftTCF(hasFirewallRule(t, &f, "in", "80", "tcp", []string{"0.0.0.0/0", "::/0"}, []string{})),
+					testsupport.LiftTCF(hasFirewallRule(t, &f, "in", "any", "udp", []string{"0.0.0.0/0", "::/0"}, []string{})),
 					testsupport.LiftTCF(hasFirewallRule(t, &f, "out", "80", "tcp", []string{}, []string{"0.0.0.0/0", "::/0"})),
 				),
 			},
@@ -95,6 +96,7 @@ func TestFirewallResource_Basic(t *testing.T) {
 						fmt.Sprintf("basic-firewall--%d", tmplMan.RandInt)),
 					resource.TestCheckResourceAttr(res.TFID(), "rule.#", "3"),
 					testsupport.LiftTCF(hasFirewallRule(t, &f, "in", "443", "tcp", []string{"0.0.0.0/0", "::/0"}, []string{})),
+					testsupport.LiftTCF(hasFirewallRule(t, &f, "in", "any", "udp", []string{"0.0.0.0/0", "::/0"}, []string{})),
 					testsupport.LiftTCF(hasFirewallRule(t, &f, "out", "443", "tcp", []string{}, []string{"0.0.0.0/0", "::/0"})),
 				),
 			},

diff --git a/internal/firewall/data_source.go b/internal/firewall/data_source.go
@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hetznercloud/hcloud-go/hcloud"
+	"github.com/hetznercloud/terraform-provider-hcloud/internal/hcclient"
 )
 
 // DataSourceType is the type name of the Hetzner Cloud Firewall resource.
@@ -81,7 +82,7 @@ func dataSourceHcloudFirewallRead(ctx context.Context, d *schema.ResourceData, m
 	if id, ok := d.GetOk("id"); ok {
 		i, _, err := client.Firewall.GetByID(ctx, id.(int))
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if i == nil {
 			return diag.Errorf("no firewall found with id %d", id)
@@ -92,7 +93,7 @@ func dataSourceHcloudFirewallRead(ctx context.Context, d *schema.ResourceData, m
 	if name, ok := d.GetOk("name"); ok {
 		i, _, err := client.Firewall.GetByName(ctx, name.(string))
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if i == nil {
 			return diag.Errorf("no firewall found with name %v", name)
@@ -107,7 +108,7 @@ func dataSourceHcloudFirewallRead(ctx context.Context, d *schema.ResourceData, m
 		opts := hcloud.FirewallListOpts{ListOpts: hcloud.ListOpts{LabelSelector: selector.(string)}}
 		allFirewalls, err := client.Firewall.AllWithOpts(ctx, opts)
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if len(allFirewalls) == 0 {
 			return diag.Errorf("no firewall found for selector %q", selector)

diff --git a/internal/firewall/resource.go b/internal/firewall/resource.go
@@ -124,12 +124,12 @@ func resourceFirewallCreate(ctx context.Context, d *schema.ResourceData, m inter
 
 	res, _, err := client.Firewall.Create(ctx, opts)
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 
 	for _, nextAction := range res.Actions {
 		if err := hcclient.WaitForAction(ctx, &client.Action, nextAction); err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 	}
 	d.SetId(strconv.Itoa(res.Firewall.ID))
@@ -193,7 +193,7 @@ func resourceFirewallRead(ctx context.Context, d *schema.ResourceData, m interfa
 
 	firewall, _, err := client.Firewall.GetByID(ctx, id)
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 	if firewall == nil {
 		log.Printf("[WARN] firewall (%s) not found, removing from state", d.Id())
@@ -219,7 +219,7 @@ func resourceFirewallUpdate(ctx context.Context, d *schema.ResourceData, m inter
 		if resourceFirewallIsNotFound(err, d) {
 			return nil
 		}
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 
 	d.Partial(true)
@@ -233,7 +233,7 @@ func resourceFirewallUpdate(ctx context.Context, d *schema.ResourceData, m inter
 			if resourceFirewallIsNotFound(err, d) {
 				return nil
 			}
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 	}
 
@@ -249,10 +249,10 @@ func resourceFirewallUpdate(ctx context.Context, d *schema.ResourceData, m inter
 				if resourceFirewallIsNotFound(err, d) {
 					return nil
 				}
-				return diag.FromErr(err)
+				return hcclient.ErrorToDiag(err)
 			}
 			if err := waitForFirewallActions(ctx, client, actions, firewall); err != nil {
-				return diag.FromErr(err)
+				return hcclient.ErrorToDiag(err)
 			}
 		}
 	}
@@ -270,7 +270,7 @@ func resourceFirewallUpdate(ctx context.Context, d *schema.ResourceData, m inter
 			if resourceFirewallIsNotFound(err, d) {
 				return nil
 			}
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 	}
 	d.Partial(false)
@@ -289,7 +289,7 @@ func resourceFirewallDelete(ctx context.Context, d *schema.ResourceData, m inter
 	}
 	firewall, _, err := client.Firewall.GetByID(ctx, firewallID)
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 
 	err = control.Retry(control.DefaultRetries, func() error {
@@ -309,7 +309,7 @@ func resourceFirewallDelete(ctx context.Context, d *schema.ResourceData, m inter
 		return nil
 	})
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 
 	return nil

diff --git a/internal/firewall/validation.go b/internal/firewall/validation.go
@@ -5,13 +5,14 @@ import (
 
 	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hetznercloud/terraform-provider-hcloud/internal/hcclient"
 )
 
 func validateIPDiag(i interface{}, path cty.Path) diag.Diagnostics {
 	ipS := i.(string)
 	ip, n, err := net.ParseCIDR(ipS)
 	if err != nil {
-		return diag.FromErr(err)
+		return hcclient.ErrorToDiag(err)
 	}
 	if ip.String() != n.IP.String() {
 		return diag.Errorf("%s is not the start of the cidr block %s", ipS, n)

diff --git a/internal/floatingip/data_source.go b/internal/floatingip/data_source.go
@@ -3,6 +3,8 @@ package floatingip
 import (
 	"context"
 
+	"github.com/hetznercloud/terraform-provider-hcloud/internal/hcclient"
+
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -77,7 +79,7 @@ func dataSourceHcloudFloatingIPRead(ctx context.Context, d *schema.ResourceData,
 	if id, ok := d.GetOk("id"); ok {
 		f, _, err := client.FloatingIP.GetByID(ctx, id.(int))
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if f == nil {
 			return diag.Errorf("no Floating IP found with id %d", id)
@@ -88,7 +90,7 @@ func dataSourceHcloudFloatingIPRead(ctx context.Context, d *schema.ResourceData,
 	if name, ok := d.GetOk("name"); ok {
 		f, _, err := client.FloatingIP.GetByName(ctx, name.(string))
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if f == nil {
 			return diag.Errorf("no Floating IP found with name %s", name)
@@ -100,7 +102,7 @@ func dataSourceHcloudFloatingIPRead(ctx context.Context, d *schema.ResourceData,
 		var allIPs []*hcloud.FloatingIP
 		allIPs, err := client.FloatingIP.All(ctx)
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 
 		// Find by 'ip_address'
@@ -128,7 +130,7 @@ func dataSourceHcloudFloatingIPRead(ctx context.Context, d *schema.ResourceData,
 		}
 		allIPs, err := client.FloatingIP.AllWithOpts(ctx, opts)
 		if err != nil {
-			return diag.FromErr(err)
+			return hcclient.ErrorToDiag(err)
 		}
 		if len(allIPs) == 0 {
 			return diag.Errorf("no Floating IP found for selector %q", selector)