provider/aws: Setting static_routes_only on import of vpn_connection #9802
Conversation
| Type: schema.TypeBool, | ||
| Required: true, | ||
| Optional: true, |
There was a problem hiding this comment.
The reason this changes from Required to Optional is that if we d.Get("") on a bool and it doesn't exist then it falls back to the default value - which is false
There was a problem hiding this comment.
Required to Optional is that if we d.Get("") on a bool and it doesn't exist
Why wouldn't it exist if it's a Required attribute?
Can you elaborate why this change is needed? It seems that the below change:
+ } else {
+ //If there no Options on the connection then we do not support *static_routes*
+ d.Set("static_routes_only", false)
+ }
is what is needed here. #9110 states that on import it wasn't being set; with the above, it is. What am I missing such that we now make a once Required attribute to be optional and computed?
There was a problem hiding this comment.
without the Optional, addition of Computed means that we get the following error:
Errors: []string{"provider.aws: Internal validation of the provider failed! This is always a bug\nwith the provider itself, and not a user issue. Please report\nthis bug:\n\n1 error(s) occurred:\n\n* resource aws_vpn_connection: static_routes_only: Cannot be both Required and Computed"}
There was a problem hiding this comment.
We cleared up my confusion in chat
| @@ -300,10 +302,14 @@ func resourceAwsVpnConnectionRead(d *schema.ResourceData, meta interface{}) erro | |||
| d.Set("type", vpnConnection.Type) | |||
| d.Set("tags", tagsToMap(vpnConnection.Tags)) | |||
|
|
|||
| log.Printf("[INFO] VPN Connection Options: %s", spew.Sdump(vpnConnection.Options)) | |||
| Type: schema.TypeBool, | ||
| Required: true, | ||
| Optional: true, |
There was a problem hiding this comment.
Required to Optional is that if we d.Get("") on a bool and it doesn't exist
Why wouldn't it exist if it's a Required attribute?
Can you elaborate why this change is needed? It seems that the below change:
+ } else {
+ //If there no Options on the connection then we do not support *static_routes*
+ d.Set("static_routes_only", false)
+ }
is what is needed here. #9110 states that on import it wasn't being set; with the above, it is. What am I missing such that we now make a once Required attribute to be optional and computed?
fixes #9110 An error was found where, static_routes_only was not set on a vpn connection import. This commit introduces setting the static_routes_only to false when no Options are found. This follows the AWS convention as follows: ``` - options (structure) Indicates whether the VPN connection requires static routes. If you are creating a VPN connection for a device that does not support BGP, you must specify true . Default: false ``` So we take it that `static_options_only` is false by default ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVpnConnection_' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2016/11/02 10:38:18 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVpnConnection_ -timeout 120m === RUN TestAccAWSVpnConnection_importBasic --- PASS: TestAccAWSVpnConnection_importBasic (178.29s) === RUN TestAccAWSVpnConnection_basic --- PASS: TestAccAWSVpnConnection_basic (336.81s) === RUN TestAccAWSVpnConnection_withoutStaticRoutes --- PASS: TestAccAWSVpnConnection_withoutStaticRoutes (195.45s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 710.572s ```
stack72
force-pushed
the
b-aws-import-vpn-connection
branch
from
28d402f
to
a120f0f
Compare
| Type: schema.TypeBool, | ||
| Required: true, | ||
| Optional: true, |
There was a problem hiding this comment.
We cleared up my confusion in chat
…ashicorp#9802) fixes hashicorp#9110 An error was found where, static_routes_only was not set on a vpn connection import. This commit introduces setting the static_routes_only to false when no Options are found. This follows the AWS convention as follows: ``` - options (structure) Indicates whether the VPN connection requires static routes. If you are creating a VPN connection for a device that does not support BGP, you must specify true . Default: false ``` So we take it that `static_options_only` is false by default ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVpnConnection_' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2016/11/02 10:38:18 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVpnConnection_ -timeout 120m === RUN TestAccAWSVpnConnection_importBasic --- PASS: TestAccAWSVpnConnection_importBasic (178.29s) === RUN TestAccAWSVpnConnection_basic --- PASS: TestAccAWSVpnConnection_basic (336.81s) === RUN TestAccAWSVpnConnection_withoutStaticRoutes --- PASS: TestAccAWSVpnConnection_withoutStaticRoutes (195.45s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 710.572s ```
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
fixes #9110
An error was found where, static_routes_only was not set on a vpn
connection import. This commit introduces setting the static_routes_only
to false when no Options are found. This follows the AWS convention as follows:
So we take it that
static_options_onlyis false by default