bhouse-nexthop commented on code in PR #282:
URL:
https://github.com/apache/cloudstack-terraform-provider/pull/282#discussion_r3570074379
##########
cloudstack/resource_cloudstack_network.go:
##########
@@ -471,3 +550,93 @@ func parseCIDR(d *schema.ResourceData, specifyiprange
bool) (map[string]string,
return m, nil
}
+
+// addToIPv6 adds an integer offset to an IPv6 address with proper carry
across all bytes.
+// Returns a new net.IP with the result.
+func addToIPv6(ip net.IP, offset uint64) net.IP {
+ result := make(net.IP, len(ip))
+ copy(result, ip)
+
+ carry := offset
+ // Start from the least significant byte (rightmost) and work backwards
+ for i := len(result) - 1; i >= 0 && carry > 0; i-- {
+ sum := uint64(result[i]) + carry
+ result[i] = byte(sum & 0xff)
+ carry = sum >> 8
+ }
+
+ return result
+}
+
+func parseCIDRv6(d *schema.ResourceData, specifyiprange bool)
(map[string]string, error) {
+ m := make(map[string]string, 4)
+
+ cidr := d.Get("ip6cidr").(string)
+ ip, ipnet, err := net.ParseCIDR(cidr)
+ if err != nil {
+ return nil, fmt.Errorf("Unable to parse cidr %s: %s", cidr, err)
+ }
+
+ // Validate that this is actually an IPv6 CIDR
+ if ip.To4() != nil {
+ return nil, fmt.Errorf("ip6cidr must be an IPv6 CIDR, got IPv4:
%s", cidr)
+ }
+ if len(ipnet.Mask) != net.IPv6len {
+ return nil, fmt.Errorf("ip6cidr must be an IPv6 CIDR with
16-byte mask, got %d bytes: %s", len(ipnet.Mask), cidr)
+ }
+
+ // Validate prefix length to ensure we have enough addresses for
gateway/start/end
+ ones, _ := ipnet.Mask.Size()
+ if specifyiprange {
+ // When specifyiprange is true, we need at least 3 addresses:
+ // - gateway (network + 1)
+ // - start IP (network + 2)
+ // - end IP (network + 3 or more)
+ // This requires a /126 or larger prefix (4 addresses minimum)
+ if ones > 126 {
+ return nil, fmt.Errorf("ip6cidr prefix /%d is too small
for automatic IP range generation; minimum is /126 (4 addresses)", ones)
+ }
+ } else {
+ // When specifyiprange is false, we only need the gateway
(network + 1)
+ // This requires a /127 or larger prefix (2 addresses minimum)
+ if ones > 127 {
+ return nil, fmt.Errorf("ip6cidr prefix /%d is too small
for automatic gateway generation; minimum is /127 (2 addresses)", ones)
+ }
+ }
+
+ if gateway, ok := d.GetOk("ip6gateway"); ok {
+ m["ip6gateway"] = gateway.(string)
+ } else {
+ // Default gateway to network address + 1 (e.g., 2001:db8::1)
+ gwip := addToIPv6(ipnet.IP, 1)
+ m["ip6gateway"] = gwip.String()
+ }
+
+ if startipv6, ok := d.GetOk("startipv6"); ok {
+ m["startipv6"] = startipv6.(string)
+ } else if specifyiprange {
+ // Default start IP to network address + 2
+ startip := addToIPv6(ipnet.IP, 2)
+ m["startipv6"] = startip.String()
+ }
+
+ if endip, ok := d.GetOk("endipv6"); ok {
+ m["endipv6"] = endip.(string)
Review Comment:
Fixed in 0eb68de. Added a `validateIPv6InCIDR` helper that parses each
user-supplied `ip6gateway`/`startipv6`/`endipv6` and verifies it falls within
`ip6cidr`, returning a clear local error before the API call. Only rejects
values that would already fail server-side, so no change to valid configs.
##########
website/docs/r/network.html.markdown:
##########
@@ -43,6 +55,19 @@ The following arguments are supported:
* `endip` - (Optional) End of the IP block that will be available on the
network. Defaults to the last available IP in the range.
+* `ip6cidr` - (Optional) The IPv6 CIDR block for the network. Changing this
+ forces a new resource to be created.
+
+* `ip6gateway` - (Optional) IPv6 Gateway that will be provided to the instances
+ in this network. Defaults to the second address in the subnet (network
address + 1,
+ e.g., 2001:db8::1 for 2001:db8::/64).
+
+* `startipv6` - (Optional) Start of the IPv6 block that will be available on
the
+ network. Defaults to the second available IP in the range.
+
+* `endipv6` - (Optional) End of the IPv6 block that will be available on the
+ network. Defaults to the last available IP in the range.
Review Comment:
Fixed in 0eb68de. Clarified in the docs that `startipv6`/`endipv6` defaults
are only applied when the network offering has `specifyipranges` enabled
(otherwise they are not sent to the API).
##########
website/docs/r/network.html.markdown:
##########
@@ -43,6 +55,19 @@ The following arguments are supported:
* `endip` - (Optional) End of the IP block that will be available on the
network. Defaults to the last available IP in the range.
+* `ip6cidr` - (Optional) The IPv6 CIDR block for the network. Changing this
+ forces a new resource to be created.
Review Comment:
Fixed in 0eb68de. Documented on `ip6cidr` that the value must be an IPv6
CIDR (IPv4 is rejected) and the allowed prefix range (min /127, or /126 with
`specifyipranges`).
##########
cloudstack/resource_cloudstack_network_unit_test.go:
##########
@@ -0,0 +1,321 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package cloudstack
+
+import (
+ "strings"
+ "testing"
+
+ "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+)
+
+func TestParseCIDRv6_DefaultGateway(t *testing.T) {
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::/64",
+ })
+
+ result, err := parseCIDRv6(d, false)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 failed: %v", err)
+ }
+
+ // Default gateway should be network address + 1
+ expectedGateway := "2001:db8::1"
+ if result["ip6gateway"] != expectedGateway {
+ t.Errorf("Expected gateway %s, got %s", expectedGateway,
result["ip6gateway"])
+ }
+
+ // When specifyiprange is false, startipv6 and endipv6 should not be set
+ if _, ok := result["startipv6"]; ok {
+ t.Errorf("startipv6 should not be set when specifyiprange is
false")
+ }
+ if _, ok := result["endipv6"]; ok {
+ t.Errorf("endipv6 should not be set when specifyiprange is
false")
+ }
+}
+
+func TestParseCIDRv6_CustomGateway(t *testing.T) {
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::/64",
+ "ip6gateway": "2001:db8::1",
+ })
+
+ result, err := parseCIDRv6(d, false)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 failed: %v", err)
+ }
+
+ expectedGateway := "2001:db8::1"
+ if result["ip6gateway"] != expectedGateway {
+ t.Errorf("Expected gateway %s, got %s", expectedGateway,
result["ip6gateway"])
+ }
+}
+
+func TestParseCIDRv6_WithIPRange(t *testing.T) {
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::/64",
+ })
+
+ result, err := parseCIDRv6(d, true)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 failed: %v", err)
+ }
+
+ // Check gateway (should be network address + 1)
+ expectedGateway := "2001:db8::1"
+ if result["ip6gateway"] != expectedGateway {
+ t.Errorf("Expected gateway %s, got %s", expectedGateway,
result["ip6gateway"])
+ }
+
+ // Check start IP (should be network address + 2)
+ expectedStartIP := "2001:db8::2"
+ if result["startipv6"] != expectedStartIP {
+ t.Errorf("Expected start IP %s, got %s", expectedStartIP,
result["startipv6"])
+ }
+
+ // Check end IP (should be the last address in the /64 range)
+ expectedEndIP := "2001:db8::ffff:ffff:ffff:ffff"
+ if result["endipv6"] != expectedEndIP {
+ t.Errorf("Expected end IP %s, got %s", expectedEndIP,
result["endipv6"])
+ }
+}
+
+func TestParseCIDRv6_CustomIPRange(t *testing.T) {
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8:1::/64",
+ "startipv6": "2001:db8:1::100",
+ "endipv6": "2001:db8:1::200",
+ })
+
+ result, err := parseCIDRv6(d, true)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 failed: %v", err)
+ }
+
+ // Check that custom values are used
+ if result["startipv6"] != "2001:db8:1::100" {
+ t.Errorf("Expected custom start IP 2001:db8:1::100, got %s",
result["startipv6"])
+ }
+ if result["endipv6"] != "2001:db8:1::200" {
+ t.Errorf("Expected custom end IP 2001:db8:1::200, got %s",
result["endipv6"])
+ }
+}
+
+func TestParseCIDRv6_SmallerPrefix(t *testing.T) {
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::/48",
+ })
+
+ result, err := parseCIDRv6(d, true)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 failed: %v", err)
+ }
+
+ // For a /48, the end IP should have the last 80 bits set to 1
+ expectedEndIP := "2001:db8:0:ffff:ffff:ffff:ffff:ffff"
+ if result["endipv6"] != expectedEndIP {
+ t.Errorf("Expected end IP %s, got %s", expectedEndIP,
result["endipv6"])
+ }
+}
+
+func TestParseCIDRv6_RejectsIPv4(t *testing.T) {
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "10.0.0.0/24",
+ })
+
+ _, err := parseCIDRv6(d, false)
+ if err == nil {
+ t.Fatal("parseCIDRv6 should reject IPv4 CIDR")
+ }
+
+ expectedError := "ip6cidr must be an IPv6 CIDR, got IPv4"
+ if !strings.HasPrefix(err.Error(), expectedError) {
+ t.Errorf("Expected error message to start with '%s', got '%s'",
expectedError, err.Error())
+ }
+}
+
+func TestParseCIDRv6_Prefix128_NoIPRange(t *testing.T) {
+ // /128 is a single address - should fail even without IP range
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::1/128",
+ })
+
+ _, err := parseCIDRv6(d, false)
+ if err == nil {
+ t.Fatal("parseCIDRv6 should reject /128 prefix (single
address)")
+ }
+
+ expectedError := "ip6cidr prefix /128 is too small"
+ if !strings.HasPrefix(err.Error(), expectedError) {
+ t.Errorf("Expected error message to start with '%s', got '%s'",
expectedError, err.Error())
+ }
+}
+
+func TestParseCIDRv6_Prefix127_NoIPRange(t *testing.T) {
+ // /127 has 2 addresses - should work without IP range (only needs
gateway)
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::/127",
+ })
+
+ result, err := parseCIDRv6(d, false)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 should accept /127 prefix without IP
range: %v", err)
+ }
+
+ // Should have gateway
+ if _, ok := result["ip6gateway"]; !ok {
+ t.Error("Expected ip6gateway to be set")
+ }
+
+ // Should not have start/end IP
+ if _, ok := result["startipv6"]; ok {
+ t.Error("startipv6 should not be set when specifyiprange is
false")
+ }
+}
+
+func TestParseCIDRv6_Prefix127_WithIPRange(t *testing.T) {
+ // /127 has only 2 addresses - should fail with IP range (needs 3+
addresses)
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::/127",
+ })
+
+ _, err := parseCIDRv6(d, true)
+ if err == nil {
+ t.Fatal("parseCIDRv6 should reject /127 prefix with IP range
(only 2 addresses)")
+ }
+
+ expectedError := "ip6cidr prefix /127 is too small for automatic IP
range generation"
+ if !strings.HasPrefix(err.Error(), expectedError) {
+ t.Errorf("Expected error message to start with '%s', got '%s'",
expectedError, err.Error())
+ }
+}
+
+func TestParseCIDRv6_Prefix126_WithIPRange(t *testing.T) {
+ // /126 has 4 addresses - should work with IP range
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::/126",
+ })
+
+ result, err := parseCIDRv6(d, true)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 should accept /126 prefix with IP range:
%v", err)
+ }
+
+ // Should have gateway, start, and end
+ if _, ok := result["ip6gateway"]; !ok {
+ t.Error("Expected ip6gateway to be set")
+ }
+ if _, ok := result["startipv6"]; !ok {
+ t.Error("Expected startipv6 to be set")
+ }
+ if _, ok := result["endipv6"]; !ok {
+ t.Error("Expected endipv6 to be set")
+ }
+
+ // Verify the end IP is correct for /126 (last 2 bits set to 1)
+ expectedEndIP := "2001:db8::3"
+ if result["endipv6"] != expectedEndIP {
+ t.Errorf("Expected end IP %s for /126, got %s", expectedEndIP,
result["endipv6"])
+ }
+}
+
+func TestParseCIDRv6_NonZeroNetworkAddress(t *testing.T) {
+ // Test with a CIDR where the network address doesn't end in ::0
+ // This tests the fix for proper IPv6 address arithmetic with carry
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::4/126",
+ })
+
+ result, err := parseCIDRv6(d, true)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 failed: %v", err)
+ }
+
+ // For 2001:db8::4/126, the network is 2001:db8::4
+ // Gateway should be network + 1 = 2001:db8::5
+ expectedGateway := "2001:db8::5"
+ if result["ip6gateway"] != expectedGateway {
+ t.Errorf("Expected gateway %s, got %s", expectedGateway,
result["ip6gateway"])
+ }
+
+ // Start IP should be network + 2 = 2001:db8::6
+ expectedStartIP := "2001:db8::6"
+ if result["startipv6"] != expectedStartIP {
+ t.Errorf("Expected start IP %s, got %s", expectedStartIP,
result["startipv6"])
+ }
+
+ // End IP should be network + 3 = 2001:db8::7 (last address in /126)
+ expectedEndIP := "2001:db8::7"
+ if result["endipv6"] != expectedEndIP {
+ t.Errorf("Expected end IP %s, got %s", expectedEndIP,
result["endipv6"])
+ }
+}
+
+func TestParseCIDRv6_NonAlignedPrefix(t *testing.T) {
+ // Test with a /124 prefix where network address has non-zero low-order
bits
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::f0/124",
+ })
+
+ result, err := parseCIDRv6(d, true)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 failed: %v", err)
+ }
+
+ // For 2001:db8::f0/124, the network is 2001:db8::f0
+ // Gateway should be network + 1 = 2001:db8::f1
+ expectedGateway := "2001:db8::f1"
+ if result["ip6gateway"] != expectedGateway {
+ t.Errorf("Expected gateway %s, got %s", expectedGateway,
result["ip6gateway"])
+ }
+
+ // Start IP should be network + 2 = 2001:db8::f2
+ expectedStartIP := "2001:db8::f2"
+ if result["startipv6"] != expectedStartIP {
+ t.Errorf("Expected start IP %s, got %s", expectedStartIP,
result["startipv6"])
+ }
+
+ // End IP should be 2001:db8::ff (last address in /124)
+ expectedEndIP := "2001:db8::ff"
+ if result["endipv6"] != expectedEndIP {
+ t.Errorf("Expected end IP %s, got %s", expectedEndIP,
result["endipv6"])
+ }
+}
+
+func TestParseCIDRv6_CarryAcrossBytes(t *testing.T) {
+ // Test carry across byte boundaries
+ // Use a network address ending in ::ff to test carry to next byte
+ d := schema.TestResourceDataRaw(t, resourceCloudStackNetwork().Schema,
map[string]interface{}{
+ "ip6cidr": "2001:db8::ff/120",
+ })
+
+ result, err := parseCIDRv6(d, true)
+ if err != nil {
+ t.Fatalf("parseCIDRv6 failed: %v", err)
+ }
+
+ // For 2001:db8::ff/120, network is 2001:db8::0 (masked)
+ // But let's test with an address that will actually carry
+ // Gateway should be network + 1
+ expectedGateway := "2001:db8::1"
+ if result["ip6gateway"] != expectedGateway {
+ t.Errorf("Expected gateway %s, got %s", expectedGateway,
result["ip6gateway"])
+ }
+}
Review Comment:
Fixed in 0eb68de. Replaced the test with `TestAddToIPv6_CarryAcrossBytes`,
which calls `addToIPv6` directly with addresses ending in `::ff`/`::ffff` so a
real carry across byte boundaries is actually exercised.
--
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]
