hashicorp / terraform-provider-dyn

Terraform Dyn provider. Please note: This Terraform provider is archived per our provider archiving process: https://terraform.io/docs/internals/archiving.html
https://registry.terraform.io
Mozilla Public License 2.0
7 stars 16 forks source link

Session handling: create and teardown a new client session for every action #34

Open phamann opened 5 years ago

phamann commented 5 years ago

TL;DR

Changes how the API session is handled via the client. Currently the provider creates a single session per run but never logs out / tears-down, thus leaving orphaned sessions within Dynect which leads to side-effects.

Why?

The provider currently creates a new client instance and thus API Session within the ConfigureFunc which is ran once per run. However, as the Terraform Provider API doesn't expose a configure clean-up/teardown-down function on the interface (see this issue for further detail: https://github.com/hashicorp/terraform/issues/6258) the provider never calls the Logout() method on the client - which means the API session gets left orphaned.

Historically this (orphaned sessions) hasn't been an issue, but Dyn have recently tightened their API security whilst increasing API timeouts. Therefore, when you try to use the provider multiple times with the same credentials across machines or IP addresses it leads to a API session error being returned:

login: IP address does not match current session

We now observe this consistently in CI environments, where one build stage creates a Dyn session via the provider and then the next stage - and thus machine - attempts to use the provider we get the error returned from the API. After much debugging and discussion with Dyn support we have narrowed now the issue to be the session handling within the provider.

How?

As the Terraform Provider API doesn't expose a teardown method, this PR refactors the provider to create a new client session and then logout for each resource action. Very nasty hack and not DRY at all; but it solves the issue.

Thoughts?

I don't like how this is implemented (I'm also pretty new to Go), but it does the trick. We (Fastly) are using this fork successfully in CI but would like a real fix upstream. However please feel free to reject or suggest alternatives until the wider issue in https://github.com/hashicorp/terraform/issues/6258 is resolved.