Add vnet param to tunnel routes #1668
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 |
|
This project handles dependency version bumps (including upstream changes from cloudflare-go) independently of the standard PR process using automation. This allows the dependency upgrades to land without causing merge conflicts in multiple branches and handled in a consistent way. The exception to this is security related dependency upgrades but they should be co-ordinated with the maintainer team privately. Please remove the changes to the go.mod or go.sum files from this PR in order to proceed with review and merging. |
vavsab
force-pushed
the
add-vnet-param
branch
2 times, most recently
from
5cc56a8
to
7df8d47
Compare
| d.SetId(newTunnelRoute.Network) | ||
| if virtualNetworkID != "" { | ||
| // It's possible to create several routes with the same network but different virtual network ids. | ||
| d.SetId(fmt.Sprintf("%s/%s", newTunnelRoute.Network, virtualNetworkID)) |
There was a problem hiding this comment.
if you are doing this, you'll want to make virtual_network_id ForceNew otherwise, the underlying ID will change everytime the virtual_network_id updates and you'll have issues in Update/Read operations.
we should also use stringChecksum to keep the ID value looking consistent with others.
There was a problem hiding this comment.
Have not found a way to update vnet. Changed it for ForceNew and used stringChecksum (btw, do you know why we need to use it (except following project code style)?)
There was a problem hiding this comment.
We always had slashes in ID even before (because CIDR always contains slash) but were not using stringChecksum.. so what is the real purpose?
| if err != nil { | ||
| return diag.FromErr(fmt.Errorf("error deleting Tunnel Route for Network %q: %w", d.Get("network").(string), err)) | ||
| return diag.FromErr(fmt.Errorf("error deleting Tunnel Route for Network %q: %w", network, err)) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func resourceCloudflareTunnelRouteImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { |
There was a problem hiding this comment.
this seems unnecessarily complex; let's just force it to include the virtual_network_id for explicitness.
There was a problem hiding this comment.
As a cloudflared user I know that vnet is totally optional param there.
Making it required during import will create additional headache for developers that are always working inside of the "default" vnet since it's not easy at all to get GUID of the default vnet (you cannot do it through UI. Only by using API or cloudflared).
But anyway for me it does not make any difference. So let's make it required.
|
as you've updated the |
|
No problem. Will do 🙂 |
|
awesome 🍭 thanks! |
|
This functionality has been released in v3.17.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! |
Add missing vnet param
Closes #1605