From 594a8faa892fea990960f95524552825372c6fdc Mon Sep 17 00:00:00 2001 From: Christian Pearce Date: Thu, 23 Apr 2020 20:58:46 -0400 Subject: [PATCH] Fix for ER bandwidth reduction issue #3983 Reduction in bandwidth forces new resource, utilizing CustomizeDiff --- .../network/express_route_circuit_resource.go | 8 +++ .../express_route_circuit_resource_test.go | 62 ++++++++++++++++ .../helper/customdiff/compose.go | 72 +++++++++++++++++++ .../helper/customdiff/computed.go | 16 +++++ .../helper/customdiff/condition.go | 60 ++++++++++++++++ .../helper/customdiff/doc.go | 11 +++ .../helper/customdiff/force_new.go | 40 +++++++++++ .../helper/customdiff/validate.go | 38 ++++++++++ vendor/modules.txt | 1 + 9 files changed, 308 insertions(+) create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/compose.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/computed.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/condition.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/doc.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/force_new.go create mode 100644 vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/validate.go diff --git a/azurerm/internal/services/network/express_route_circuit_resource.go b/azurerm/internal/services/network/express_route_circuit_resource.go index e118898c2590..0263430428a4 100644 --- a/azurerm/internal/services/network/express_route_circuit_resource.go +++ b/azurerm/internal/services/network/express_route_circuit_resource.go @@ -6,6 +6,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-09-01/network" + "github.com/hashicorp/terraform-plugin-sdk/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" @@ -31,6 +32,13 @@ func resourceArmExpressRouteCircuit() *schema.Resource { State: schema.ImportStatePassthrough, }, + CustomizeDiff: customdiff.Sequence( + // If bandwidth is reduced force a new resource + customdiff.ForceNewIfChange("bandwidth_in_mbps", func(old, new, meta interface{}) bool { + return new.(int) < old.(int) + }), + ), + Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), Read: schema.DefaultTimeout(5 * time.Minute), diff --git a/azurerm/internal/services/network/tests/express_route_circuit_resource_test.go b/azurerm/internal/services/network/tests/express_route_circuit_resource_test.go index ee227c178bd4..851f599ba1f2 100644 --- a/azurerm/internal/services/network/tests/express_route_circuit_resource_test.go +++ b/azurerm/internal/services/network/tests/express_route_circuit_resource_test.go @@ -28,6 +28,7 @@ func TestAccAzureRMExpressRouteCircuit(t *testing.T) { "allowClassicOperationsUpdate": testAccAzureRMExpressRouteCircuit_allowClassicOperationsUpdate, "requiresImport": testAccAzureRMExpressRouteCircuit_requiresImport, "data_basic": testAccDataSourceAzureRMExpressRoute_basicMetered, + "bandwidthReduction": testAccAzureRMExpressRouteCircuit_bandwidthReduction, }, "PrivatePeering": { "azurePrivatePeering": testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeering, @@ -273,6 +274,33 @@ func testAccAzureRMExpressRouteCircuit_allowClassicOperationsUpdate(t *testing.T }) } +func testAccAzureRMExpressRouteCircuit_bandwidthReduction(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_express_route_circuit", "test") + var erc network.ExpressRouteCircuit + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMExpressRouteCircuitDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMExpressRouteCircuit_bandwidthReductionConfig(data, "100"), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMExpressRouteCircuitExists("azurerm_express_route_circuit.test", &erc), + resource.TestCheckResourceAttr(data.ResourceName, "bandwidth_in_mbps", "100"), + ), + }, + { + Config: testAccAzureRMExpressRouteCircuit_bandwidthReductionConfig(data, "50"), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMExpressRouteCircuitExists("azurerm_express_route_circuit.test", &erc), + resource.TestCheckResourceAttr(data.ResourceName, "bandwidth_in_mbps", "50"), + ), + }, + }, + }) +} + func testCheckAzureRMExpressRouteCircuitExists(resourceName string, erc *network.ExpressRouteCircuit) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] @@ -528,3 +556,37 @@ resource "azurerm_express_route_circuit" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, allowClassicOperations) } + +func testAccAzureRMExpressRouteCircuit_bandwidthReductionConfig(data acceptance.TestData, bandwidth string) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_express_route_circuit" "test" { + name = "acctest-erc-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + service_provider_name = "Equinix" + peering_location = "Silicon Valley" + bandwidth_in_mbps = %s + + sku { + tier = "Standard" + family = "MeteredData" + } + + allow_classic_operations = false + + tags = { + Environment = "production" + Purpose = "AcceptanceTests" + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, bandwidth) +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/compose.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/compose.go new file mode 100644 index 000000000000..b09199953e8e --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/compose.go @@ -0,0 +1,72 @@ +package customdiff + +import ( + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform-plugin-sdk/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(d *schema.ResourceDiff, meta interface{}) error { + var err error + for _, f := range funcs { + thisErr := f(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(d *schema.ResourceDiff, meta interface{}) error { + for _, f := range funcs { + err := f(d, meta) + if err != nil { + return err + } + } + return nil + } +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/computed.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/computed.go new file mode 100644 index 000000000000..54ea5c402069 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/computed.go @@ -0,0 +1,16 @@ +package customdiff + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +// ComputedIf returns a CustomizeDiffFunc that sets the given key's new value +// as computed if the given condition function returns true. +func ComputedIf(key string, f ResourceConditionFunc) schema.CustomizeDiffFunc { + return func(d *schema.ResourceDiff, meta interface{}) error { + if f(d, meta) { + d.SetNewComputed(key) + } + return nil + } +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/condition.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/condition.go new file mode 100644 index 000000000000..1d8e2bfd6558 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/condition.go @@ -0,0 +1,60 @@ +package customdiff + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +// ResourceConditionFunc is a function type that makes a boolean decision based +// on an entire resource diff. +type ResourceConditionFunc func(d *schema.ResourceDiff, meta interface{}) bool + +// ValueChangeConditionFunc is a function type that makes a boolean decision +// by comparing two values. +type ValueChangeConditionFunc func(old, new, meta interface{}) bool + +// ValueConditionFunc is a function type that makes a boolean decision based +// on a given value. +type ValueConditionFunc func(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(d *schema.ResourceDiff, meta interface{}) error { + if cond(d, meta) { + return f(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(d *schema.ResourceDiff, meta interface{}) error { + old, new := d.GetChange(key) + if cond(old, new, meta) { + return f(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(d *schema.ResourceDiff, meta interface{}) error { + if cond(d.Get(key), meta) { + return f(d, meta) + } + return nil + } +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/doc.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/doc.go new file mode 100644 index 000000000000..c6ad1199cdc6 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/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/helper/customdiff/force_new.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/force_new.go new file mode 100644 index 000000000000..26afa8cb697d --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/force_new.go @@ -0,0 +1,40 @@ +package customdiff + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +// 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. +func ForceNewIf(key string, f ResourceConditionFunc) schema.CustomizeDiffFunc { + return func(d *schema.ResourceDiff, meta interface{}) error { + if f(d, meta) { + d.ForceNew(key) + } + 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. +func ForceNewIfChange(key string, f ValueChangeConditionFunc) schema.CustomizeDiffFunc { + return func(d *schema.ResourceDiff, meta interface{}) error { + old, new := d.GetChange(key) + if f(old, new, meta) { + d.ForceNew(key) + } + return nil + } +} diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/validate.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/validate.go new file mode 100644 index 000000000000..0bc2c69505b6 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/customdiff/validate.go @@ -0,0 +1,38 @@ +package customdiff + +import ( + "github.com/hashicorp/terraform-plugin-sdk/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(old, new, meta interface{}) error + +// ValueValidationFunc is a function type that validates a particular value, +// returning an error if the value is invalid. +type ValueValidationFunc func(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(d *schema.ResourceDiff, meta interface{}) error { + old, new := d.GetChange(key) + return f(old, new, 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(d *schema.ResourceDiff, meta interface{}) error { + val := d.Get(key) + return f(val, meta) + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 779961db7630..be3a283f34ce 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -246,6 +246,7 @@ github.com/hashicorp/logutils github.com/hashicorp/terraform-config-inspect/tfconfig # github.com/hashicorp/terraform-plugin-sdk v1.6.0 github.com/hashicorp/terraform-plugin-sdk/helper/acctest +github.com/hashicorp/terraform-plugin-sdk/helper/customdiff github.com/hashicorp/terraform-plugin-sdk/helper/hashcode github.com/hashicorp/terraform-plugin-sdk/helper/logging github.com/hashicorp/terraform-plugin-sdk/helper/mutexkv