Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Network peering gets network project from network self_link #498

Merged
merged 1 commit into from
Oct 3, 2017

Conversation

rosbo
Copy link
Contributor

@rosbo rosbo commented Sep 29, 2017

Fixes #496

Network peering requires two network self_link. One for each of the network in the peering.
We were reading the project from config.Project. This means that if config.Project is not set, we could not set the peering.

Since we already require a network self_link, we should extract the project from self_link.

cc/ @rochdev

@rochdev
Copy link

rochdev commented Sep 29, 2017

I tend to use network.name most of the time. Would it be possible to also support an explicit project? Or should I maybe switch to self_link instead?

@rosbo
Copy link
Contributor Author

rosbo commented Sep 29, 2017

As of now, this resource requires a self_link, otherwise the ValidateFunc will fail. Since having a self_link in the network field is enforced at plan time, I liked the idea of having only one way to specify a project to prevent the situation where the network field contains a self_link with project A and the project field contains B. In that case, should we fail and display an error message? Should we silently pick A? Or B?

Additionally, the peer_network field can reference a network in the same or a different project. This means, for consistency, we would need to support specifying only the network name in peer_network. However, since the peer network may not be in the same project, then we would need a separate project field (we could call it peer_project). This would create the same problem as above if someone use a self_link and a project with a project that doesn't match.

The use of self_link and its enforcement at plan time remove all this confusion for the users.

@rosbo rosbo merged commit e943696 into hashicorp:master Oct 3, 2017
@rosbo rosbo deleted the network_peering_project branch October 3, 2017 20:30
negz pushed a commit to negz/terraform-provider-google that referenced this pull request Oct 17, 2017
negz pushed a commit to negz/terraform-provider-google that referenced this pull request Oct 17, 2017
@ghost
Copy link

ghost commented Mar 30, 2020

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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google_compute_network_peering project field is required but cannot be set
3 participants