From 50a2923322aa4a53c7597ba2467efc223e60dd59 Mon Sep 17 00:00:00 2001 From: Ayush Rangwala Date: Thu, 7 Mar 2024 22:13:53 +0530 Subject: [PATCH] Add caseinsensitive plan modifier for metro attribute --- internal/planmodifiers/caseinsensitive.go | 35 ++++++++++++ .../planmodifiers/caseinsensitive_test.go | 56 +++++++++++++++++++ internal/provider/provider.go | 6 +- .../metal/{vlans => vlan}/datasource.go | 4 +- .../{vlans => vlan}/datasource_schema.go | 2 +- .../metal/{vlans => vlan}/datasource_test.go | 6 +- .../resources/metal/{vlans => vlan}/models.go | 18 ++++-- .../metal/{vlans => vlan}/resource.go | 18 +++++- .../metal/{vlans => vlan}/resource_schema.go | 10 ++-- .../metal/{vlans => vlan}/resource_test.go | 36 +++++++++++- .../metal/{vlans => vlan}/sweeper.go | 10 ++-- internal/sweep/sweep_test.go | 4 +- 12 files changed, 177 insertions(+), 28 deletions(-) create mode 100644 internal/planmodifiers/caseinsensitive.go create mode 100644 internal/planmodifiers/caseinsensitive_test.go rename internal/resources/metal/{vlans => vlan}/datasource.go (97%) rename internal/resources/metal/{vlans => vlan}/datasource_schema.go (99%) rename internal/resources/metal/{vlans => vlan}/datasource_test.go (98%) rename internal/resources/metal/{vlans => vlan}/models.go (76%) rename internal/resources/metal/{vlans => vlan}/resource.go (93%) rename internal/resources/metal/{vlans => vlan}/resource_schema.go (89%) rename internal/resources/metal/{vlans => vlan}/resource_test.go (85%) rename internal/resources/metal/{vlans => vlan}/sweeper.go (88%) diff --git a/internal/planmodifiers/caseinsensitive.go b/internal/planmodifiers/caseinsensitive.go new file mode 100644 index 000000000..88f3be6bb --- /dev/null +++ b/internal/planmodifiers/caseinsensitive.go @@ -0,0 +1,35 @@ +package planmodifiers + +import ( + "context" + "strings" + + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +func CaseInsensitiveString() planmodifier.String { + return &caseInsensitivePlanModifier{} +} + +type caseInsensitivePlanModifier struct{} + +func (d *caseInsensitivePlanModifier) PlanModifyString(ctx context.Context, request planmodifier.StringRequest, response *planmodifier.StringResponse) { + oldValue := request.StateValue.ValueString() + newValue := request.PlanValue.ValueString() + + result := oldValue + if !strings.EqualFold(strings.ToLower(newValue), strings.ToLower(oldValue)) { + result = newValue + } + + response.PlanValue = types.StringValue(result) +} + +func (d *caseInsensitivePlanModifier) Description(ctx context.Context) string { + return "For same string but different cases, does not trigger diffs in the plan" +} + +func (d *caseInsensitivePlanModifier) MarkdownDescription(ctx context.Context) string { + return d.Description(ctx) +} diff --git a/internal/planmodifiers/caseinsensitive_test.go b/internal/planmodifiers/caseinsensitive_test.go new file mode 100644 index 000000000..a061718d8 --- /dev/null +++ b/internal/planmodifiers/caseinsensitive_test.go @@ -0,0 +1,56 @@ +package planmodifiers + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +func TestCaseInsensitiveSet(t *testing.T) { + testCases := []struct { + Old, New, Expected string + }{ + { + Old: "foo", + New: "foo", + Expected: "foo", + }, + { + Old: "Bar", + New: "bar", + Expected: "Bar", + }, + { + Old: "foo", + New: "fOO", + Expected: "foo", + }, + } + + testPlanModifier := CaseInsensitiveString() + + for i, testCase := range testCases { + stateValue := types.StringValue(testCase.Old) + planValue := types.StringValue(testCase.New) + expectedValue := types.StringValue(testCase.Expected) + + req := planmodifier.StringRequest{ + StateValue: stateValue, + PlanValue: planValue, + } + + var resp planmodifier.StringResponse + + testPlanModifier.PlanModifyString(context.Background(), req, &resp) + + if resp.Diagnostics.HasError() { + t.Fatalf("%d: got error modifying plan: %v", i, resp.Diagnostics.Errors()) + } + + if !resp.PlanValue.Equal(expectedValue) { + t.Fatalf("%d: output plan value does not equal expected plan value", i) + } + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 14ed66083..ef9565b72 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -18,7 +18,7 @@ import ( metalorganization "github.com/equinix/terraform-provider-equinix/internal/resources/metal/organization" metalprojectsshkey "github.com/equinix/terraform-provider-equinix/internal/resources/metal/project_ssh_key" metalsshkey "github.com/equinix/terraform-provider-equinix/internal/resources/metal/ssh_key" - "github.com/equinix/terraform-provider-equinix/internal/resources/metal/vlans" + "github.com/equinix/terraform-provider-equinix/internal/resources/metal/vlan" equinix_validation "github.com/equinix/terraform-provider-equinix/internal/validation" ) @@ -118,7 +118,7 @@ func (p *FrameworkProvider) Resources(ctx context.Context) []func() resource.Res metalsshkey.NewResource, metalconnection.NewResource, metalorganization.NewResource, - vlans.NewResource, + vlan.NewResource, } } @@ -128,6 +128,6 @@ func (p *FrameworkProvider) DataSources(ctx context.Context) []func() datasource metalprojectsshkey.NewDataSource, metalconnection.NewDataSource, metalorganization.NewDataSource, - vlans.NewDataSource, + vlan.NewDataSource, } } diff --git a/internal/resources/metal/vlans/datasource.go b/internal/resources/metal/vlan/datasource.go similarity index 97% rename from internal/resources/metal/vlans/datasource.go rename to internal/resources/metal/vlan/datasource.go index d9c7aadc9..1fa0353d2 100644 --- a/internal/resources/metal/vlans/datasource.go +++ b/internal/resources/metal/vlan/datasource.go @@ -1,4 +1,4 @@ -package vlans +package vlan import ( "context" @@ -81,7 +81,7 @@ func (r *DataSource) Read( &packngo.GetOptions{Includes: []string{"assigned_to"}}, ) if err != nil { - resp.Diagnostics.AddError("Error fetching vlans list for projectId", + resp.Diagnostics.AddError("Error fetching vlan list for projectId", equinix_errors.FriendlyError(err).Error()) return } diff --git a/internal/resources/metal/vlans/datasource_schema.go b/internal/resources/metal/vlan/datasource_schema.go similarity index 99% rename from internal/resources/metal/vlans/datasource_schema.go rename to internal/resources/metal/vlan/datasource_schema.go index 32498e840..4f0fb525c 100644 --- a/internal/resources/metal/vlans/datasource_schema.go +++ b/internal/resources/metal/vlan/datasource_schema.go @@ -1,4 +1,4 @@ -package vlans +package vlan import ( "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" diff --git a/internal/resources/metal/vlans/datasource_test.go b/internal/resources/metal/vlan/datasource_test.go similarity index 98% rename from internal/resources/metal/vlans/datasource_test.go rename to internal/resources/metal/vlan/datasource_test.go index 27628cb0f..6688f46e0 100644 --- a/internal/resources/metal/vlans/datasource_test.go +++ b/internal/resources/metal/vlan/datasource_test.go @@ -1,4 +1,4 @@ -package vlans_test +package vlan_test import ( "fmt" @@ -7,7 +7,7 @@ import ( "github.com/equinix/terraform-provider-equinix/internal/acceptance" "github.com/equinix/terraform-provider-equinix/internal/config" - "github.com/equinix/terraform-provider-equinix/internal/resources/metal/vlans" + "github.com/equinix/terraform-provider-equinix/internal/resources/metal/vlan" "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -265,7 +265,7 @@ func TestMetalVlan_matchingVlan(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := vlans.MatchingVlan(tt.args.vlans, tt.args.vxlan, tt.args.projectID, tt.args.facility, tt.args.metro) + got, err := vlan.MatchingVlan(tt.args.vlans, tt.args.vxlan, tt.args.projectID, tt.args.facility, tt.args.metro) if (err != nil) != tt.wantErr { t.Errorf("matchingVlan() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/internal/resources/metal/vlans/models.go b/internal/resources/metal/vlan/models.go similarity index 76% rename from internal/resources/metal/vlans/models.go rename to internal/resources/metal/vlan/models.go index a60b3da14..bb4e61286 100644 --- a/internal/resources/metal/vlans/models.go +++ b/internal/resources/metal/vlan/models.go @@ -1,4 +1,4 @@ -package vlans +package vlan import ( "context" @@ -69,13 +69,23 @@ func (m *ResourceModel) parse(vlan *packngo.VirtualNetwork) diag.Diagnostics { m.ProjectID = types.StringValue(vlan.Project.ID) } + var metroCode, facilityCode types.String if vlan.Facility != nil { - m.Facility = types.StringValue(vlan.Facility.Code) - m.Metro = types.StringValue(strings.ToLower(vlan.Facility.Metro.Code)) + facilityCode = types.StringValue(vlan.Facility.Code) + metroCode = types.StringValue(strings.ToLower(vlan.Facility.Metro.Code)) + } + // version of this resource. StateFunc doesn't exist in terraform and it requires implementation + // of bespoke logic before storing state. To ensure backward compatibility we ignore lower/upper + // case diff for now, but we may want to require input upper case + if !strings.EqualFold(m.Facility.ValueString(), facilityCode.ValueString()) { + m.Facility = facilityCode } if vlan.Metro != nil { - m.Metro = types.StringValue(strings.ToLower(vlan.Metro.Code)) + metroCode = types.StringValue(strings.ToLower(vlan.Metro.Code)) + } + if !strings.EqualFold(m.Metro.ValueString(), metroCode.ValueString()) { + m.Metro = metroCode } return nil } diff --git a/internal/resources/metal/vlans/resource.go b/internal/resources/metal/vlan/resource.go similarity index 93% rename from internal/resources/metal/vlans/resource.go rename to internal/resources/metal/vlan/resource.go index 143e23e04..23028f6a2 100644 --- a/internal/resources/metal/vlans/resource.go +++ b/internal/resources/metal/vlan/resource.go @@ -1,4 +1,4 @@ -package vlans +package vlan import ( "context" @@ -6,6 +6,7 @@ import ( "fmt" equinix_errors "github.com/equinix/terraform-provider-equinix/internal/errors" "github.com/equinix/terraform-provider-equinix/internal/framework" + "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/packethost/packngo" @@ -62,7 +63,7 @@ func (r *Resource) Create(ctx context.Context, request resource.CreateRequest, r } if !data.Facility.IsNull() && !data.Vxlan.IsNull() { response.Diagnostics.AddError("Invalid input params", - equinix_errors.FriendlyError(errors.New("you can set vxlan only for metro vlans")).Error()) + equinix_errors.FriendlyError(errors.New("you can set vxlan only for metro vlan")).Error()) return } @@ -137,7 +138,18 @@ func (r *Resource) Read(ctx context.Context, request resource.ReadRequest, respo response.Diagnostics.Append(response.State.Set(ctx, &data)...) } -func (r *Resource) Update(_ context.Context, _ resource.UpdateRequest, _ *resource.UpdateResponse) {} +func (r *Resource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + var data ResourceModel + if diag := req.Plan.Get(ctx, &data); diag.HasError() { + resp.Diagnostics.Append(diag...) + return + } + + if diag := resp.State.Set(ctx, &data); diag.HasError() { + resp.Diagnostics.Append(diag...) + return + } +} func (r *Resource) Delete(ctx context.Context, request resource.DeleteRequest, response *resource.DeleteResponse) { r.Meta.AddFwModuleToMetalUserAgent(ctx, request.ProviderMeta) diff --git a/internal/resources/metal/vlans/resource_schema.go b/internal/resources/metal/vlan/resource_schema.go similarity index 89% rename from internal/resources/metal/vlans/resource_schema.go rename to internal/resources/metal/vlan/resource_schema.go index b1fcee622..3ad6f5261 100644 --- a/internal/resources/metal/vlans/resource_schema.go +++ b/internal/resources/metal/vlan/resource_schema.go @@ -1,8 +1,8 @@ -package vlans +package vlan import ( "context" - + equinixplanmodifiers "github.com/equinix/terraform-provider-equinix/internal/planmodifiers" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource/schema" @@ -54,11 +54,13 @@ func resourceSchema(ctx context.Context) schema.Schema { Optional: true, Computed: true, PlanModifiers: []planmodifier.String{ - stringplanmodifier.RequiresReplace(), + //stringplanmodifier.RequiresReplace(), + equinixplanmodifiers.CaseInsensitiveString(), }, Validators: []validator.String{ stringvalidator.ConflictsWith(path.MatchRoot("facility")), - stringvalidator.AtLeastOneOf(path.MatchRoot("facility"), path.MatchRoot("metro")), + stringvalidator.ExactlyOneOf(path.MatchRoot("facility"), + path.MatchRoot("metro")), }, }, "vxlan": schema.Int64Attribute{ diff --git a/internal/resources/metal/vlans/resource_test.go b/internal/resources/metal/vlan/resource_test.go similarity index 85% rename from internal/resources/metal/vlans/resource_test.go rename to internal/resources/metal/vlan/resource_test.go index 61cf64b82..17b5a858c 100644 --- a/internal/resources/metal/vlans/resource_test.go +++ b/internal/resources/metal/vlan/resource_test.go @@ -1,7 +1,8 @@ -package vlans_test +package vlan_test import ( "fmt" + "strings" "testing" "github.com/equinix/terraform-provider-equinix/internal/acceptance" @@ -207,3 +208,36 @@ func TestAccMetalVlan_metro_upgradeFromVersion(t *testing.T) { }, }) } + +func TestAccMetalVlan_metro_suppress_diff(t *testing.T) { + var vlan packngo.VirtualNetwork + rs := acctest.RandString(10) + metro := "sv" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.TestAccPreCheckMetal(t) }, + ExternalProviders: acceptance.TestExternalProviders, + ProtoV5ProviderFactories: acceptance.ProtoV5ProviderFactories, + CheckDestroy: testAccMetalVlanCheckDestroyed, + Steps: []resource.TestStep{ + { + Config: testAccCheckMetalVlanConfig_metro(rs, metro, "tfacc-vlan"), + Check: resource.ComposeTestCheckFunc( + testAccCheckMetalVlanExists("equinix_metal_vlan.foovlan", &vlan), + resource.TestCheckResourceAttr( + "equinix_metal_vlan.foovlan", "description", "tfacc-vlan"), + resource.TestCheckResourceAttr( + "equinix_metal_vlan.foovlan", "metro", metro), + ), + }, + { + Config: testAccCheckMetalVlanConfig_metro(rs, strings.ToUpper(metro), "tfacc-vlan"), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + }, + }, + }) +} diff --git a/internal/resources/metal/vlans/sweeper.go b/internal/resources/metal/vlan/sweeper.go similarity index 88% rename from internal/resources/metal/vlans/sweeper.go rename to internal/resources/metal/vlan/sweeper.go index 7bb00ff69..b55788bbd 100644 --- a/internal/resources/metal/vlans/sweeper.go +++ b/internal/resources/metal/vlan/sweeper.go @@ -1,4 +1,4 @@ -package vlans +package vlan import ( "fmt" @@ -17,15 +17,15 @@ func AddTestSweeper() { } func testSweepVlans(region string) error { - log.Printf("[DEBUG] Sweeping vlans") + log.Printf("[DEBUG] Sweeping vlan") config, err := sweep.GetConfigForMetal() if err != nil { - return fmt.Errorf("[INFO][SWEEPER_LOG] Error getting configuration for sweeping vlans: %s", err) + return fmt.Errorf("[INFO][SWEEPER_LOG] Error getting configuration for sweeping vlan: %s", err) } metal := config.NewMetalClient() ps, _, err := metal.Projects.List(nil) if err != nil { - return fmt.Errorf("[INFO][SWEEPER_LOG] Error getting project list for sweeping vlans: %s", err) + return fmt.Errorf("[INFO][SWEEPER_LOG] Error getting project list for sweeping vlan: %s", err) } pids := []string{} for _, p := range ps { @@ -37,7 +37,7 @@ func testSweepVlans(region string) error { for _, pid := range pids { ds, _, err := metal.ProjectVirtualNetworks.List(pid, nil) if err != nil { - log.Printf("Error listing vlans to sweep: %s", err) + log.Printf("Error listing vlan to sweep: %s", err) continue } for _, d := range ds.VirtualNetworks { diff --git a/internal/sweep/sweep_test.go b/internal/sweep/sweep_test.go index 778330de6..57f9fbb64 100644 --- a/internal/sweep/sweep_test.go +++ b/internal/sweep/sweep_test.go @@ -1,7 +1,7 @@ package sweep_test import ( - "github.com/equinix/terraform-provider-equinix/internal/resources/metal/vlans" + "github.com/equinix/terraform-provider-equinix/internal/resources/metal/vlan" "testing" "github.com/equinix/terraform-provider-equinix/internal/resources/metal/device" @@ -28,6 +28,6 @@ func addTestSweepers() { ssh_key.AddTestSweeper() user_api_key.AddTestSweeper() virtual_circuit.AddTestSweeper() - vlans.AddTestSweeper() + vlan.AddTestSweeper() vrf.AddTestSweeper() }