fix: make cidr optional for L2 network creation#289
fix: make cidr optional for L2 network creation#289Damans227 wants to merge 1 commit intoapache:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the cloudstack_network Terraform resource to support creating L2 networks without requiring a CIDR, aligning the provider schema with CloudStack’s createNetwork API requirements.
Changes:
- Made
cidroptional in thecloudstack_networkschema. - Applied gateway/netmask/IP-range parameter logic only when
cidris provided. - Added an acceptance test that creates an L2 network without specifying
cidr.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
cloudstack/resource_cloudstack_network.go |
Makes cidr optional and gates CIDR-derived network parameter setting behind a presence check. |
cloudstack/resource_cloudstack_network_test.go |
Adds an acceptance test for L2 network creation without CIDR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if _, ok := d.GetOk("cidr"); ok { | ||
| m, err := parseCIDR(d, no.Specifyipranges) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Now that cidr is optional, users can set gateway/startip/endip without setting cidr. In that case this block is skipped and those arguments are silently ignored, which can lead to confusing behavior and perpetual diffs (e.g., config sets gateway but API/state stays empty). Consider validating that gateway, startip, and endip may only be set when cidr is also set (e.g., schema RequiredWith/ConflictsWith or an explicit check in Create returning a clear error).
| if _, ok := d.GetOk("cidr"); ok { | ||
| m, err := parseCIDR(d, no.Specifyipranges) | ||
| if err != nil { |
There was a problem hiding this comment.
With cidr optional, GetNetworkOfferingByID is only needed when cidr is present (to read Specifyipranges for parseCIDR). To avoid an extra API call on L2/no-cidr networks, consider moving the network-offering lookup inside the cidr conditional block.
The
cidrfield was marked asRequiredin the Terraform provider's schema forcloudstack_network, but the CloudStackcreateNetworkAPI only requiresname,networkOfferingId, andzoneId.gatewayandnetmaskare optional and unnecessary for L2 networks.This change makes
cidroptional and conditionally applies the CIDR/gateway/netmask logic only when a CIDR is provided.An acceptance test has been added to verify L2 network creation without CIDR.