Add new resource google_compute_network_peering
#259
Conversation
rosbo
force-pushed
the
resource_vpc_peering
branch
from
164ede4 to
c9e054a
Compare
rosbo
force-pushed
the
resource_vpc_peering
branch
from
c9e054a to
4441908
Compare
| auto_create_subnetworks = "false" | ||
| } | ||
|
|
||
| // Both network must create a peering with each other for the peering |
There was a problem hiding this comment.
Let's move these comments to the resource description above; that way users will read them as they read what the resource is and not just in the example.
|
|
||
| # google\_compute\_network\_peering | ||
|
|
||
| Manages a network peering within GCE. |
There was a problem hiding this comment.
Let's add a link to the API reference and to a relevant set of GCP docs so that users can quickly learn more about the resource if they'd like.
|
|
||
| // Both network must create a peering with each other for the peering | ||
| // to be functional. | ||
| resource "google_compute_network_peering" "peering1" { |
There was a problem hiding this comment.
Nit: Let's make the resource this page is about live at the top of the example config.
| * `peer_network` - (Required) Resource link of the peer network. | ||
|
|
||
| * `auto_create_routes` - (Optional) If set to true, the routes between the two networks will | ||
| be created and managed automatically. Defaults to true. |
| In addition to the arguments listed above, the following computed attributes are | ||
| exported: | ||
|
|
||
| * `state` - (Computed) State for the peering. |
There was a problem hiding this comment.
Nit: No need to include that they are (Computed)
| return nil | ||
| } | ||
|
|
||
| // No need to set the `name` and `network` fields. We use both of them to find the peering. |
There was a problem hiding this comment.
This comment can probably be removed.
| return m[2] | ||
| } | ||
|
|
||
| func peerNetworkLinkDiffSuppress(k, old, new string, d *schema.ResourceData) bool { |
There was a problem hiding this comment.
Would compareSelfLinkRelativePaths() from self_link_helpers.go work here instead? It compares self links by just the projects/ onwards.
There was a problem hiding this comment.
It works. Removed my method. Thanks for the pointer
|
|
||
| var testAccComputeNetworkPeering_basic = fmt.Sprintf(` | ||
| resource "google_compute_network" "network1" { | ||
| name = "network-test-1-%s" |
There was a problem hiding this comment.
nit: Let's use tabs instead of spaces to be consistent with other test configs.
google/validation.go
Outdated
| @@ -6,6 +6,8 @@ import ( | |||
| "regexp" | |||
| ) | |||
|
|
|||
| const ProjectRegex = "(?:(?:[-a-z0-9]{1,63}\\.)*(?:[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?):)?(?:[0-9]{1,19}|(?:[a-z0-9](?:[-a-z0-9]{0,61}[a-z0-9])?))" | |||
There was a problem hiding this comment.
Project IDs generated through Terraform will follow this form, but I'm not sure if those rules have been followed for the entire lifetime of project ids in general.
We should be able to match against [^/]* and capture anything inside the projects/ part of a self link without overmatching past a /.
There was a problem hiding this comment.
I got the regex for the project description in the google cloud compute generated client:
// "project": {
// "description": "Project ID for this request.",
// "location": "path",
// "pattern": "(?:(?:[-a-z0-9]{1,63}\\.)*(?:[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?):)?(?:[0-9]{1,19}|(?:[a-z0-9](?:[-a-z0-9]{0,61}[a-z0-9])?))",
// "required": true,
// "type": "string"
// }My goal with a specific regex is to try to fail at plan instead of apply. I think we should start moving towards this when format for names are clearly defined.
There was a problem hiding this comment.
Oh cool, I hadn't noticed that regex. Do you mind adding a comment of where you got it from? 👍
| @@ -0,0 +1,229 @@ | |||
| package google | |||
|
|
|||
| import ( | |||
There was a problem hiding this comment.
nit: standard imports should all live at the top of the import block.
There was a problem hiding this comment.
Super minor: looks like sort snuck in afterwards.
rosbo
force-pushed
the
resource_vpc_peering
branch
from
9e95795 to
78f20a6
Compare
| return err | ||
| } | ||
|
|
||
| name := d.Get("name").(string) |
There was a problem hiding this comment.
nit: name, peerNetworkLink, and autoCreateRoutes can be inlined.
| @@ -0,0 +1,229 @@ | |||
| package google | |||
|
|
|||
| import ( | |||
There was a problem hiding this comment.
Super minor: looks like sort snuck in afterwards.
|
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes #178
cc @rileykarson @danawillow
Add the vpc peering functionality using its own resource.
I opened another PR yesterday with an implementation embedded into the
google_compute_network.The main advantage of having a separate resource for network peering is to go around the circlular dependency issue since both network needs to point at each other. See #242 for more details about the issue.
The main drawback of having a separate resource is that one can create peering manually (outside of Terraform) and Terraform will never now about it.