-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add 'google_compute_shared_vpc_service_project_association' resource #453
Conversation
@@ -150,7 +147,16 @@ func resourceComputeSharedVpcDelete(d *schema.ResourceData, meta interface{}) er | |||
return nil | |||
} | |||
|
|||
func enableResource(config *Config, hostProject, project string) error { | |||
func isDisabledXpnResourceError(err error) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added isDisabledXpnResourceError()
otherwise resource deletion (esp. for the Shared VPC resource) fails when attempting to disable an already disabled service project:
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: HTTP/2.0 400 Bad Request
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: Alt-Svc: quic=":443"; ma=2592000; v="39,38,37,35"
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: Cache-Control: private, max-age=0
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: Content-Type: application/json; charset=UTF-8
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: Date: Sat, 23 Sep 2017 21:37:26 GMT
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: Expires: Sat, 23 Sep 2017 21:37:26 GMT
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: Server: GSE
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: Vary: Origin
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: Vary: X-Origin
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: X-Content-Type-Options: nosniff
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: X-Frame-Options: SAMEORIGIN
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: X-Xss-Protection: 1; mode=block
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google:
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: {
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: "error": {
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: "errors": [
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: {
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: "domain": "global",
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: "reason": "invalidResourceUsage",
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: "message": "Invalid resource usage: 'The resource 'projects/test-service4-id' is not linked to shared VPC host 'projects/test-host4-id'.'."
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: }
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: ],
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: "code": 400,
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: "message": "Invalid resource usage: 'The resource 'projects/test-service4-id' is not linked to shared VPC host 'projects/test-host4-id'.'."
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: }
2017-09-23T17:37:26.228-0400 [DEBUG] plugin.terraform-provider-google: }
} | ||
|
||
serviceProjects := []string{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced this code with new function findXpnResources()
.
@@ -106,7 +101,7 @@ func resourceComputeSharedVpcUpdate(d *schema.ResourceData, meta interface{}) er | |||
for project, _ := range oldMap { | |||
if _, ok := newMap[project]; !ok { | |||
// The project is in the old config but not the new one, disable it | |||
if err := disableResource(config, hostProject, project); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed disableResource
to disableXpnResource
.
@@ -115,7 +110,7 @@ func resourceComputeSharedVpcUpdate(d *schema.ResourceData, meta interface{}) er | |||
for project, _ := range newMap { | |||
if _, ok := oldMap[project]; !ok { | |||
// The project is in the new config but not the old one, enable it | |||
if err := enableResource(config, hostProject, project); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed enableResource
to enableXpnResource
.
@@ -74,19 +76,12 @@ func resourceComputeSharedVpcRead(d *schema.ResourceData, meta interface{}) erro | |||
if project.XpnProjectStatus != "HOST" { | |||
log.Printf("[WARN] Removing Shared VPC host resource %q because it's not enabled server-side", hostProject) | |||
d.SetId("") | |||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug fix 😄.
@@ -26,6 +27,7 @@ func resourceComputeSharedVpc() *schema.Resource { | |||
"service_projects": &schema.Schema{ | |||
Type: schema.TypeSet, | |||
Optional: true, | |||
Computed: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
service_projects
becomes Computed
as these IDs can also be managed by google_compute_shared_vpc_service_project_association
resources.
It looks like this PR is obsoleted by the changes in #572. |
To avoid having two different ways of setting up a shared VPC that can lead to indeterministic behavior if used at the same time, we decided to have one resource to enable the host project and one resource to enable each service project. We decided not to include a resource to setup both in the same resource to avoid the confusion described above. Thanks for this PR, I used bits of it to handle errors case properly in #572. |
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! |
Fixes #452.
Add a
google_compute_shared_vpc_service_project_association
resource which allows associating a service project with a Shared VPC host project.Acceptance test: