jweaver-skytap
commented
2 years ago Turns out this is sort of half right. outbound_traffic is a valid option, but only on the V2 API.
Environments are created with the function here
The function for creating environments is posting to the V1 Skytap API: environmentLegacyBasePath
The function for updating status is using the V2 Skytap API. This may be a cause of other issues that have been reported with this provider.
The V1 Skytap API uses disable_internet, which is TRUE when Internet access is disabled, and FALSE when Internet access is enabled. On the V2 API, we use outbound_traffic, and it is TRUE when Internet access is disabled, and FALSE when Internet access is enabled. The documentation for the Skytap Terraform provider has this documented backwards. I will work with the team to get the documentation corrected.
This can be fixed by adding disable_internet to the Environment structure, and ensuring its set to the same value as outbound_traffic, and also ensuring that if either variable is changed, that change is reflected in both, and then updating the CreateEnvironmentRequest structure to use the V1 API semantics.
There is a downside: fixing this would mean any Terraform plan with this outbound_traffic set to TRUE would find that their environments have Internet access disabled on the next tf apply when run with an updated Skytap provider.
| Field Name | Type | Access | Description | API Version |
|---|---|---|---|---|
| disable_internet | Boolean | rw | If true, outbound internet is disabled for VMs in this environment. Note that VMs with public IPs or published services will still be exposed to the Internet. | V1 |
| outbound_traffic | Boolean | rw | Indicates whether networks in the environment can send outbound traffic. | V2 |
Skytap Terraform provider is using 'outbound_traffic' when generating the JSON body used with working with Skytap environments. Our API documentation does not have this element documented anywhere, and neither do environment details JSON responses, instead we use 'disable_internet'.
Skytap API doc ref: https://help.skytap.com/API_Documentation.html#Configur
Incorrect code: https://github.com/skytap/terraform-provider-skytap/blob/e81602bd18d73af86687099e90a2234ef17a0556/vendor/github.com/skytap/skytap-sdk-go/skytap/environment.go#L83
Changing outbound_traffic in the code would probably fix this, but outbound_traffic setting is described as having an opposite effect to the actual Skytap setting disable_internet, so simply correcting that here could have unintentional consequences. I think the best way to fix it would be adding a new setting, disable_internet, and updating the environment handling code to populate the API calls to Skytap with the specified disable_internet setting, or the inverse value of what's specified if outbound_traffic is specified so that it has the intended effect.
Our Terraform reference documentation says: