adds cloudflare_teams_route and acceptance tests #1572
Conversation
|
Oops! It looks like no changelog entry is attached to this PR. Please include a release note as described in https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/docs/changelog-process.md. Example: If you do not require a release note to be included, please add the |
terinjokes
force-pushed
the
teams_route
branch
2 times, most recently
from
6e4a326 to
287e36c
Compare
terinjokes
force-pushed
the
teams_route
branch
2 times, most recently
from
4b2e54d to
cac449a
Compare
Fixes cloudflare#1086 Bug: K8S-4828
…source for TunnelID
| func resourceCloudflareTeamsRouteImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { | ||
| attributes := strings.SplitN(d.Id(), "/", 3) | ||
|
|
||
| if len(attributes) != 2 { |
There was a problem hiding this comment.
This import doesn't seem like it'll work as it splits into 3 parts but looks for only 2 here. Similarly, since the read function only uses the account_id and network to find the route, shouldn't import just require those two fields? tunnel_id here isn't actually used, as the read call overrides it from the api response.
There was a problem hiding this comment.
Good catch, I originally implemented it with a different API call where tunnel_id was required. I'll send a follow-up PR in a few moments.
There was a problem hiding this comment.
doh. i hadn't noticed you'd responded ... and just submitted a pr myself ... feel free to kill mine if you've got one.
| func resourceCloudflareTeamsRouteRead(d *schema.ResourceData, meta interface{}) error { | ||
| client := meta.(*cloudflare.API) | ||
|
|
||
| tunnelRoute, err := client.GetTunnelRouteForIP(context.Background(), cloudflare.TunnelRoutesForIPParams{ |
There was a problem hiding this comment.
The use of this to read the resource seems fragile. If you are managing two routes, say 10.0.0.0/8 and 10.10.0.0/24 via terraform and 10.10.0.0/24 gets manually removed on accident ... terraform will scan resources and actually find the route for 10.0.0.0/8 for the 10.10.0.0/24 resource. Was it not feasible to use the List Tunnel Routes API passing in the tunnel_id parameter to specify an exact tunnel?
There was a problem hiding this comment.
Alternatively, perhaps this should verify the network returned is an exact match for what was requested to prevent this?
There was a problem hiding this comment.
i guess more than one network could route to the same tunnel, but you could pass in network_subnet=10.10.0.0/24 and network_superset=10.10.0.0/24 to get the exact route. And probably include is_deleted=false too ...
There was a problem hiding this comment.
I'll take a look at doing that instead. It's a lot of workaround for an endpoint that doesn't follow the normal pattern of the V4 API.
| Network: d.Get("network").(string), | ||
| } | ||
|
|
||
| if comment, ok := d.Get("comment").(string); ok { |
There was a problem hiding this comment.
If you have a comment in a resource and then simply delete the line with the intent to remove the comment, will it remove it? Perhaps the comment parameter should have a default of "" so it's always set to something since the API seems to always return an empty string as the comment field.
|
This functionality has been released in v3.14.0 of the Terraform Cloudflare Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
No description provided.