From e0074b896a4a5ab24d3ad5c1aa31cf9291a66265 Mon Sep 17 00:00:00 2001 From: Oscar Cobles Date: Thu, 28 Jul 2022 23:13:06 +0200 Subject: [PATCH 1/4] check for forbidden in metal device read function and mock test --- equinix/resource_metal_device.go | 5 +- equinix/resource_metal_device_acc_test.go | 190 ++++++++++++++++++++++ 2 files changed, 193 insertions(+), 2 deletions(-) diff --git a/equinix/resource_metal_device.go b/equinix/resource_metal_device.go index 1cc85d884..5a79a543c 100644 --- a/equinix/resource_metal_device.go +++ b/equinix/resource_metal_device.go @@ -584,8 +584,9 @@ func resourceMetalDeviceRead(d *schema.ResourceData, meta interface{}) error { err = friendlyError(err) // If the device somehow already destroyed, mark as successfully gone. - if isNotFound(err) { - log.Printf("[WARN] Device (%s) not found, removing from state", d.Id()) + // Checking for IsNewResource prevents resource import from failing silently + if !d.IsNewResource() && (isNotFound(err) || isForbidden(err)) { + log.Printf("[WARN] Device (%s) not found or in failed status, removing from state", d.Id()) d.SetId("") return nil } diff --git a/equinix/resource_metal_device_acc_test.go b/equinix/resource_metal_device_acc_test.go index 46df8dee0..593d5e156 100644 --- a/equinix/resource_metal_device_acc_test.go +++ b/equinix/resource_metal_device_acc_test.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "net" + "net/http" "regexp" "strings" "testing" @@ -11,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/packethost/packngo" ) @@ -902,3 +904,191 @@ resource "equinix_metal_device" "test_ipxe_missing" { ] } }` + +type mockDeviceService struct { + GetFn func(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) + CreateFn func(device *packngo.DeviceCreateRequest) (*packngo.Device, *packngo.Response, error) + DeleteFn func(deviceID string, fdv bool) (*packngo.Response, error) +} + +func (m *mockDeviceService) Get(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) { + return m.GetFn(deviceID, opts) +} + +func (m *mockDeviceService) Create(device *packngo.DeviceCreateRequest) (*packngo.Device, *packngo.Response, error){ + return nil, nil, mockFuncNotImplemented("Create") +} + +func (m *mockDeviceService) Delete(string, bool) (*packngo.Response, error) { + return nil, mockFuncNotImplemented("Delete") +} + +func (m *mockDeviceService) List(string, *packngo.ListOptions) ([]packngo.Device, *packngo.Response, error) { + return nil, nil, mockFuncNotImplemented("List") +} + +func (m *mockDeviceService) Update(string, *packngo.DeviceUpdateRequest) (*packngo.Device, *packngo.Response, error) { + return nil, nil, mockFuncNotImplemented("Update") +} + +func (m *mockDeviceService) Reboot(string) (*packngo.Response, error) { + return nil, mockFuncNotImplemented("Reboot") +} + +func (m *mockDeviceService) Reinstall(string, *packngo.DeviceReinstallFields) (*packngo.Response, error) { + return nil, mockFuncNotImplemented("Reinstall") +} + +func (m *mockDeviceService) PowerOff(string) (*packngo.Response, error) { + return nil, mockFuncNotImplemented("PowerOff") +} + +func (m *mockDeviceService) PowerOn(string) (*packngo.Response, error) { + return nil, mockFuncNotImplemented("PowerOn") +} + +func (m *mockDeviceService) Lock(string) (*packngo.Response, error) { + return nil, mockFuncNotImplemented("Lock") +} + +func (m *mockDeviceService) Unlock(string) (*packngo.Response, error) { + return nil, mockFuncNotImplemented("Unlock") +} + +func (m *mockDeviceService) ListBGPSessions(string, *packngo.ListOptions) ([]packngo.BGPSession, *packngo.Response, error) { + return nil, nil, mockFuncNotImplemented("ListBGPSessions") +} + +func (m *mockDeviceService) ListBGPNeighbors(string, *packngo.ListOptions) ([]packngo.BGPNeighbor, *packngo.Response, error) { + return nil, nil, mockFuncNotImplemented("ListBGPNeighbors") +} + +func (m *mockDeviceService) ListEvents(string, *packngo.ListOptions) ([]packngo.Event, *packngo.Response, error) { + return nil, nil, mockFuncNotImplemented("ListEvents") +} + +func (m *mockDeviceService) GetBandwidth(string, *packngo.BandwidthOpts) (*packngo.BandwidthIO, *packngo.Response, error) { + return nil, nil, mockFuncNotImplemented("GetBandwidth") +} + +func mockFuncNotImplemented(f string) error { + return fmt.Errorf("mockDeviceService %s function not yet implemented", f) +} + +var _ packngo.DeviceService = (*mockDeviceService)(nil) + +func TestAccMetalDevice_readErrorHandling(t *testing.T) { + type args struct { + newResource bool + meta *Config + } + + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "forbiddenAfterProvision", + args: args{ + newResource: false, + meta: &Config{ + metal: &packngo.Client{ + Devices: &mockDeviceService{ + GetFn: func(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) { + httpResp := &http.Response{Status: "403 Forbidden", StatusCode: 403} + return nil, &packngo.Response{Response: httpResp}, &packngo.ErrorResponse{Response: httpResp} + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "notFoundAfterProvision", + args: args{ + newResource: false, + meta: &Config{ + metal: &packngo.Client{ + Devices: &mockDeviceService{ + GetFn: func(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) { + httpResp := &http.Response{ + Status: "404 NotFound", + StatusCode: 404, + Header: http.Header{"Content-Type": []string{"application/json"}, "X-Request-Id": []string{"12345"}}, + } + return nil, &packngo.Response{Response: httpResp}, &packngo.ErrorResponse{Response: httpResp} + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "forbiddenWaitForActiveDeviceProvision", + args: args{ + newResource: true, + meta: &Config{ + metal: &packngo.Client{ + Devices: &mockDeviceService{ + GetFn: func(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) { + httpResp := &http.Response{Status: "403 Forbidden", StatusCode: 403} + return nil, &packngo.Response{Response: httpResp}, &packngo.ErrorResponse{Response: httpResp} + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "notFoundImport", + args: args{ + newResource: true, + meta: &Config{ + metal: &packngo.Client{ + Devices: &mockDeviceService{ + GetFn: func(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) { + httpResp := &http.Response{Status: "404 NotFound", StatusCode: 404} + return nil, &packngo.Response{Response: httpResp}, &packngo.ErrorResponse{Response: httpResp} + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "errorProvision", + args: args{ + newResource: true, + meta: &Config{ + metal: &packngo.Client{ + Devices: &mockDeviceService{ + GetFn: func(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) { + httpResp := &http.Response{Status: "400 BadRequest", StatusCode: 400} + return nil, &packngo.Response{Response: httpResp}, &packngo.ErrorResponse{Response: httpResp} + }, + }, + }, + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var d *schema.ResourceData + d = new(schema.ResourceData) + if tt.args.newResource { + d.MarkNewResource() + } + if err := resourceMetalDeviceRead(d, tt.args.meta); (err != nil) != tt.wantErr { + t.Errorf("resourceMetalDeviceRead() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From 305d2683bc1e0031f0ed049f475046a4c7892a49 Mon Sep 17 00:00:00 2001 From: Oscar Cobles Date: Fri, 29 Jul 2022 09:34:59 +0200 Subject: [PATCH 2/4] fixup! check for forbidden in metal device read function and mock test --- equinix/resource_metal_device_acc_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/equinix/resource_metal_device_acc_test.go b/equinix/resource_metal_device_acc_test.go index 593d5e156..3f987351a 100644 --- a/equinix/resource_metal_device_acc_test.go +++ b/equinix/resource_metal_device_acc_test.go @@ -907,8 +907,6 @@ resource "equinix_metal_device" "test_ipxe_missing" { type mockDeviceService struct { GetFn func(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) - CreateFn func(device *packngo.DeviceCreateRequest) (*packngo.Device, *packngo.Response, error) - DeleteFn func(deviceID string, fdv bool) (*packngo.Response, error) } func (m *mockDeviceService) Get(deviceID string, opts *packngo.GetOptions) (*packngo.Device, *packngo.Response, error) { From d87e51e1b00097dad362a94e46841485c1cfb688 Mon Sep 17 00:00:00 2001 From: Oscar Cobles Date: Fri, 29 Jul 2022 14:26:51 +0200 Subject: [PATCH 3/4] fixup! check for forbidden in metal device read function and mock test --- equinix/resource_metal_device_acc_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/equinix/resource_metal_device_acc_test.go b/equinix/resource_metal_device_acc_test.go index 3f987351a..0f501ca8e 100644 --- a/equinix/resource_metal_device_acc_test.go +++ b/equinix/resource_metal_device_acc_test.go @@ -1042,7 +1042,7 @@ func TestAccMetalDevice_readErrorHandling(t *testing.T) { wantErr: true, }, { - name: "notFoundImport", + name: "notFoundProvision", args: args{ newResource: true, meta: &Config{ @@ -1056,7 +1056,7 @@ func TestAccMetalDevice_readErrorHandling(t *testing.T) { }, }, }, - wantErr: true, + wantErr: false, }, { name: "errorProvision", From 5106a84bc909ba2e884e75c700b0997ef3401c75 Mon Sep 17 00:00:00 2001 From: Oscar Cobles Date: Mon, 1 Aug 2022 10:35:48 +0200 Subject: [PATCH 4/4] fix failing test, api error should be returned in create operations --- equinix/resource_metal_device.go | 3 ++- equinix/resource_metal_device_acc_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/equinix/resource_metal_device.go b/equinix/resource_metal_device.go index 5a79a543c..07f23c3c9 100644 --- a/equinix/resource_metal_device.go +++ b/equinix/resource_metal_device.go @@ -584,7 +584,8 @@ func resourceMetalDeviceRead(d *schema.ResourceData, meta interface{}) error { err = friendlyError(err) // If the device somehow already destroyed, mark as successfully gone. - // Checking for IsNewResource prevents resource import from failing silently + // Checking d.IsNewResource prevents the creation of a resource from failing + // silently. Note d.IsNewResource is false in resource import operations. if !d.IsNewResource() && (isNotFound(err) || isForbidden(err)) { log.Printf("[WARN] Device (%s) not found or in failed status, removing from state", d.Id()) d.SetId("") diff --git a/equinix/resource_metal_device_acc_test.go b/equinix/resource_metal_device_acc_test.go index 0f501ca8e..dd7502007 100644 --- a/equinix/resource_metal_device_acc_test.go +++ b/equinix/resource_metal_device_acc_test.go @@ -1056,7 +1056,7 @@ func TestAccMetalDevice_readErrorHandling(t *testing.T) { }, }, }, - wantErr: false, + wantErr: true, }, { name: "errorProvision",