From 70d6aad3014c1c61ec307bc31e0be43fe085b9cd Mon Sep 17 00:00:00 2001 From: Jacob Spizziri Date: Tue, 14 Jun 2022 10:31:10 -0500 Subject: [PATCH 01/18] chore(vendor): add tf customdiff helper --- .../v2/helper/customdiff/compose.go | 75 +++++++++++++++++++ .../v2/helper/customdiff/computed.go | 33 ++++++++ .../v2/helper/customdiff/condition.go | 62 +++++++++++++++ .../v2/helper/customdiff/doc.go | 11 +++ .../v2/helper/customdiff/force_new.go | 71 ++++++++++++++++++ .../v2/helper/customdiff/validate.go | 40 ++++++++++ vendor/modules.txt | 1 + 7 files changed, 293 insertions(+) create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/compose.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/computed.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/condition.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/doc.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/force_new.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/validate.go diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/compose.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/compose.go new file mode 100644 index 0000000..9b1b929 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/compose.go @@ -0,0 +1,75 @@ +package customdiff + +import ( + "context" + + "github.com/hashicorp/go-multierror" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// All returns a CustomizeDiffFunc that runs all of the given +// CustomizeDiffFuncs and returns all of the errors produced. +// +// If one function produces an error, functions after it are still run. +// If this is not desirable, use function Sequence instead. +// +// If multiple functions returns errors, the result is a multierror. +// +// For example: +// +// &schema.Resource{ +// // ... +// CustomizeDiff: customdiff.All( +// customdiff.ValidateChange("size", func (old, new, meta interface{}) error { +// // If we are increasing "size" then the new value must be +// // a multiple of the old value. +// if new.(int) <= old.(int) { +// return nil +// } +// if (new.(int) % old.(int)) != 0 { +// return fmt.Errorf("new size value must be an integer multiple of old value %d", old.(int)) +// } +// return nil +// }), +// customdiff.ForceNewIfChange("size", func (old, new, meta interface{}) bool { +// // "size" can only increase in-place, so we must create a new resource +// // if it is decreased. +// return new.(int) < old.(int) +// }), +// customdiff.ComputedIf("version_id", func (d *schema.ResourceDiff, meta interface{}) bool { +// // Any change to "content" causes a new "version_id" to be allocated. +// return d.HasChange("content") +// }), +// ), +// } +// +func All(funcs ...schema.CustomizeDiffFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + var err error + for _, f := range funcs { + thisErr := f(ctx, d, meta) + if thisErr != nil { + err = multierror.Append(err, thisErr) + } + } + return err + } +} + +// Sequence returns a CustomizeDiffFunc that runs all of the given +// CustomizeDiffFuncs in sequence, stopping at the first one that returns +// an error and returning that error. +// +// If all functions succeed, the combined function also succeeds. +func Sequence(funcs ...schema.CustomizeDiffFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + for _, f := range funcs { + err := f(ctx, d, meta) + if err != nil { + return err + } + } + return nil + } +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/computed.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/computed.go new file mode 100644 index 0000000..c65a564 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/computed.go @@ -0,0 +1,33 @@ +package customdiff + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" +) + +// ComputedIf returns a CustomizeDiffFunc that sets the given key's new value +// as computed if the given condition function returns true. +// +// This function is best effort and will generate a warning log on any errors. +func ComputedIf(key string, f ResourceConditionFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + if f(ctx, d, meta) { + // To prevent backwards compatibility issues, this logic only + // generates a warning log instead of returning the error to + // the provider and ultimately the practitioner. Providers may + // not be aware of all situations in which the key may not be + // present in the data, such as during resource creation, so any + // further changes here should take that into account by + // documenting how to prevent the error. + if err := d.SetNewComputed(key); err != nil { + logging.HelperSchemaWarn(ctx, "unable to set attribute value to unknown", map[string]interface{}{ + logging.KeyAttributePath: key, + logging.KeyError: err, + }) + } + } + return nil + } +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/condition.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/condition.go new file mode 100644 index 0000000..40c06e7 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/condition.go @@ -0,0 +1,62 @@ +package customdiff + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// ResourceConditionFunc is a function type that makes a boolean decision based +// on an entire resource diff. +type ResourceConditionFunc func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) bool + +// ValueChangeConditionFunc is a function type that makes a boolean decision +// by comparing two values. +type ValueChangeConditionFunc func(ctx context.Context, oldValue, newValue, meta interface{}) bool + +// ValueConditionFunc is a function type that makes a boolean decision based +// on a given value. +type ValueConditionFunc func(ctx context.Context, value, meta interface{}) bool + +// If returns a CustomizeDiffFunc that calls the given condition +// function and then calls the given CustomizeDiffFunc only if the condition +// function returns true. +// +// This can be used to include conditional customizations when composing +// customizations using All and Sequence, but should generally be used only in +// simple scenarios. Prefer directly writing a CustomizeDiffFunc containing +// a conditional branch if the given CustomizeDiffFunc is already a +// locally-defined function, since this avoids obscuring the control flow. +func If(cond ResourceConditionFunc, f schema.CustomizeDiffFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + if cond(ctx, d, meta) { + return f(ctx, d, meta) + } + return nil + } +} + +// IfValueChange returns a CustomizeDiffFunc that calls the given condition +// function with the old and new values of the given key and then calls the +// given CustomizeDiffFunc only if the condition function returns true. +func IfValueChange(key string, cond ValueChangeConditionFunc, f schema.CustomizeDiffFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + oldValue, newValue := d.GetChange(key) + if cond(ctx, oldValue, newValue, meta) { + return f(ctx, d, meta) + } + return nil + } +} + +// IfValue returns a CustomizeDiffFunc that calls the given condition +// function with the new values of the given key and then calls the +// given CustomizeDiffFunc only if the condition function returns true. +func IfValue(key string, cond ValueConditionFunc, f schema.CustomizeDiffFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + if cond(ctx, d.Get(key), meta) { + return f(ctx, d, meta) + } + return nil + } +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/doc.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/doc.go new file mode 100644 index 0000000..c6ad119 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/doc.go @@ -0,0 +1,11 @@ +// Package customdiff provides a set of reusable and composable functions +// to enable more "declarative" use of the CustomizeDiff mechanism available +// for resources in package helper/schema. +// +// The intent of these helpers is to make the intent of a set of diff +// customizations easier to see, rather than lost in a sea of Go function +// boilerplate. They should _not_ be used in situations where they _obscure_ +// intent, e.g. by over-using the composition functions where a single +// function containing normal Go control flow statements would be more +// straightforward. +package customdiff diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/force_new.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/force_new.go new file mode 100644 index 0000000..f94257b --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/force_new.go @@ -0,0 +1,71 @@ +package customdiff + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" +) + +// ForceNewIf returns a CustomizeDiffFunc that flags the given key as +// requiring a new resource if the given condition function returns true. +// +// The return value of the condition function is ignored if the old and new +// values of the field compare equal, since no attribute diff is generated in +// that case. +// +// This function is best effort and will generate a warning log on any errors. +func ForceNewIf(key string, f ResourceConditionFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + if f(ctx, d, meta) { + // To prevent backwards compatibility issues, this logic only + // generates a warning log instead of returning the error to + // the provider and ultimately the practitioner. Providers may + // not be aware of all situations in which the key may not be + // present in the data, such as during resource creation, so any + // further changes here should take that into account by + // documenting how to prevent the error. + if err := d.ForceNew(key); err != nil { + logging.HelperSchemaWarn(ctx, "unable to require attribute replacement", map[string]interface{}{ + logging.KeyAttributePath: key, + logging.KeyError: err, + }) + } + } + return nil + } +} + +// ForceNewIfChange returns a CustomizeDiffFunc that flags the given key as +// requiring a new resource if the given condition function returns true. +// +// The return value of the condition function is ignored if the old and new +// values compare equal, since no attribute diff is generated in that case. +// +// This function is similar to ForceNewIf but provides the condition function +// only the old and new values of the given key, which leads to more compact +// and explicit code in the common case where the decision can be made with +// only the specific field value. +// +// This function is best effort and will generate a warning log on any errors. +func ForceNewIfChange(key string, f ValueChangeConditionFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + oldValue, newValue := d.GetChange(key) + if f(ctx, oldValue, newValue, meta) { + // To prevent backwards compatibility issues, this logic only + // generates a warning log instead of returning the error to + // the provider and ultimately the practitioner. Providers may + // not be aware of all situations in which the key may not be + // present in the data, such as during resource creation, so any + // further changes here should take that into account by + // documenting how to prevent the error. + if err := d.ForceNew(key); err != nil { + logging.HelperSchemaWarn(ctx, "unable to require attribute replacement", map[string]interface{}{ + logging.KeyAttributePath: key, + logging.KeyError: err, + }) + } + } + return nil + } +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/validate.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/validate.go new file mode 100644 index 0000000..ad381fb --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff/validate.go @@ -0,0 +1,40 @@ +package customdiff + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// ValueChangeValidationFunc is a function type that validates the difference +// (or lack thereof) between two values, returning an error if the change +// is invalid. +type ValueChangeValidationFunc func(ctx context.Context, oldValue, newValue, meta interface{}) error + +// ValueValidationFunc is a function type that validates a particular value, +// returning an error if the value is invalid. +type ValueValidationFunc func(ctx context.Context, value, meta interface{}) error + +// ValidateChange returns a CustomizeDiffFunc that applies the given validation +// function to the change for the given key, returning any error produced. +func ValidateChange(key string, f ValueChangeValidationFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + oldValue, newValue := d.GetChange(key) + return f(ctx, oldValue, newValue, meta) + } +} + +// ValidateValue returns a CustomizeDiffFunc that applies the given validation +// function to value of the given key, returning any error produced. +// +// This should generally not be used since it is functionally equivalent to +// a validation function applied directly to the schema attribute in question, +// but is provided for situations where composing multiple CustomizeDiffFuncs +// together makes intent clearer than spreading that validation across the +// schema. +func ValidateValue(key string, f ValueValidationFunc) schema.CustomizeDiffFunc { + return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + val := d.Get(key) + return f(ctx, val, meta) + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 3f860fb..34f282d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -145,6 +145,7 @@ github.com/hashicorp/terraform-plugin-log/tfsdklog # github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0 ## explicit; go 1.17 github.com/hashicorp/terraform-plugin-sdk/v2/diag +github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema From b29accc849874ba9be9bf41dc58799442eedc4db Mon Sep 17 00:00:00 2001 From: Jacob Spizziri Date: Wed, 1 Jun 2022 13:09:37 -0500 Subject: [PATCH 02/18] feat(bunny_storagezone): add a storagezone resource https://github.com/simplesurance/terraform-provider-bunny/issues/62 --- go.mod | 2 +- internal/provider/provider.go | 7 +- internal/provider/resource_storagezone.go | 397 ++++++++++++++++++ .../provider/resource_storagezone_test.go | 232 ++++++++++ 4 files changed, 634 insertions(+), 4 deletions(-) create mode 100644 internal/provider/resource_storagezone.go create mode 100644 internal/provider/resource_storagezone_test.go diff --git a/go.mod b/go.mod index 7ec2a41..275b90d 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.18 require ( github.com/AlekSi/pointer v1.2.0 + github.com/google/go-cmp v0.5.8 github.com/google/uuid v1.3.0 github.com/hashicorp/terraform-plugin-docs v0.10.1 github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0 @@ -21,7 +22,6 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/fatih/color v1.13.0 // indirect github.com/golang/protobuf v1.5.2 // indirect - github.com/google/go-cmp v0.5.8 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-checkpoint v0.5.0 // indirect diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 6e25fb1..9bbe92b 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -33,9 +33,10 @@ func New() *schema.Provider { }, }, ResourcesMap: map[string]*schema.Resource{ - "bunny_pullzone": resourcePullZone(), - "bunny_edgerule": resourceEdgeRule(), - "bunny_hostname": resourceHostname(), + "bunny_pullzone": resourcePullZone(), + "bunny_edgerule": resourceEdgeRule(), + "bunny_hostname": resourceHostname(), + "bunny_storagezone": resourceStorageZone(), }, ConfigureContextFunc: newProvider, } diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go new file mode 100644 index 0000000..2f91a53 --- /dev/null +++ b/internal/provider/resource_storagezone.go @@ -0,0 +1,397 @@ +package provider + +import ( + "context" + "fmt" + "strconv" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + bunny "github.com/simplesurance/bunny-go" +) + +const ( + keyUserID = "user_id" + keyPassword = "password" + keyDateModified = "date_modified" + keyDeleted = "deleted" + keyStorageUsed = "storage_used" + keyFilesStored = "files_stored" + keyRegion = "region" + keyReplicationRegions = "replication_regions" + keyReadOnlyPassword = "read_only_password" + keyCustom404FilePath = "custom_404_file_path" + keyRewrite404To200 = "rewrite_404_to_200" +) + +func resourceStorageZone() *schema.Resource { + return &schema.Resource{ + CreateContext: resourceStorageZoneCreate, + ReadContext: resourceStorageZoneRead, + UpdateContext: resourceStorageZoneUpdate, + DeleteContext: resourceStorageZoneDelete, + + Schema: map[string]*schema.Schema{ + // immutable properties + // NOTE: immutable properties are made immutable via + // validation in the `CustomizeDiff` function. There + // should be a validation function for each immutable + // property. + keyName: { + Type: schema.TypeString, + Description: "The name of the storage zone.", + Required: true, + }, + keyRegion: { + Type: schema.TypeString, + Description: "The code of the main storage zone region (Possible values: DE, NY, LA, SG).", + Optional: true, + Default: "DE", + ValidateDiagFunc: validation.ToDiagFunc( + validation.StringInSlice([]string{"DE", "NY", "LA", "SG"}, false), + ), + }, + keyReplicationRegions: { + Type: schema.TypeSet, + Description: "The list of replication zones for the storage zone (Possible values: DE, NY, LA, SG, SYD). Replication zones cannot be removed once the zone has been created.", + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateDiagFunc: validation.ToDiagFunc( + validation.StringInSlice([]string{"DE", "NY", "LA", "SG", "SYD"}, false), + ), + }, + Optional: true, + }, + + // mutable properties + keyOriginURL: { + Type: schema.TypeString, + Description: "The origin URL of the storage zone.", + Optional: true, + }, + keyCustom404FilePath: { + Type: schema.TypeString, + Description: "The path to the custom file that will be returned in a case of 404.", + Optional: true, + }, + keyRewrite404To200: { + Type: schema.TypeBool, + Description: "Rewrite 404 status code to 200 for URLs without extension.", + Optional: true, + }, + + // computed properties + keyUserID: { + Type: schema.TypeString, + Computed: true, + }, + keyPassword: { + Type: schema.TypeString, + Description: "The password granting read/write access to the storage zone.", + 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, + }, + keyStorageUsed: { + Type: schema.TypeInt, + Description: "The amount of storage used in the storage zone in bytes.", + Computed: true, + }, + keyFilesStored: { + Type: schema.TypeInt, + Description: "The number of files stored in the storage zone.", + Computed: true, + }, + keyReadOnlyPassword: { + Type: schema.TypeString, + Description: "The password granting read-only access to the storage zone.", + Computed: true, + Sensitive: true, + }, + }, + + CustomizeDiff: customdiff.All( + customdiff.ValidateChange(keyName, func(_ context.Context, old interface{}, new interface{}, meta interface{}) error { + return validateImmutableStringProperty(keyName, old, new) + }), + customdiff.ValidateChange(keyRegion, func(_ context.Context, old interface{}, new interface{}, meta interface{}) error { + return validateImmutableStringProperty(keyRegion, old, new) + }), + customdiff.ValidateChange(keyReplicationRegions, func(_ context.Context, old interface{}, new interface{}, meta interface{}) error { + if old == nil { + return nil + } + + var oldRep *schema.Set = old.(*schema.Set) + + if new == nil { + return immutableReplicationRegionError( + keyReplicationRegions, + oldRep.List(), + ) + } + + // verify that none of the previous replication regions have been removed. + var newRep *schema.Set = new.(*schema.Set) + var intersect *schema.Set = newRep.Intersection(oldRep) + var removed *schema.Set = oldRep.Difference(intersect) + + // NOTE: oldRep.Equal(intersect) doesn't work for some reason + areEqual := cmp.Equal(oldRep.List(), intersect.List()) + if !areEqual { + return immutableReplicationRegionError( + keyReplicationRegions, + removed.List(), + ) + } + + return nil + }), + ), + } +} + +func validateImmutableStringProperty(key string, old interface{}, new interface{}) error { + o := old.(string) + n, nok := new.(string) + + if new == nil || !nok { + return immutableStringPropertyError(key, o, "") + } + + if o != n { + return immutableStringPropertyError(key, o, n) + } + + return nil +} + +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. " + + "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. " + + "WARNING: deleting a 'bunny_storagezone' will also delete all the data it contains!" + return fmt.Errorf( + message, + key, + removed, + key, + key, + ) +} + +func resourceStorageZoneCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + clt := meta.(*bunny.Client) + + originURL := getStrPtr(d, keyOriginURL) + if !d.HasChange(keyOriginURL) { + originURL = nil + } + + sz, err := clt.StorageZone.Add(ctx, &bunny.StorageZoneAddOptions{ + Name: getStrPtr(d, keyName), + OriginURL: originURL, + Region: getStrPtr(d, keyRegion), + ReplicationRegions: getStrSetAsSlice(d, keyReplicationRegions), + }) + if err != nil { + return diag.FromErr(fmt.Errorf("creating storage zone failed: %w", err)) + } + + d.SetId(strconv.FormatInt(*sz.ID, 10)) + + // StorageZone.Add() only supports to set a subset of a Storage Zone object, + // call Update to set the remaining ones. + if diags := resourceStorageZoneUpdate(ctx, d, meta); diags.HasError() { + // if updating fails the sz was still created, initialize with the SZ + // returned from the Add operation + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "setting storage zone attributes via update failed", + }) + + if err := storageZoneToResource(sz, d); err != nil { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "converting api-type to resource data failed: " + err.Error(), + }) + } + + return diags + } + + return nil +} + +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) + } + + id, err := getIDAsInt64(d) + if err != nil { + return diag.FromErr(err) + } + + 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) + } + + updatedStorageZone, err := clt.StorageZone.Get(ctx, id) + if err != nil { + return diagsErrFromErr("fetching updated storage zone via API failed", err) + } + + if err := storageZoneToResource(updatedStorageZone, d); err != nil { + return diagsErrFromErr("converting api type to resource data after successful update failed", err) + } + + return nil +} + +func resourceStorageZoneRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + clt := meta.(*bunny.Client) + + id, err := getIDAsInt64(d) + if err != nil { + return diag.FromErr(err) + } + + sz, err := clt.StorageZone.Get(ctx, id) + if err != nil { + return diagsErrFromErr("could not retrieve storage zone", err) + } + + if err := storageZoneToResource(sz, d); err != nil { + return diagsErrFromErr("converting api type to resource data after successful read failed", err) + } + + return nil +} + +func resourceStorageZoneDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + clt := meta.(*bunny.Client) + + id, err := getIDAsInt64(d) + if err != nil { + return diag.FromErr(err) + } + + err = clt.StorageZone.Delete(ctx, id) + if err != nil { + return diagsErrFromErr("could not delete storage zone", err) + } + + d.SetId("") + + return nil +} + +// storageZoneToResource sets fields in d to the values in sz. +func storageZoneToResource(sz *bunny.StorageZone, d *schema.ResourceData) error { + if sz.ID != nil { + d.SetId(strconv.FormatInt(*sz.ID, 10)) + } + + if err := d.Set(keyUserID, sz.UserID); err != nil { + return err + } + if err := d.Set(keyName, sz.Name); err != nil { + return err + } + 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 + } + if err := d.Set(keyStorageUsed, sz.StorageUsed); err != nil { + return err + } + if err := d.Set(keyFilesStored, sz.FilesStored); err != nil { + return err + } + if err := d.Set(keyRegion, sz.Region); err != nil { + return err + } + if err := d.Set(keyReadOnlyPassword, sz.ReadOnlyPassword); err != nil { + return err + } + if err := setStrSet(d, keyReplicationRegions, sz.ReplicationRegions, ignoreOrderOpt, caseInsensitiveOpt); err != nil { + return err + } + + 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) + } + + return &res, nil +} diff --git a/internal/provider/resource_storagezone_test.go b/internal/provider/resource_storagezone_test.go new file mode 100644 index 0000000..6da5a85 --- /dev/null +++ b/internal/provider/resource_storagezone_test.go @@ -0,0 +1,232 @@ +package provider + +import ( + "context" + + "errors" + "fmt" + "strconv" + "strings" + "testing" + + ptr "github.com/AlekSi/pointer" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + + bunny "github.com/simplesurance/bunny-go" +) + +func randStorageZoneName() string { + return resource.PrefixedUniqueId(resourcePrefix) +} + +type storageZoneWanted struct { + TerraformResourceName string + bunny.StorageZone + Name string + Region string +} + +func checkBasicStorageZoneAPIState(wanted *storageZoneWanted) resource.TestCheckFunc { + return func(s *terraform.State) error { + clt := newAPIClient() + + strID, err := idFromState(s, wanted.TerraformResourceName) + if err != nil { + return err + } + + id, err := strconv.Atoi(strID) + if err != nil { + return fmt.Errorf("could not convert resource ID %q to int64: %w", id, err) + } + + sz, err := clt.StorageZone.Get(context.Background(), int64(id)) + if err != nil { + return fmt.Errorf("fetching storage-zone with id %d from api client failed: %w", id, err) + } + + if err := stringsAreEqual(wanted.Name, sz.Name); err != nil { + return fmt.Errorf("name of created storagezone differs: %w", err) + } + + return nil + } +} + +func checkStorageZoneNotExists(storageZoneName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + clt := newAPIClient() + + var page int32 + + for { + storagezones, err := clt.StorageZone.List(context.Background(), &bunny.PaginationOptions{ + Page: page, + PerPage: 1000, + }) + if err != nil { + return fmt.Errorf("listing storagezones failed: %w", err) + } + + for _, sz := range storagezones.Items { + if sz.Name == nil { + return fmt.Errorf("got storagezone from api with empty Name: %+v", sz) + } + + if storageZoneName == *sz.Name { + return &resource.UnexpectedStateError{ + State: "exists", + ExpectedState: []string{"not exists"}, + } + + } + + if !*storagezones.HasMoreItems { + return nil + } + + page++ + } + } + } +} + +func TestAccStorageZone_basic(t *testing.T) { + attrs := storageZoneWanted{ + TerraformResourceName: "bunny_storagezone.mytest1", + Name: randStorageZoneName(), + Region: "DE", + } + + tf := fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "%s" +} +`, + attrs.Name, + attrs.Region, + ) + + resource.Test(t, resource.TestCase{ + Providers: testProviders, + Steps: []resource.TestStep{ + { + Config: tf, + Check: checkBasicStorageZoneAPIState(&attrs), + }, + { + Config: tf, + Destroy: true, + }, + }, + CheckDestroy: checkStorageZoneNotExists(attrs.Name), + }) +} + +func TestAccStorageZone_full(t *testing.T) { + const resourceName = "mytest1" + const fullResourceName = "bunny_storagezone." + resourceName + + // set fields to different values then their defaults, to be able to test if the settings are applied + attrs := bunny.StorageZone{ + Name: ptr.ToString(randStorageZoneName()), + Region: ptr.ToString("DE"), + ReplicationRegions: []string{"NY", "LA"}, + } + + tf := fmt.Sprintf(` +resource "bunny_storagezone" "%s" { + name = "%s" + region = "%s" + replication_regions = %s + origin_url = "%s" + custom_404_file_path = "%s" + rewrite_404_to_200 = %t +} +`, + resourceName, + + ptr.GetString(attrs.Name), + ptr.GetString(attrs.Region), + tfStrList(attrs.ReplicationRegions), + "http://terraform.io", + "/custom_404.html", + false, + ) + + resource.Test(t, resource.TestCase{ + Providers: testProviders, + Steps: []resource.TestStep{ + { + Config: tf, + Check: checkSzState(t, fullResourceName, &attrs), + }, + { + Config: tf, + Destroy: true, + }, + }, + CheckDestroy: checkStorageZoneNotExists(fullResourceName), + }) +} + +func checkSzState(t *testing.T, resourceName string, wanted *bunny.StorageZone) resource.TestCheckFunc { + return func(s *terraform.State) error { + clt := newAPIClient() + + resourceState := s.Modules[0].Resources[resourceName] + if resourceState == nil { + return fmt.Errorf("resource %s not found in state", resourceName) + } + + insState := resourceState.Primary + if insState == nil { + return fmt.Errorf("resource %s has no primary state", resourceName) + } + + if insState.ID == "" { + return errors.New("ID is empty") + } + + id, err := strconv.Atoi(insState.ID) + if err != nil { + return fmt.Errorf("could not convert resource ID %q to int64: %w", id, err) + } + + sz, err := clt.StorageZone.Get(context.Background(), int64(id)) + if err != nil { + return fmt.Errorf("fetching storage-zone with id %d from api client failed: %w", id, err) + } + + diff := szDiff(t, wanted, sz) + if len(diff) != 0 { + return fmt.Errorf("wanted and actual state differs:\n%s", strings.Join(diff, "\n")) + } + + return nil + + } +} + +// storageZoneDiffIgnoredFields contains a list of fieldsnames in a bunny.StorageZone struct that are ignored by szDiff. +var storageZoneDiffIgnoredFields = map[string]struct{}{ + "ID": {}, // computed field + "UserID": {}, // computed field + "Password": {}, // computed field + "DateModified": {}, // computed field + "Deleted": {}, // computed field + "StorageUsed": {}, // computed field + "FilesStored": {}, // computed field + "ReadOnlyPassword": {}, // computed field + + // The following fields are tested by separate testcases and ignored in + // storage zone testcases. + "PullZones": {}, +} + +func szDiff(t *testing.T, a, b interface{}) []string { + t.Helper() + return diffStructs(t, a, b, storageZoneDiffIgnoredFields) +} From 7d73f1558215fe0bd3915ce7e1623b319b073f5b Mon Sep 17 00:00:00 2001 From: Jacob Spizziri Date: Tue, 14 Jun 2022 10:41:58 -0500 Subject: [PATCH 03/18] changelog: add entry for new storagezone resource --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a57c9e..a19c464 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ BUG FIXES: IMPROVEMENTS: * provider: upgrade github.com/hashicorp/terraform-plugin-docs to version 0.10.1 +* resource/storagezone: add the ability to manage storagezone resources ## 0.7.1 (Juni 08, 2022) From 5d540b65ceddf247fc38b393a06f64f72b035de2 Mon Sep 17 00:00:00 2001 From: Jacob Spizziri Date: Tue, 14 Jun 2022 10:42:33 -0500 Subject: [PATCH 04/18] docs(storagezone): generate documentation for storagezone resource --- docs/resources/storagezone.md | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 docs/resources/storagezone.md diff --git a/docs/resources/storagezone.md b/docs/resources/storagezone.md new file mode 100644 index 0000000..c89b7d4 --- /dev/null +++ b/docs/resources/storagezone.md @@ -0,0 +1,41 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "bunny_storagezone Resource - bunny" +subcategory: "" +description: |- + +--- + +# bunny_storagezone (Resource) + + + + + + +## Schema + +### Required + +- `name` (String) The name of the storage zone. + +### Optional + +- `custom_404_file_path` (String) The path to the custom file that will be returned in a case of 404. +- `origin_url` (String) The origin URL of the storage zone. +- `region` (String) The code of the main storage zone region (Possible values: DE, NY, LA, SG). +- `replication_regions` (Set of String) The list of replication zones for the storage zone (Possible values: DE, NY, LA, SG, SYD). Replication zones cannot be removed once the zone has been created. +- `rewrite_404_to_200` (Boolean) Rewrite 404 status code to 200 for URLs without extension. + +### 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. +- `password` (String, Sensitive) The password granting read/write access to the storage zone. +- `read_only_password` (String, Sensitive) The password granting read-only access to the storage zone. +- `storage_used` (Number) The amount of storage used in the storage zone in bytes. +- `user_id` (String) + + From 89bda18a4f1de46a17dc884e520d54c52f61e00c Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 09:56:57 +0200 Subject: [PATCH 05/18] storagezone: fix: creation fails Creating a storagezone failed with the immutable attribute errors: * 'name' is immutable and cannot be changed from '' to 'tf-test-20220615075051824800000019'.[..] * 'region' is immutable and cannot be changed from '' to 'DE'. [..] Fix it by always allowing changes in validateImmutableStringProperty if the old value was an empty string. --- internal/provider/resource_storagezone.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 2f91a53..1b338e8 100644 --- a/internal/provider/resource_storagezone.go +++ b/internal/provider/resource_storagezone.go @@ -166,6 +166,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, "") } From 0ebb31b8b06268c1cab317b2ed1ac92d10c2d24e Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 10:45:26 +0200 Subject: [PATCH 06/18] storagezone: improve error message for immutable fields - add newlines to make the message easier to read, - make the message vars constants, --- internal/provider/resource_storagezone.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 1b338e8..6767f5d 100644 --- a/internal/provider/resource_storagezone.go +++ b/internal/provider/resource_storagezone.go @@ -182,16 +182,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, From 103e042798dc875ba5564a89b8eadaf1549aa4b3 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 10:46:15 +0200 Subject: [PATCH 07/18] storagezone: remove date_modified key --- docs/resources/storagezone.md | 1 - internal/provider/resource_storagezone.go | 9 --------- 2 files changed, 10 deletions(-) diff --git a/docs/resources/storagezone.md b/docs/resources/storagezone.md index c89b7d4..7bd574e 100644 --- a/docs/resources/storagezone.md +++ b/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. diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 6767f5d..97a0e77 100644 --- a/internal/provider/resource_storagezone.go +++ b/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, @@ -336,9 +330,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 } From 932b9e2149f15501f11752a14a0d05fc81a1c634 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 12:53:58 +0200 Subject: [PATCH 08/18] storagezone: remove setting state to current values on failed update It is not needed. d.Get() only returns the proposed state, if updating fails it is not stored as current state. --- internal/provider/resource_storagezone.go | 25 ----------------------- 1 file changed, 25 deletions(-) diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 97a0e77..85979b8 100644 --- a/internal/provider/resource_storagezone.go +++ b/internal/provider/resource_storagezone.go @@ -254,14 +254,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) } @@ -352,23 +344,6 @@ 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) { From 773b9beca8d5ae51bf7b3279f019e6dceedcb96c Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 12:57:10 +0200 Subject: [PATCH 09/18] simplify storageZoneFromResource storageZoneFromResource was only setting fields in *bunny.StorageZoneUpdateOptions that changed in the resource. Change it to set always all fields to the current values. This is consistent with how other *FromResource functions are behaving. The update message sent to the bunny API will contain all settings also the current ones instead of only the changed ones. Depending on how the bunny.net endpoint handles missing fields in JSON payloads for update messages this could be better or worse. :-) If it always interprets missing fields as no change is done, it causes only unnecessary network traffic. If it interprets missing fields as the setting should be unset/disabled, it's better to include the whole configuration. The bunny.net API distinguishes between missing and empty string values for the OriginURL and Custom404FilePath fields. When submitting empty strings for those it resulted in an error, that those fields set to invalid values. To handle it the helper function getOkStrPtr() is introduced. It returns nil if the value is unset instead of an empty string. Replacing all getPtr() calls with getOkPtr() might make sense and should be investigated in a follow-up. --- internal/provider/resource_storagezone.go | 28 ++++++--------------- internal/provider/types_resource_getters.go | 12 +++++++++ 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 85979b8..99bff48 100644 --- a/internal/provider/resource_storagezone.go +++ b/internal/provider/resource_storagezone.go @@ -242,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 { @@ -346,22 +343,11 @@ func storageZoneToResource(sz *bunny.StorageZone, d *schema.ResourceData) error // 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 } diff --git a/internal/provider/types_resource_getters.go b/internal/provider/types_resource_getters.go index ec7337f..95b4ac9 100644 --- a/internal/provider/types_resource_getters.go +++ b/internal/provider/types_resource_getters.go @@ -8,6 +8,18 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) +// getOkStrPtr returns the value of the string field keyName in d. +// If the field is not it returns nil. +func getOkStrPtr(d *schema.ResourceData, keyName string) *string { + val, isSet := d.GetOk(keyName) + if !isSet || val == nil { + return nil + } + + v := val.(string) + return &v +} + func getStrPtr(d *schema.ResourceData, keyName string) *string { val := d.Get(keyName) if val == nil { From cc6281881532c3b662546375d83ce18c0ccd3ae1 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 12:58:37 +0200 Subject: [PATCH 10/18] examples: add a simple storagezone resource example --- examples/resources/storagezone_resource/basic.tf | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 examples/resources/storagezone_resource/basic.tf diff --git a/examples/resources/storagezone_resource/basic.tf b/examples/resources/storagezone_resource/basic.tf new file mode 100644 index 0000000..b857781 --- /dev/null +++ b/examples/resources/storagezone_resource/basic.tf @@ -0,0 +1,4 @@ +resource "bunny_storagezone" "mysz" { + name = "testsz" + region = "DE" +} From 0764bd1c19d5de5d2a543aea34b561cd39a7e1d3 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 15:49:06 +0200 Subject: [PATCH 11/18] tests: add testcase for immutable storagezone fields Add a testcase that ensures changing an immutable storagezone field fails --- .../provider/resource_storagezone_test.go | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/internal/provider/resource_storagezone_test.go b/internal/provider/resource_storagezone_test.go index 6da5a85..fa687f1 100644 --- a/internal/provider/resource_storagezone_test.go +++ b/internal/provider/resource_storagezone_test.go @@ -2,6 +2,7 @@ package provider import ( "context" + "regexp" "errors" "fmt" @@ -172,6 +173,94 @@ resource "bunny_storagezone" "%s" { }) } +func TestChangingImmutableFieldsFails(t *testing.T) { + const resourceName = "mytest1" + const fullResourceName = "bunny_storagezone." + resourceName + storageZoneName := randStorageZoneName() + + attrs := bunny.StorageZone{ + Name: ptr.ToString(storageZoneName), + Region: ptr.ToString("NY"), + ReplicationRegions: []string{"DE"}, + } + + resource.Test(t, resource.TestCase{ + Providers: testProviders, + Steps: []resource.TestStep{ + // create storagezone + { + Config: fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "%s" + replication_regions = %s +} +`, + storageZoneName, + *attrs.Region, + tfStrList(attrs.ReplicationRegions), + ), + Check: checkSzState(t, fullResourceName, &attrs), + }, + // change region + { + Config: fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "LA" + replication_regions = ["DE"] +} +`, + storageZoneName, + ), + ExpectError: regexp.MustCompile(".*'region' is immutable.*"), + }, + // change name + { + Config: fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "LA" + replication_regions = ["DE"] +} +`, + storageZoneName+resource.UniqueId(), + ), + Check: checkSzState(t, fullResourceName, &attrs), + ExpectError: regexp.MustCompile(".*'name' is immutable.*"), + }, + // replace a replication_region + { + Config: fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "NY" + replication_regions = ["LA"] +} +`, + storageZoneName, + ), + Check: checkSzState(t, fullResourceName, &attrs), + ExpectError: regexp.MustCompile(".*'replication_regions' can be added but not removed.*"), + }, + // remove replication_region + { + Config: fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "NY" +} +`, + storageZoneName, + ), + Check: checkSzState(t, fullResourceName, &attrs), + ExpectError: regexp.MustCompile(".*'replication_regions' can be added but not removed.*"), + }, + }, + CheckDestroy: checkStorageZoneNotExists(fullResourceName), + }) +} + func checkSzState(t *testing.T, resourceName string, wanted *bunny.StorageZone) resource.TestCheckFunc { return func(s *terraform.State) error { clt := newAPIClient() From a4ca60679ed4ab923fe0459dd42044c6fb1ad6f3 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 16:04:54 +0200 Subject: [PATCH 12/18] readme: mention support for storage zones --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f713481..f057de9 100644 --- a/README.md +++ b/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 From ab029029c65e10eee96338fdcbfe9e710b2feb23 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Wed, 15 Jun 2022 16:07:00 +0200 Subject: [PATCH 13/18] tests: replace randPullZoneName(), randStorageZoneName() with 1 function replace the randPullZoneName() and randStorageZoneName() functions that were doing the same with a function called randResourceName() --- internal/provider/resource_edgerule_test.go | 12 ++++++------ internal/provider/resource_hostname_test.go | 16 ++++++++-------- internal/provider/resource_pullzone_test.go | 12 ++++-------- internal/provider/resource_storagezone_test.go | 10 +++------- internal/provider/utils_test.go | 5 +++++ 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/internal/provider/resource_edgerule_test.go b/internal/provider/resource_edgerule_test.go index 54b6f2a..284a961 100644 --- a/internal/provider/resource_edgerule_test.go +++ b/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" { diff --git a/internal/provider/resource_hostname_test.go b/internal/provider/resource_hostname_test.go index 1242d83..b0bdf3f 100644 --- a/internal/provider/resource_hostname_test.go +++ b/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 diff --git a/internal/provider/resource_pullzone_test.go b/internal/provider/resource_pullzone_test.go index c5a4696..6bba1a4 100644 --- a/internal/provider/resource_pullzone_test.go +++ b/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, diff --git a/internal/provider/resource_storagezone_test.go b/internal/provider/resource_storagezone_test.go index fa687f1..35369cc 100644 --- a/internal/provider/resource_storagezone_test.go +++ b/internal/provider/resource_storagezone_test.go @@ -17,10 +17,6 @@ import ( bunny "github.com/simplesurance/bunny-go" ) -func randStorageZoneName() string { - return resource.PrefixedUniqueId(resourcePrefix) -} - type storageZoneWanted struct { TerraformResourceName string bunny.StorageZone @@ -96,7 +92,7 @@ func checkStorageZoneNotExists(storageZoneName string) resource.TestCheckFunc { func TestAccStorageZone_basic(t *testing.T) { attrs := storageZoneWanted{ TerraformResourceName: "bunny_storagezone.mytest1", - Name: randStorageZoneName(), + Name: randResourceName(), Region: "DE", } @@ -132,7 +128,7 @@ func TestAccStorageZone_full(t *testing.T) { // set fields to different values then their defaults, to be able to test if the settings are applied attrs := bunny.StorageZone{ - Name: ptr.ToString(randStorageZoneName()), + Name: ptr.ToString(randResourceName()), Region: ptr.ToString("DE"), ReplicationRegions: []string{"NY", "LA"}, } @@ -176,7 +172,7 @@ resource "bunny_storagezone" "%s" { func TestChangingImmutableFieldsFails(t *testing.T) { const resourceName = "mytest1" const fullResourceName = "bunny_storagezone." + resourceName - storageZoneName := randStorageZoneName() + storageZoneName := randResourceName() attrs := bunny.StorageZone{ Name: ptr.ToString(storageZoneName), diff --git a/internal/provider/utils_test.go b/internal/provider/utils_test.go index ea81f3f..d7bedb5 100644 --- a/internal/provider/utils_test.go +++ b/internal/provider/utils_test.go @@ -6,9 +6,14 @@ import ( "reflect" "testing" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) +func randResourceName() string { + return resource.PrefixedUniqueId(resourcePrefix) +} + func idFromState(s *terraform.State, resourceName string) (string, error) { resourceState := s.Modules[0].Resources[resourceName] if resourceState == nil { From 69eb1c6740f09898bbcc1db62567c174c96a47d6 Mon Sep 17 00:00:00 2001 From: Jacob Spizziri Date: Wed, 15 Jun 2022 11:31:18 -0500 Subject: [PATCH 14/18] feat(storagezone): add appropriate default values for 404 properties --- internal/provider/resource_storagezone.go | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 99bff48..047644b 100644 --- a/internal/provider/resource_storagezone.go +++ b/internal/provider/resource_storagezone.go @@ -75,11 +75,13 @@ func resourceStorageZone() *schema.Resource { Type: schema.TypeString, Description: "The path to the custom file that will be returned in a case of 404.", Optional: true, + Default: "/bunnycdn_errors/404.html", }, keyRewrite404To200: { Type: schema.TypeBool, Description: "Rewrite 404 status code to 200 for URLs without extension.", Optional: true, + Default: false, }, // computed properties @@ -251,6 +253,14 @@ 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) } @@ -341,6 +351,23 @@ 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 { From 445fff84b0ed141699eb1736658b4bcc328f5956 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Thu, 16 Jun 2022 09:55:03 +0200 Subject: [PATCH 15/18] tests: add testcase that ensures failed update does not change current state --- .../provider/resource_storagezone_test.go | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/internal/provider/resource_storagezone_test.go b/internal/provider/resource_storagezone_test.go index 35369cc..4a0473c 100644 --- a/internal/provider/resource_storagezone_test.go +++ b/internal/provider/resource_storagezone_test.go @@ -295,6 +295,61 @@ func checkSzState(t *testing.T, resourceName string, wanted *bunny.StorageZone) } } +// TestAccFailedUpdateDoesNotApplychanges tests the scenario described in +// https://github.com/5-stones/terraform-provider-bunny/pull/1#discussion_r898134629 +func TestAccFailedUpdateDoesNotApplychanges(t *testing.T) { + attrs := storageZoneWanted{ + TerraformResourceName: "bunny_storagezone.mytest1", + Name: randResourceName(), + Region: "DE", + } + + resource.Test(t, resource.TestCase{ + Providers: testProviders, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "%s" + custom_404_file_path = "/error.html" +}`, + attrs.Name, + attrs.Region, + ), + Check: checkBasicStorageZoneAPIState(&attrs), + }, + // change custom_404_file_path to a value that spawns a generation error on bunny side + { + Config: fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "%s" + custom_404_file_path = "abc" +}`, + attrs.Name, + attrs.Region, + ), + Check: checkBasicStorageZoneAPIState(&attrs), + ExpectError: regexp.MustCompile(".*updating storage zone via API failed.*"), + }, + { + Config: fmt.Sprintf(` +resource "bunny_storagezone" "mytest1" { + name = "%s" + region = "%s" + custom_404_file_path = "/error.html" +}`, + attrs.Name, + attrs.Region, + ), + PlanOnly: true, + }, + }, + CheckDestroy: checkStorageZoneNotExists(attrs.Name), + }) +} + // storageZoneDiffIgnoredFields contains a list of fieldsnames in a bunny.StorageZone struct that are ignored by szDiff. var storageZoneDiffIgnoredFields = map[string]struct{}{ "ID": {}, // computed field From 5711315a28a55342b78950784f41be19f3c0e526 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Thu, 16 Jun 2022 10:50:36 +0200 Subject: [PATCH 16/18] storagezone: use d.Partial in Update instead of revertUpdateValues That after a failed update changes to the resource are still applied is a know bug (https://github.com/hashicorp/terraform-plugin-sdk/issues/476). It affects attributes that are not retrieved via the Get function and set only by the provider. custom_404_file_path is such a field. As workaround d.Partial() can be called. This is more simple then the revertUpdateValues() implementation. --- internal/provider/resource_storagezone.go | 30 ++++------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 047644b..1f2cd2b 100644 --- a/internal/provider/resource_storagezone.go +++ b/internal/provider/resource_storagezone.go @@ -253,14 +253,11 @@ 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) - } - + // The storagezone contains fields /custom_404_file_path) that are only updated by the + // provider and not retrieved via ReadContext(). This causes that we run into the bug + // https://github.com/hashicorp/terraform-plugin-sdk/issues/476. + // As workaround d.Partial(true) is called. + d.Partial(true) return diagsErrFromErr("updating storage zone via API failed", updateErr) } @@ -351,23 +348,6 @@ 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 { From ea76f21eb8db45ba78c823316836113131851a04 Mon Sep 17 00:00:00 2001 From: Jacob Spizziri Date: Thu, 16 Jun 2022 11:02:29 -0500 Subject: [PATCH 17/18] fix(storagezone): fix typo in error message for immutable properties --- internal/provider/resource_storagezone.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 1f2cd2b..5c500af 100644 --- a/internal/provider/resource_storagezone.go +++ b/internal/provider/resource_storagezone.go @@ -179,7 +179,7 @@ func validateImmutableStringProperty(key string, old interface{}, new interface{ func immutableStringPropertyError(key string, old string, new string) error { 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" + + "To change the 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, key, old, new, key) } From bc126db7e87c5d86a64e95a5230a67432a421219 Mon Sep 17 00:00:00 2001 From: Jacob Spizziri Date: Thu, 16 Jun 2022 11:11:47 -0500 Subject: [PATCH 18/18] feat(storagezone): remove unnecessary state read/write on update --- internal/provider/resource_storagezone.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/internal/provider/resource_storagezone.go b/internal/provider/resource_storagezone.go index 5c500af..4e9428e 100644 --- a/internal/provider/resource_storagezone.go +++ b/internal/provider/resource_storagezone.go @@ -261,15 +261,6 @@ func resourceStorageZoneUpdate(ctx context.Context, d *schema.ResourceData, meta return diagsErrFromErr("updating storage zone via API failed", updateErr) } - updatedStorageZone, err := clt.StorageZone.Get(ctx, id) - if err != nil { - return diagsErrFromErr("fetching updated storage zone via API failed", err) - } - - if err := storageZoneToResource(updatedStorageZone, d); err != nil { - return diagsErrFromErr("converting api type to resource data after successful update failed", err) - } - return nil }