Copilot commented on code in PR #282:
URL:
https://github.com/apache/cloudstack-terraform-provider/pull/282#discussion_r2910363546
##########
cloudstack/resource_cloudstack_network.go:
##########
@@ -471,3 +536,61 @@ func parseCIDR(d *schema.ResourceData, specifyiprange
bool) (map[string]string,
return m, nil
}
+
+func parseCIDRv6(d *schema.ResourceData, specifyiprange bool)
(map[string]string, error) {
+ m := make(map[string]string, 4)
+
+ cidr := d.Get("ip6cidr").(string)
+ _, ipnet, err := net.ParseCIDR(cidr)
+ if err != nil {
+ return nil, fmt.Errorf("Unable to parse cidr %s: %s", cidr, err)
+ }
+
+ if gateway, ok := d.GetOk("ip6gateway"); ok {
+ m["ip6gateway"] = gateway.(string)
+ } else {
+ // Default gateway to network address + 1 (e.g., 2001:db8::1)
+ ip16 := ipnet.IP.To16()
+ if ip16 == nil {
+ return nil, fmt.Errorf("cidr not valid for ipv6")
+ }
+ gwip := make(net.IP, len(ip16))
+ copy(gwip, ip16)
+ gwip[len(ip16)-1] = 1
+ m["ip6gateway"] = gwip.String()
+ }
+
+ if startipv6, ok := d.GetOk("startipv6"); ok {
+ m["startipv6"] = startipv6.(string)
+ } else if specifyiprange {
+ ip16 := ipnet.IP.To16()
+ if ip16 == nil {
+ return nil, fmt.Errorf("cidr not valid for ipv6")
+ }
+
+ myip := make(net.IP, len(ip16))
+ copy(myip, ip16)
+ myip[len(ip16)-1] = 2
+ m["startipv6"] = myip.String()
+ }
Review Comment:
Defaulting gateway/start IPv6 by setting only the last byte to 1/2 assumes
the subnet has at least 3 usable addresses. For prefixes like /128 (single
address) or /127 (two addresses), this will produce addresses outside the CIDR.
Consider validating the prefix length and returning an error (or adjusting the
defaulting rules) when the range cannot accommodate gateway/start/end.
##########
cloudstack/resource_cloudstack_network.go:
##########
@@ -209,6 +236,31 @@ func resourceCloudStackNetworkCreate(d
*schema.ResourceData, meta interface{}) e
p.SetEndip(endip)
}
+ // IPv6 support
+ if ip6cidr, ok := d.GetOk("ip6cidr"); ok && ip6cidr.(string) != none {
+ m6, err := parseCIDRv6(d, no.Specifyipranges)
+ if err != nil {
+ return err
+ }
+
+ p.SetIp6cidr(ip6cidr.(string))
Review Comment:
The `ip6cidr` create guard checks `ip6cidr.(string) != none`, but `none` is
an ACL sentinel and `ip6cidr` has no default of `none`. This makes `ip6cidr =
"none"` behave like a CIDR and fail parsing unnecessarily; consider removing
the `!= none` check and just rely on `GetOk`/non-empty CIDR validation.
##########
cloudstack/resource_cloudstack_network_test.go:
##########
@@ -165,6 +172,75 @@ func TestAccCloudStackNetwork_importProject(t *testing.T) {
})
}
+func TestAccCloudStackNetwork_ipv6(t *testing.T) {
+ t.Skip("Skipping IPv6 test: CloudStack simulator only supports IPv6
with advanced shared networks")
+ var network cloudstack.Network
Review Comment:
These IPv6 acceptance tests are unconditionally skipped via `t.Skip(...)`,
so they will never run even on real CloudStack environments with IPv6 support.
Consider making the skip conditional (e.g., based on an env var or a
simulator-detection helper) so the tests can execute in non-simulator CI/jobs.
##########
cloudstack/resource_cloudstack_network.go:
##########
@@ -306,6 +358,19 @@ func resourceCloudStackNetworkRead(d *schema.ResourceData,
meta interface{}) err
d.Set("network_domain", n.Networkdomain)
d.Set("vpc_id", n.Vpcid)
+ // Only set ip6cidr if it has a value
+ if n.Ip6cidr != "" {
+ d.Set("ip6cidr", n.Ip6cidr)
+ }
+
+ // Only set ip6gateway if it has a value
+ if n.Ip6gateway != "" {
+ d.Set("ip6gateway", n.Ip6gateway)
+ }
Review Comment:
In Read, `ip6cidr`/`ip6gateway` are only set when non-empty. This can leave
stale IPv6 values in state if IPv6 is removed or cleared server-side,
preventing drift detection. Consider always setting these attributes from the
API response (even to an empty string) and rely on the schema to avoid diffs
when users haven't configured IPv6.
##########
cloudstack/resource_cloudstack_network.go:
##########
@@ -471,3 +536,61 @@ func parseCIDR(d *schema.ResourceData, specifyiprange
bool) (map[string]string,
return m, nil
}
+
+func parseCIDRv6(d *schema.ResourceData, specifyiprange bool)
(map[string]string, error) {
+ m := make(map[string]string, 4)
+
+ cidr := d.Get("ip6cidr").(string)
+ _, ipnet, err := net.ParseCIDR(cidr)
+ if err != nil {
+ return nil, fmt.Errorf("Unable to parse cidr %s: %s", cidr, err)
+ }
Review Comment:
`parseCIDRv6` currently accepts IPv4 CIDRs (since `To16()` is non-nil for
IPv4), and when `specifyiprange` is true it will index `ipnet.Mask[i]` assuming
a 16-byte mask, which can panic for IPv4 masks. Add an explicit IPv6 check
(e.g., `ip.To4() == nil` and `len(ipnet.Mask) == net.IPv6len`) and return a
validation error for non-IPv6 CIDRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
