-
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
Autogenerate Subnetwork. #1661
Autogenerate Subnetwork. #1661
Conversation
216ce6e
to
45a5273
Compare
45a5273
to
0b8e353
Compare
return true | ||
} | ||
|
||
func splitSubnetID(id string) (region string, name 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.
This seems to only be used for tests- mind moving it there?
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.
Kind of a hassle to move things between generated and non-generated code in one PR. I can do it by copying the test into MagicModules, but that kind of doesn't feel right... is that how you'd suggest I do it?
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.
Ah yeah, you could open a quick PR in TF to do the move first, or we can just do it later.
}, | ||
"ip_cidr_range": &schema.Schema{ | ||
"range_name": { |
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.
can this keep the validatefunc?
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.
Yes, actually! I tried yesterday and failed, but I figured it out this morning. Thanks!
Type: schema.TypeBool, | ||
Optional: true, | ||
}, | ||
|
||
"self_link": &schema.Schema{ | ||
"region": { |
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.
This should also be computed so it can be filled in if the user is using the provider-level region
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.
Ah, that's default_from_api: true
. Yep!
Type: schema.TypeBool, | ||
Optional: true, | ||
ForceNew: 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.
I think it's the docs that are wrong on this- we have a test that patches enable_flow_logs and it works just fine.
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.
Okay, then I'll switch it back and file another bug. The docs folks must be sick of me.
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.
Hopefully they think you're super for making the docs better :D
is not provided, the provider project is used. | ||
```hcl | ||
resource "google_compute_network" "custom-test" { | ||
name = "%s" |
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.
Let's have actual fake names instead of %s. Also, mind aligning =
s?
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.
Yeah, that looks a lot better.
The name of the resource, provided by the client when initially | ||
creating the resource. The name must be 1-63 characters long, and | ||
comply with RFC1035. Specifically, the name must be 1-63 characters | ||
long and match the regular expression `[a-z]([-a-z0-9]*[a-z0-9])?` which |
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.
sounds like a validatefunc to me!
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.
Good ol' validateGCPName
.
assigned external IP addresses. | ||
* `region` - | ||
(Optional) | ||
URL of the region where the regional address resides. |
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.
This looks like a copy/paste error
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.
Haha, I think that was in magic modules already, but it was someone's copy/paste error.
The gateway address for default routes to reach destination addresses | ||
outside this subnetwork. | ||
* `fingerprint` - | ||
Fingerprint of this resource. A hash of the contents stored in this |
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.
nit: this feels like a lot of information for something we're hoping the user ignores
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.
Agreed, substantially reduced.
510d8b0
to
8850c64
Compare
8850c64
to
7205a71
Compare
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! |
/cc @ndmckinley