search

equinix / terraform-provider-metal

DEPRECATED Equinix Metal standalone TF provider. Use the unified Equinix Terraform Provider.
https://registry.terraform.io/providers/equinix/equinix/latest/docs/guides/migration_guide_equinix_metal
Mozilla Public License 2.0
14 stars 11 forks source link

fix 403 and 404 detection #192

Closed displague closed 3 years ago

displague commented 3 years ago

Revisiting #44

I received │ Error: You are not authorized to view this resource while terraform destroying a metal_vlan. I also received Error: Error deleting IP reservation block 4f5fa092-099d-497e-a514-5f25e24b098a: DELETE https://api.equinix.com/metal/v1/ips/4f5fa092-099d-497e-a514-5f25e24b098a: 404 Not found in https://github.com/equinix/terraform-metal-vsphere/pull/40/checks

While any metal_vlan is being deleted an attempt is made to unassign all device ports. The request returned a 403 because the ports were unavailable because the device was deleted first.

403 and 404 should be handled as success during deletions.

The problem addressed in this PR is that ignoreResponseErrors was returning the first error if it was not the first ignored error.

Additionally, the response objects in the ignoring functions can be safely sent as null. When they are null the error will be examined, which may contain a response.

This error detection has become confusing, in part because the response and error are typically both sent, with the error including the response within it. It's also no longer obvious to me what value there is in "IsAPIError".

displague commented 3 years ago

It might be nice to add tests that try to E2E delete a bad UUID (to simulate a 404). I haven't seen Terraform tests that set artificial state, but presumably we should be able to create a metal_vlan and assert that a metal_port_vlan_attachment exists with state that includes the actual vlan id and a fake port 123. We would then attempt to delete that metal_port_vlan_attachment.

displague commented 3 years ago

When any packngo request is sent we get both the HTTP response and an error.

Packngo will decorate the API and any HTTP errors as an ErrorResponse. In doing so, a reference to the HTTP response is bundled with the error.

This Terraform provider will also decorate errors (friendlyError). If the error was a packngo.ErrorResponse a metal.ErrorResponse will be returned. The ErrorResponse adds the StatusCode (already present in packngo.ErrorResponse).

This Terraform provider attempts to dig deeper into some errors to apply the correct Terraform behavior for acceptable errors (Forbidden NotFound), either of which can indicate a successful delete.

When the Terraform provider digs into errors, the error may not always be a packngo.ErrorResponse. The error may be a metal.ErrorResponse. This is because in some call patterns, presumably helpers, friendlyError() may have already been applied.

metal.ErrorResponse has an IsAPIError property which indicates whether or not the error was an API error or an error from a fronting service (loadbalancer, cache). This is important to capture because a 403/404 from the fronting service could indicate an API delivery problem rather than indicating that EM resources have been removed. Note: This property is used once.

Thoughts: