Merge pull request #1 from simplesurance/feat/storage-zone-improvements

Feat/storage zone improvements

README.md

@@ -4,7 +4,7 @@
 
 This repository provides a [Terraform](https://terraform.io) provider for the
 [Bunny.net CDN platform](https://bunny.net/). \
-It currently only supports to manage Pull Zones.
+It supports to manage Pull and Storage Zones.
 
 ## Development
 

docs/resources/storagezone.md

@@ -29,7 +29,6 @@ description: |-
 
 ### Read-Only
 
-- `date_modified` (String) The last modified date of the storage zone.
 - `deleted` (Boolean)
 - `files_stored` (Number) The number of files stored in the storage zone.
 - `id` (String) The ID of this resource.

examples/resources/storagezone_resource/basic.tf

@@ -0,0 +1,4 @@
+resource "bunny_storagezone" "mysz" {
+  name   = "testsz"
+  region = "DE"
+}

internal/provider/resource_edgerule_test.go

@@ -98,7 +98,7 @@ func defPullZoneHostname(pullzoneName string) string {
 }
 
 func TestAccEdgeRule_full(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 
 	tfPz := fmt.Sprintf(`
 resource "bunny_pullzone" "mypz" {
@@ -311,7 +311,7 @@ resource "bunny_edgerule" "er3" {
 }
 
 func TestAccEdgeRule_basic(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	tf := fmt.Sprintf(`
 resource "bunny_pullzone" "mypz" {
 	name = "%s"
@@ -363,7 +363,7 @@ resource "bunny_edgerule" "myer" {
 }
 
 func TestAccEdgeRule_delete(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 
 	tfPz := fmt.Sprintf(`
 resource "bunny_pullzone" "mypz" {
@@ -421,7 +421,7 @@ resource "bunny_pullzone" "mypz" {
 }
 
 func TestAccEdgeRule_enable_disable(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 
 	tfPz := fmt.Sprintf(`
 resource "bunny_pullzone" "mypz" {
@@ -558,8 +558,8 @@ resource "bunny_edgerule" "er2" {
 }
 
 func TestAccEdgeRule_changePullZoneID(t *testing.T) {
-	pzName1 := randPullZoneName()
-	pzName2 := randPullZoneName()
+	pzName1 := randResourceName()
+	pzName2 := randResourceName()
 
 	tfPz := fmt.Sprintf(`
 resource "bunny_pullzone" "pz1" {

internal/provider/resource_hostname_test.go

@@ -76,7 +76,7 @@ func sortHostnames(hostnames []*bunny.Hostname) {
 }
 
 func TestAccHostname_basic(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	tf := fmt.Sprintf(`
 resource "bunny_pullzone" "pz" {
 	name = "%s"
@@ -121,7 +121,7 @@ resource "bunny_hostname" "h1" {
 }
 
 func TestAccHostname_addRemove(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	hostname1 := randHostname()
 	hostname2 := randHostname()
 	hostname3 := randHostname()
@@ -244,7 +244,7 @@ resource "bunny_hostname" "h2" {
 }
 
 func TestAccHostname_DefiningDuplicateHostnamesFails(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	tf := fmt.Sprintf(`
 resource "bunny_pullzone" "pz" {
 	name = "%s"
@@ -275,7 +275,7 @@ resource "bunny_hostname" "h2" {
 }
 
 func TestAccHostname_DefiningDefPullZoneHostnameFails(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	tf := fmt.Sprintf(`
 resource "bunny_pullzone" "pz" {
 	name = "%s"
@@ -300,7 +300,7 @@ resource "bunny_hostname" "h1" {
 }
 
 func TestAccCertificateOneof(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	tf := fmt.Sprintf(`
 resource "bunny_pullzone" "pz" {
 	name = "%s"
@@ -333,7 +333,7 @@ resource "bunny_hostname" "h1" {
 }
 
 func TestAccCertificateCanBeSetWhenLoadFreeCertIsDisabled(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	tf := fmt.Sprintf(`
 resource "bunny_pullzone" "pz" {
 	name = "%s"
@@ -366,7 +366,7 @@ resource "bunny_hostname" "h1" {
 }
 
 func TestAccCertificates(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	hostname := randHostname()
 
 	resource.Test(t, resource.TestCase{
@@ -486,7 +486,7 @@ resource "bunny_hostname" "h1" {
 func TestAccHostname_StateIsValidWhenCertUploadFails(t *testing.T) {
 	t.Skip("disabled, because test sends 800kiB of bogus data to bunny api, which is not kind")
 
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 	hostname := randHostname()
 
 	// The bunny API does not return an error if the posted data is not a

internal/provider/resource_pullzone_test.go

@@ -18,10 +18,6 @@ import (
 	bunny "github.com/simplesurance/bunny-go"
 )
 
-func randPullZoneName() string {
-	return resource.PrefixedUniqueId(resourcePrefix)
-}
-
 func randHostname() string {
 	return resource.PrefixedUniqueId(resourcePrefix) + ".test"
 }
@@ -173,7 +169,7 @@ func TestAccPullZone_basic(t *testing.T) {
 	*/
 	attrs := pullZoneWanted{
 		TerraformResourceName: "bunny_pullzone.mytest1",
-		Name:                  randPullZoneName(),
+		Name:                  randResourceName(),
 		OriginURL:             "https://tabletennismap.de",
 		EnableGeoZoneAsia:     true,
 		EnableGeoZoneEU:       true,
@@ -285,7 +281,7 @@ func TestAccPullZone_full(t *testing.T) {
 		VerifyOriginSSL:                 ptr.ToBool(true),
 		ZoneSecurityEnabled:             ptr.ToBool(true),
 		ZoneSecurityIncludeHashRemoteIP: ptr.ToBool(false),
-		Name:                            ptr.ToString(randPullZoneName()),
+		Name:                            ptr.ToString(randResourceName()),
 		// TODO: Test StorageZoneID
 		ZoneSecurityKey: ptr.ToString("xyz"),
 
@@ -523,7 +519,7 @@ resource "bunny_pullzone" "%s" {
 }
 
 func TestAccPullZone_CaseInsensitiveOrderIndependentFields(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 
 	resource.Test(t, resource.TestCase{
 		Providers: testProviders,
@@ -772,7 +768,7 @@ func pzDiff(t *testing.T, a, b interface{}) []string {
 }
 
 func TestAccPullZone_OriginURLAndStorageZoneIDAreExclusive(t *testing.T) {
-	pzName := randPullZoneName()
+	pzName := randResourceName()
 
 	resource.Test(t, resource.TestCase{
 		Providers: testProviders,

internal/provider/resource_storagezone.go

@@ -16,7 +16,6 @@ import (
 const (
 	keyUserID             = "user_id"
 	keyPassword           = "password"
-	keyDateModified       = "date_modified"
 	keyDeleted            = "deleted"
 	keyStorageUsed        = "storage_used"
 	keyFilesStored        = "files_stored"
@@ -94,11 +93,6 @@ func resourceStorageZone() *schema.Resource {
 				Computed:    true,
 				Sensitive:   true,
 			},
-			keyDateModified: {
-				Type:        schema.TypeString,
-				Description: "The last modified date of the storage zone.",
-				Computed:    true,
-			},
 			keyDeleted: {
 				Type:     schema.TypeBool,
 				Computed: true,
@@ -166,6 +160,10 @@ func validateImmutableStringProperty(key string, old interface{}, new interface{
 	o := old.(string)
 	n, nok := new.(string)
 
+	if o == "" {
+		return nil
+	}
+
 	if new == nil || !nok {
 		return immutableStringPropertyError(key, o, "")
 	}
@@ -178,16 +176,16 @@ func validateImmutableStringProperty(key string, old interface{}, new interface{
 }
 
 func immutableStringPropertyError(key string, old string, new string) error {
-	message := "'%s' is immutable and cannot be changed from '%s' to '%s'. " +
-		"If you must change the '%s' of our region you must first delete your resource and then redefine it. " +
+	const message = "'%s' is immutable and cannot be changed from '%s' to '%s'.\n" +
+		"If you must change the '%s' of our region, first delete your resource and then redefine it.\n" +
 		"WARNING: deleting a 'bunny_storagezone' will also delete all the data it contains!"
 	return fmt.Errorf(message, key, old, new, key)
 }
 
 func immutableReplicationRegionError(key string, removed []interface{}) error {
-	message := "'%s' can be added to but not be removed once the zone has been created. " +
-		"This error occurred when attempting to remove values %+q from '%s'. " +
-		"To remove an existing '%s' the 'bunny_storagezone' must be deleted and recreated. " +
+	const message = "'%s' can be added but not removed once the zone has been created.\n" +
+		"This error occurred when attempting to remove values %+q from '%s'.\n" +
+		"To remove an existing '%s' the 'bunny_storagezone' must be deleted and recreated.\n" +
 		"WARNING: deleting a 'bunny_storagezone' will also delete all the data it contains!"
 	return fmt.Errorf(
 		message,
@@ -244,10 +242,7 @@ func resourceStorageZoneCreate(ctx context.Context, d *schema.ResourceData, meta
 func resourceStorageZoneUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	clt := meta.(*bunny.Client)
 
-	storageZone, err := storageZoneFromResource(d)
-	if err != nil {
-		return diagsErrFromErr("converting resource to API type failed", err)
-	}
+	storageZone := storageZoneFromResource(d)
 
 	id, err := getIDAsInt64(d)
 	if err != nil {
@@ -256,14 +251,6 @@ func resourceStorageZoneUpdate(ctx context.Context, d *schema.ResourceData, meta
 
 	updateErr := clt.StorageZone.Update(ctx, id, storageZone)
 	if updateErr != nil {
-		// if our update failed then revert our values to their original
-		// state so that we can run an apply again.
-		revertErr := revertUpdateValues(d)
-
-		if revertErr != nil {
-			return diagsErrFromErr("updating storage zone via API failed", revertErr)
-		}
-
 		return diagsErrFromErr("updating storage zone via API failed", updateErr)
 	}
 
@@ -332,9 +319,6 @@ func storageZoneToResource(sz *bunny.StorageZone, d *schema.ResourceData) error
 	if err := d.Set(keyPassword, sz.Password); err != nil {
 		return err
 	}
-	if err := d.Set(keyDateModified, sz.DateModified); err != nil {
-		return err
-	}
 	if err := d.Set(keyDeleted, sz.Deleted); err != nil {
 		return err
 	}
@@ -357,41 +341,13 @@ func storageZoneToResource(sz *bunny.StorageZone, d *schema.ResourceData) error
 	return nil
 }
 
-func revertUpdateValues(d *schema.ResourceData) error {
-	o, _ := d.GetChange(keyOriginURL)
-	if err := d.Set(keyOriginURL, o); err != nil {
-		return err
-	}
-	o, _ = d.GetChange(keyCustom404FilePath)
-	if err := d.Set(keyCustom404FilePath, o); err != nil {
-		return err
-	}
-	o, _ = d.GetChange(keyRewrite404To200)
-	if err := d.Set(keyRewrite404To200, o); err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // storageZoneFromResource returns a StorageZoneUpdateOptions API type that
 // has fields set to the values in d.
-func storageZoneFromResource(d *schema.ResourceData) (*bunny.StorageZoneUpdateOptions, error) {
-	var res bunny.StorageZoneUpdateOptions
-
-	res.ReplicationRegions = getStrSetAsSlice(d, keyReplicationRegions)
-
-	if d.HasChange(keyOriginURL) {
-		res.OriginURL = getStrPtr(d, keyOriginURL)
-	}
-
-	if d.HasChange(keyCustom404FilePath) {
-		res.Custom404FilePath = getStrPtr(d, keyCustom404FilePath)
-	}
-
-	if d.HasChange(keyRewrite404To200) {
-		res.Rewrite404To200 = getBoolPtr(d, keyRewrite404To200)
+func storageZoneFromResource(d *schema.ResourceData) *bunny.StorageZoneUpdateOptions {
+	return &bunny.StorageZoneUpdateOptions{
+		ReplicationRegions: getStrSetAsSlice(d, keyReplicationRegions),
+		OriginURL:          getOkStrPtr(d, keyOriginURL),
+		Custom404FilePath:  getOkStrPtr(d, keyCustom404FilePath),
+		Rewrite404To200:    getBoolPtr(d, keyRewrite404To200),
 	}
-
-	return &res, nil
 }