thousandeyes / terraform-provider-thousandeyes

ThousandEyes Terraform Provider
Apache License 2.0
21 stars 27 forks source link

fix: fixes agent_to_server ICMP tests which should not require a port #104

Closed pedro-te closed 2 years ago

pedro-te commented 2 years ago

Terraform was always passing a port on agent_to_agent and agent_to_server tests. This is fine for agent_to_agent as that test type does indeed require a port to be passed, but not for agent_to_server since ICMP based tests mustn't contain a port and TCP tests do not require the port to be passed (it'll default to 80 on ThousandEyes).

There are many approaches possible to fix this, but my constraints were:

These constraints meant that the port had to be optional and not have default value for the agent_to_server resource. This meant that the schema definition for the port had to be different for agent_to_agent and agent_to_server, so I created this schema override logic.

Additionally, since port is now optional and has no default for agent_to_server, this means that agent_to_server TCP based resources/tests are now created using ThousandEyes' default port, which is 80. This also means that terraform will try to change the port back to 0 on every plan if using the ThousandEyes' default. To address this I used the DiffSuppressFunc, which checks if the new value of the port is "0".

Let me hear your thoughts!

go vet ./... && echo && go test ./...

?       github.com/thousandeyes/terraform-provider-thousandeyes [no test files]
ok      github.com/thousandeyes/terraform-provider-thousandeyes/thousandeyes    0.427s