Skip to content

refactor: migrate metal_vlan to equinix-sdk-go #782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ctreatma wants to merge 7 commits into from
Closed

refactor: migrate metal_vlan to equinix-sdk-go #782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
61b701a
convert VLAN resource/datasource to equinix-sdk-go
ctreatma Jul 1, 2024
e302aef
remove non-functional code
ctreatma Sep 25, 2024
ed8d7da
remove temporary packngo alias
ctreatma Sep 26, 2024
c2109a0
remove usage of FriendlyError because it only works for packngo
ctreatma Sep 26, 2024
2396803
fix calls to ignore certain HTTP errors post-rebase
ctreatma Sep 30, 2024
26d034c
different workaround for loading facility-based VLANs
ctreatma Oct 4, 2024
c6256f4
remove packngo from tests, too
ctreatma Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 21 additions & 29 deletions internal/resources/metal/vlan/datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import (
"fmt"
"strings"

equinix_errors "github.com/equinix/terraform-provider-equinix/internal/errors"
"github.com/equinix/equinix-sdk-go/services/metalv1"
"github.com/equinix/terraform-provider-equinix/internal/framework"

"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
"github.com/packethost/packngo"
)

func NewDataSource() datasource.DataSource {
Expand Down Expand Up @@ -45,8 +44,7 @@ func (r *DataSource) Read(
req datasource.ReadRequest,
resp *datasource.ReadResponse,
) {
r.Meta.AddFwModuleToMetalUserAgent(ctx, req.ProviderMeta)
client := r.Meta.Metal
client := r.Meta.NewMetalClientForFramework(ctx, req.ProviderMeta)

var data DataSourceModel
resp.Diagnostics.Append(req.Config.Get(ctx, &data)...)
Expand All @@ -57,49 +55,42 @@ func (r *DataSource) Read(
if data.VlanID.IsNull() &&
(data.Vxlan.IsNull() && data.ProjectID.IsNull() && data.Metro.IsNull() && data.Facility.IsNull()) {
resp.Diagnostics.AddError("Error fetching Vlan datasource",
equinix_errors.
FriendlyError(fmt.Errorf("You must set either vlan_id or a combination of vxlan, project_id, and, metro or facility")).
Error())
"You must set either vlan_id or a combination of vxlan, project_id, and, metro or facility")
return
}

var vlan *packngo.VirtualNetwork
var vlan *metalv1.VirtualNetwork

if !data.VlanID.IsNull() {
var err error
vlan, _, err = client.ProjectVirtualNetworks.Get(
vlan, _, err = client.VLANsApi.GetVirtualNetwork(
ctx,
data.VlanID.ValueString(),
&packngo.GetOptions{Includes: []string{"assigned_to"}},
)
).Include([]string{"assigned_to"}).Execute()
if err != nil {
resp.Diagnostics.AddError("Error fetching Vlan using vlanId", equinix_errors.FriendlyError(err).Error())
resp.Diagnostics.AddError("Error fetching Vlan using vlanId", err.Error())
return
}

} else {
vlans, _, err := client.ProjectVirtualNetworks.List(
vlans, _, err := client.VLANsApi.FindVirtualNetworks(
ctx,
data.ProjectID.ValueString(),
&packngo.GetOptions{Includes: []string{"assigned_to"}},
)
).Include([]string{"assigned_to"}).Execute()
if err != nil {
resp.Diagnostics.AddError("Error fetching vlan list for projectId",
equinix_errors.FriendlyError(err).Error())
err.Error())
return
}

vlan, err = MatchingVlan(vlans.VirtualNetworks, int(data.Vxlan.ValueInt64()), data.ProjectID.ValueString(),
data.Facility.ValueString(), data.Metro.ValueString())
if err != nil {
resp.Diagnostics.AddError("Error expected vlan not found", equinix_errors.FriendlyError(err).Error())
resp.Diagnostics.AddError("Error expected vlan not found", err.Error())
return
}
}

assignedDevices := []string{}
for _, d := range vlan.Instances {
assignedDevices = append(assignedDevices, d.ID)
}

// Set state to fully populated data
resp.Diagnostics.Append(data.parse(vlan)...)
if resp.Diagnostics.HasError() {
Expand All @@ -110,26 +101,27 @@ func (r *DataSource) Read(
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
}

func MatchingVlan(vlans []packngo.VirtualNetwork, vxlan int, projectID, facility, metro string) (*packngo.VirtualNetwork, error) {
matches := []packngo.VirtualNetwork{}
func MatchingVlan(vlans []metalv1.VirtualNetwork, vxlan int, projectID, facility, metro string) (*metalv1.VirtualNetwork, error) {
matches := []metalv1.VirtualNetwork{}
for _, v := range vlans {
if vxlan != 0 && v.VXLAN != vxlan {
if vxlan != 0 && int(v.GetVxlan()) != vxlan {
continue
}
if facility != "" && !strings.EqualFold(v.FacilityCode, facility) {
facility_code, ok := v.AdditionalProperties["facility_code"].(string)
if ok && facility != "" && !strings.EqualFold(facility_code, facility) {
continue
}
if metro != "" && !strings.EqualFold(v.MetroCode, metro) {
if metro != "" && !strings.EqualFold(v.GetMetroCode(), metro) {
continue
}
matches = append(matches, v)
}
if len(matches) > 1 {
return nil, equinix_errors.FriendlyError(fmt.Errorf("Project %s has more than one matching VLAN", projectID))
return nil, fmt.Errorf("Project %s has more than one matching VLAN", projectID)
}

if len(matches) == 0 {
return nil, equinix_errors.FriendlyError(fmt.Errorf("Project %s does not have matching VLANs for vlan [%d] and metro [%s]", projectID, vxlan, metro))
return nil, fmt.Errorf("Project %s does not have matching VLANs for vlan [%d] and metro [%s]", projectID, vxlan, metro)
}
return &matches[0], nil
}
33 changes: 17 additions & 16 deletions internal/resources/metal/vlan/datasource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package vlan_test

import (
"context"
"fmt"
"reflect"
"testing"
Expand All @@ -9,11 +10,11 @@ import (
"github.com/equinix/terraform-provider-equinix/internal/config"
"github.com/equinix/terraform-provider-equinix/internal/resources/metal/vlan"

"github.com/equinix/equinix-sdk-go/services/metalv1"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/plancheck"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"github.com/packethost/packngo"
)

func TestAccDataSourceMetalVlan_byVxlanMetro(t *testing.T) {
Expand Down Expand Up @@ -181,7 +182,7 @@ data "equinix_metal_vlan" "dsvlan" {

func TestMetalVlan_matchingVlan(t *testing.T) {
type args struct {
vlans []packngo.VirtualNetwork
vlans []metalv1.VirtualNetwork
vxlan int
projectID string
facility string
Expand All @@ -190,52 +191,52 @@ func TestMetalVlan_matchingVlan(t *testing.T) {
tests := []struct {
name string
args args
want *packngo.VirtualNetwork
want *metalv1.VirtualNetwork
wantErr bool
}{
{
name: "MatchingVLAN",
args: args{
vlans: []packngo.VirtualNetwork{{VXLAN: 123}},
vlans: []metalv1.VirtualNetwork{{Vxlan: metalv1.PtrInt32(123)}},
vxlan: 123,
projectID: "",
facility: "",
metro: "",
},
want: &packngo.VirtualNetwork{VXLAN: 123},
want: &metalv1.VirtualNetwork{Vxlan: metalv1.PtrInt32(123)},
wantErr: false,
},
{
name: "MatchingFac",
args: args{
vlans: []packngo.VirtualNetwork{{FacilityCode: "fac"}},
vlans: []metalv1.VirtualNetwork{{AdditionalProperties: map[string]interface{}{"facility_code": "fac"}}},
facility: "fac",
},
want: &packngo.VirtualNetwork{FacilityCode: "fac"},
want: &metalv1.VirtualNetwork{AdditionalProperties: map[string]interface{}{"facility_code": "fac"}},
wantErr: false,
},
{
name: "MatchingMet",
args: args{
vlans: []packngo.VirtualNetwork{{MetroCode: "met"}},
vlans: []metalv1.VirtualNetwork{{MetroCode: metalv1.PtrString("met")}},
metro: "met",
},
want: &packngo.VirtualNetwork{MetroCode: "met"},
want: &metalv1.VirtualNetwork{MetroCode: metalv1.PtrString("met")},
wantErr: false,
},
{
name: "SecondMatch",
args: args{
vlans: []packngo.VirtualNetwork{{FacilityCode: "fac"}, {MetroCode: "met"}},
vlans: []metalv1.VirtualNetwork{{AdditionalProperties: map[string]interface{}{"facility_code": "fac"}}, {MetroCode: metalv1.PtrString("met")}},
metro: "met",
},
want: &packngo.VirtualNetwork{MetroCode: "met"},
want: &metalv1.VirtualNetwork{MetroCode: metalv1.PtrString("met")},
wantErr: false,
},
{
name: "TwoMatches",
args: args{
vlans: []packngo.VirtualNetwork{{MetroCode: "met"}, {MetroCode: "met"}},
vlans: []metalv1.VirtualNetwork{{MetroCode: metalv1.PtrString("met")}, {MetroCode: metalv1.PtrString("met")}},
metro: "met",
},
want: nil,
Expand All @@ -244,10 +245,10 @@ func TestMetalVlan_matchingVlan(t *testing.T) {
{
name: "ComplexMatch",
args: args{
vlans: []packngo.VirtualNetwork{{VXLAN: 987, FacilityCode: "fac", MetroCode: "skip"}, {VXLAN: 123, FacilityCode: "fac", MetroCode: "met"}, {VXLAN: 456, FacilityCode: "fac", MetroCode: "nope"}},
vlans: []metalv1.VirtualNetwork{{Vxlan: metalv1.PtrInt32(987), AdditionalProperties: map[string]interface{}{"facility_code": "fac"}, MetroCode: metalv1.PtrString("skip")}, {Vxlan: metalv1.PtrInt32(123), AdditionalProperties: map[string]interface{}{"facility_code": "fac"}, MetroCode: metalv1.PtrString("met")}, {Vxlan: metalv1.PtrInt32(456), AdditionalProperties: map[string]interface{}{"facility_code": "fac"}, MetroCode: metalv1.PtrString("nope")}},
Copy link
Member

@displague displague Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: breaking this up with newlines may help readers without line-wrapping views

metro: "met",
},
want: &packngo.VirtualNetwork{VXLAN: 123, FacilityCode: "fac", MetroCode: "met"},
want: &metalv1.VirtualNetwork{Vxlan: metalv1.PtrInt32(123), AdditionalProperties: map[string]interface{}{"facility_code": "fac"}, MetroCode: metalv1.PtrString("met")},
wantErr: false,
},
{
Expand Down Expand Up @@ -278,13 +279,13 @@ func TestMetalVlan_matchingVlan(t *testing.T) {
}

func testAccMetalDatasourceVlanCheckDestroyed(s *terraform.State) error {
client := acceptance.TestAccProvider.Meta().(*config.Config).Metal
client := acceptance.TestAccProvider.Meta().(*config.Config).NewMetalClientForTesting()

for _, rs := range s.RootModule().Resources {
if rs.Type != "equinix_metal_vlan" {
continue
}
if _, _, err := client.ProjectVirtualNetworks.Get(rs.Primary.ID, nil); err == nil {
if _, _, err := client.VLANsApi.GetVirtualNetwork(context.Background(), rs.Primary.ID).Execute(); err == nil {
return fmt.Errorf("Data source VLAN still exists")
}
}
Expand Down
50 changes: 25 additions & 25 deletions internal/resources/metal/vlan/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import (
"fmt"
"strings"

"github.com/equinix/equinix-sdk-go/services/metalv1"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/packethost/packngo"
)

type DataSourceModel struct {
Expand All @@ -21,29 +21,29 @@ type DataSourceModel struct {
AssignedDevicesIds types.List `tfsdk:"assigned_devices_ids"`
}

func (m *DataSourceModel) parse(vlan *packngo.VirtualNetwork) (d diag.Diagnostics) {
m.ID = types.StringValue(vlan.ID)
m.VlanID = types.StringValue(vlan.ID)
m.Vxlan = types.Int64Value(int64(vlan.VXLAN))
func (m *DataSourceModel) parse(vlan *metalv1.VirtualNetwork) (d diag.Diagnostics) {
m.ID = types.StringValue(vlan.GetId())
m.VlanID = types.StringValue(vlan.GetId())
m.Vxlan = types.Int64Value(int64(vlan.GetVxlan()))
m.Facility = types.StringValue("")

if vlan.Description != "" {
m.Description = types.StringValue(vlan.Description)
if vlan.GetDescription() != "" {
m.Description = types.StringValue(vlan.GetDescription())
Copy link
Member

@displague displague Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty or not, when would we not want the model description to match the upstream description?

Copy link
Contributor Author

@ctreatma ctreatma Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#642

Copy link
Contributor Author

@ctreatma ctreatma Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be other ways to address the bug fixed by that previous PR; I forget what else, if anything, I tried at that time, but I think I was aiming to maintain backwards compatibility. The effect of this if block is that, if the description is null or "" in the response, it is null in the model. I don't recall if the API meaningfully differentiates between a null VLAN description and an empty VLAN description.

}

if vlan.Project.ID != "" {
m.ProjectID = types.StringValue(vlan.Project.ID)
if vlan.AssignedTo != nil {
m.ProjectID = types.StringValue(vlan.AssignedTo.GetId())
}

if vlan.Facility != nil {
m.Facility = types.StringValue(strings.ToLower(vlan.Facility.Code))
m.Metro = types.StringValue(strings.ToLower(vlan.Facility.Metro.Code))
facility_code := vlan.Facility.AdditionalProperties["facility_code"].(string)
m.Facility = types.StringValue(strings.ToLower(facility_code))
}

if vlan.Metro != nil {
if m.Metro.IsNull() {
m.Metro = types.StringValue(vlan.Metro.Code)
} else if !strings.EqualFold(m.Metro.ValueString(), vlan.Metro.Code) {
m.Metro = types.StringValue(vlan.Metro.GetCode())
} else if !strings.EqualFold(m.Metro.ValueString(), vlan.Metro.GetCode()) {
d.AddWarning(
"unexpected value for metro",
fmt.Sprintf("expected vlan %v to have metro %v, but metro was %v",
Expand All @@ -53,7 +53,7 @@ func (m *DataSourceModel) parse(vlan *packngo.VirtualNetwork) (d diag.Diagnostic

deviceIds := make([]types.String, 0, len(vlan.Instances))
for _, device := range vlan.Instances {
deviceIds = append(deviceIds, types.StringValue(device.ID))
deviceIds = append(deviceIds, types.StringValue(device.GetId()))
}

return m.AssignedDevicesIds.ElementsAs(context.Background(), &deviceIds, false)
Expand All @@ -68,28 +68,28 @@ type ResourceModel struct {
Description types.String `tfsdk:"description"`
}

func (m *ResourceModel) parse(vlan *packngo.VirtualNetwork) (d diag.Diagnostics) {
m.ID = types.StringValue(vlan.ID)
m.Vxlan = types.Int64Value(int64(vlan.VXLAN))
func (m *ResourceModel) parse(vlan *metalv1.VirtualNetwork) (d diag.Diagnostics) {
m.ID = types.StringValue(vlan.GetId())
m.Vxlan = types.Int64Value(int64(vlan.GetVxlan()))
m.Facility = types.StringValue("")

if vlan.Description != "" {
m.Description = types.StringValue(vlan.Description)
if vlan.GetDescription() != "" {
Copy link
Member

@displague displague Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as above, why wouldn't we set description unconditionally?

m.Description = types.StringValue(vlan.GetDescription())
}

if vlan.Project.ID != "" {
m.ProjectID = types.StringValue(vlan.Project.ID)
if vlan.AssignedTo != nil {
m.ProjectID = types.StringValue(vlan.AssignedTo.GetId())
}

if vlan.Facility != nil {
m.Facility = types.StringValue(strings.ToLower(vlan.Facility.Code))
m.Metro = types.StringValue(strings.ToLower(vlan.Facility.Metro.Code))
facility_code := vlan.Facility.AdditionalProperties["facility_code"].(string)
m.Facility = types.StringValue(strings.ToLower(facility_code))
}

if vlan.Metro != nil {
if m.Metro.IsNull() {
m.Metro = types.StringValue(vlan.Metro.Code)
} else if !strings.EqualFold(m.Metro.ValueString(), vlan.Metro.Code) {
m.Metro = types.StringValue(vlan.Metro.GetCode())
} else if !strings.EqualFold(m.Metro.ValueString(), vlan.Metro.GetCode()) {
d.AddError(
"unexpected value for metro",
fmt.Sprintf("expected vlan %v to have metro %v, but metro was %v",
Expand Down
Loading