feat(domain): add waiting process for dns zone and associated reverse test (#1217)

Author: Monitob

go.mod

@@ -9,8 +9,9 @@ require (
 	github.com/hashicorp/aws-sdk-go-base v1.1.0
 	github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
 	github.com/hashicorp/go-retryablehttp v0.7.1
+	github.com/hashicorp/terraform-plugin-log v0.3.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.13.0
-	github.com/scaleway/scaleway-sdk-go v1.0.0-beta.9.0.20220414162120-5b6c03f0ce5f
+	github.com/scaleway/scaleway-sdk-go v1.0.0-beta.9.0.20220419094512-76deec7b31e7
 	github.com/stretchr/testify v1.7.1
 	golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f
 
@@ -44,7 +45,6 @@ require (
 	github.com/hashicorp/terraform-exec v0.16.1 // indirect
 	github.com/hashicorp/terraform-json v0.13.0 // indirect
 	github.com/hashicorp/terraform-plugin-go v0.9.0 // indirect
-	github.com/hashicorp/terraform-plugin-log v0.3.0 // indirect
 	github.com/hashicorp/terraform-registry-address v0.0.0-20220131103327-5c1c5e123275 // indirect
 	github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 // indirect
 	github.com/hashicorp/yamux v0.0.0-20211028200310-0bc27b27de87 // indirect

go.sum

@@ -868,8 +868,8 @@ github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb
 github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4=
 github.com/safchain/ethtool v0.0.0-20210803160452-9aa261dae9b1/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4=
 github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0=
-github.com/scaleway/scaleway-sdk-go v1.0.0-beta.9.0.20220414162120-5b6c03f0ce5f h1:iHIYJFID9/T8F3BDVymesruLN71NSUdpsV8m7s5eDEA=
-github.com/scaleway/scaleway-sdk-go v1.0.0-beta.9.0.20220414162120-5b6c03f0ce5f/go.mod h1:fCa7OJZ/9DRTnOKmxvT6pn+LPWUptQAmHF/SBJUGEcg=
+github.com/scaleway/scaleway-sdk-go v1.0.0-beta.9.0.20220419094512-76deec7b31e7 h1:zbYmcD37hndTsbmtrH9Asi672IVtYl2fTqulDZPUaIY=
+github.com/scaleway/scaleway-sdk-go v1.0.0-beta.9.0.20220419094512-76deec7b31e7/go.mod h1:fCa7OJZ/9DRTnOKmxvT6pn+LPWUptQAmHF/SBJUGEcg=
 github.com/sclevine/agouti v3.0.0+incompatible/go.mod h1:b4WX9W9L1sfQKXeJf1mUTLZKJ48R1S7H23Ji7oFO5Bw=
 github.com/sclevine/spec v1.2.0/go.mod h1:W4J29eT/Kzv7/b9IWLB055Z+qvVC9vt0Arko24q7p+U=
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=

scaleway/helpers.go

@@ -2,6 +2,7 @@ package scaleway
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -596,3 +597,18 @@ func toUint32(number interface{}) *uint32 {
 func errorCheck(err error, message string) bool {
 	return strings.Contains(err.Error(), message)
 }
+
+// ErrCodeEquals returns true if the error matches all these conditions:
+//  * err is of type scw.Error
+//  * Error.Error() equals one of the passed codes
+func ErrCodeEquals(err error, codes ...string) bool {
+	var scwErr scw.SdkError
+	if errors.As(err, &scwErr) {
+		for _, code := range codes {
+			if scwErr.Error() == code {
+				return true
+			}
+		}
+	}
+	return false
+}

scaleway/helpers_domain.go

@@ -1,16 +1,19 @@
 package scaleway
 
 import (
+	"context"
 	"net"
 	"strings"
 	"time"
 
 	domain "github.com/scaleway/scaleway-sdk-go/api/domain/v2beta1"
+	"github.com/scaleway/scaleway-sdk-go/scw"
 )
 
 const (
-	defaultDomainRecordTimeout = 30 * time.Second
-	defaultDomainZoneTimeout   = 30 * time.Second
+	defaultDomainRecordTimeout     = 30 * time.Second
+	defaultDomainZoneTimeout       = 30 * time.Second
+	defaultDomainZoneRetryInterval = 5 * time.Second
 )
 
 // domainAPI returns a new domain API.
@@ -238,3 +241,16 @@ func expandDomainView(i interface{}, ok bool) *domain.RecordViewConfig {
 		Views: views,
 	}
 }
+
+func waitForDNSZone(ctx context.Context, domainAPI *domain.API, dnsZone string, timeout time.Duration) (*domain.DNSZone, error) {
+	retryInterval := defaultDomainZoneRetryInterval
+	if DefaultWaitRetryInterval != nil {
+		retryInterval = *DefaultWaitRetryInterval
+	}
+
+	return domainAPI.WaitForDNSZone(&domain.WaitForDNSZoneRequest{
+		DNSZone:       dnsZone,
+		Timeout:       scw.TimeDurationPtr(timeout),
+		RetryInterval: scw.TimeDurationPtr(retryInterval),
+	}, scw.WithContext(ctx))
+}

scaleway/resource_domain_record.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -260,6 +261,11 @@ func resourceScalewayDomainRecordCreate(ctx context.Context, d *schema.ResourceD
 		return diag.FromErr(err)
 	}
 
+	_, err = waitForDNSZone(ctx, domainAPI, dnsZone, d.Timeout(schema.TimeoutCreate))
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
 	return resourceScalewayDomainRecordRead(ctx, d, meta)
 }
 
@@ -283,7 +289,7 @@ func resourceScalewayDomainRecordRead(ctx context.Context, d *schema.ResourceDat
 		res, err := domainAPI.ListDNSZoneRecords(&domain.ListDNSZoneRecordsRequest{
 			DNSZone: dnsZone,
 			ID:      &id,
-		}, scw.WithAllPages())
+		}, scw.WithAllPages(), scw.WithContext(ctx))
 
 		if err != nil {
 			if is404Error(err) {
@@ -407,6 +413,11 @@ func resourceScalewayDomainRecordUpdate(ctx context.Context, d *schema.ResourceD
 		if err != nil {
 			return diag.FromErr(err)
 		}
+
+		_, err = waitForDNSZone(ctx, domainAPI, d.Get("dns_zone").(string), d.Timeout(schema.TimeoutUpdate))
+		if err != nil {
+			return diag.FromErr(err)
+		}
 	}
 
 	return resourceScalewayDomainRecordRead(ctx, d, meta)
@@ -432,7 +443,15 @@ func resourceScalewayDomainRecordDelete(ctx context.Context, d *schema.ResourceD
 	}
 	d.SetId("")
 
-	// for non root zone, if the zone have only NS records, then delete the zone
+	_, err = waitForDNSZone(ctx, domainAPI, d.Get("dns_zone").(string), d.Timeout(schema.TimeoutDelete))
+	if err != nil && !ErrCodeEquals(err, domain.ErrCodeNoSuchDNSZone) {
+		if is404Error(err) {
+			return nil
+		}
+		return diag.FromErr(err)
+	}
+
+	// for non-root zone, if the zone have only NS records, then delete the zone
 	if !d.Get("keep_empty_zone").(bool) && d.Get("root_zone") != nil && !d.Get("root_zone").(bool) {
 		res, err := domainAPI.ListDNSZoneRecords(&domain.ListDNSZoneRecordsRequest{
 			DNSZone: d.Get("dns_zone").(string),
@@ -451,6 +470,7 @@ func resourceScalewayDomainRecordDelete(ctx context.Context, d *schema.ResourceD
 				hasRecords = true
 				break
 			}
+			tflog.Debug(ctx, fmt.Sprintf("record [%s], type [%s]", r.Name, r.Type))
 		}
 
 		if !hasRecords {
@@ -460,7 +480,7 @@ func resourceScalewayDomainRecordDelete(ctx context.Context, d *schema.ResourceD
 			})
 
 			if err != nil {
-				if is404Error(err) {
+				if is404Error(err) || is403Error(err) {
 					return nil
 				}
 				return diag.FromErr(err)

scaleway/resource_instance_ip.go

@@ -160,7 +160,12 @@ func resourceScalewayInstanceIPDelete(ctx context.Context, d *schema.ResourceDat
 		Zone: zone,
 	}, scw.WithContext(ctx))
 
-	if err != nil && !is404Error(err) && !is403Error(err) {
+	if err != nil {
+		// We check for 403 because instance API returns 403 for a deleted IP
+		if is404Error(err) || is403Error(err) {
+			d.SetId("")
+			return nil
+		}
 		return diag.FromErr(err)
 	}
 

scaleway/resource_instance_ip_reverse_dns_test.go

@@ -38,11 +38,6 @@ func TestAccScalewayInstanceIPReverseDns_Basic(t *testing.T) {
 					resource.TestCheckResourceAttr("scaleway_instance_ip_reverse_dns.base", "reverse", testDNSZone),
 				),
 			},
-			{
-				Config: `
-					resource "scaleway_instance_ip" "ip" {}
-				`,
-			},
 			{
 				Config: `
 					resource "scaleway_instance_ip" "ip" {}

scaleway/resource_instance_ip_test.go

@@ -230,20 +230,20 @@ func testAccCheckScalewayInstanceIPDestroy(tt *TestTools) resource.TestCheckFunc
 				return err
 			}
 
-			_, err = instanceAPI.GetIP(&instance.GetIPRequest{
+			_, errIP := instanceAPI.GetIP(&instance.GetIPRequest{
 				Zone: zone,
 				IP:   id,
 			})
 
 			// If no error resource still exist
-			if err == nil {
+			if errIP == nil {
 				return fmt.Errorf("resource %s(%s) still exist", rs.Type, rs.Primary.ID)
 			}
 
 			// Unexpected api error we return it
 			// We check for 403 because instance API return 403 for deleted IP
-			if !is404Error(err) && !is403Error(err) {
-				return err
+			if !is404Error(errIP) && !is403Error(errIP) {
+				return errIP
 			}
 		}
 

scaleway/resource_lb_frontend_test.go

@@ -85,7 +85,7 @@ func TestAccScalewayLbFrontend_Basic(t *testing.T) {
 // One possible way to circumvent this limitation is to generate for a random set of alternative domain names that are all subdomains of the main test domain.
 // For instance: *.test.scaleway-terraform.com which is a wildcard domain name.
 // And we generate certificate for foo.test.scaleway-terraform.com, bar.test.scaleway-terraform.com, baz.test.scaleway-terraform.com, etc.
-// Even changing one alternative domaine name is enough to count as a new certificate (which is rate limited by the 50 certificates per week limit and not the 5 duplicate certificates per week limit).
+// Even changing one alternative domain name is enough to count as a new certificate (which is rate limited by the 50 certificates per week limit and not the 5 duplicate certificates per week limit).
 // The only limitation is that all subdomains must resolve to the same IP address.
 func TestAccScalewayLbFrontend_Certificate(t *testing.T) {
 	tt := NewTestTools(t)