OSAC-365: canonicalize CIDR values on VirtualNetwork and Subnet Create

internal/servers/cidr_validation.go

@@ -0,0 +1,54 @@
+/*
+Copyright (c) 2026 Red Hat Inc.
+
+Licensed 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 servers
+
+import (
+	"net/netip"
+
+	grpccodes "google.golang.org/grpc/codes"
+	grpcstatus "google.golang.org/grpc/status"
+)
+
+const (
+	cidrIPv4 = "IPv4"
+	cidrIPv6 = "IPv6"
+)
+
+// parseAndValidateCIDR parses cidrStr, validates it matches the expected IP version,
+// and returns the canonical CIDR notation.
+func parseAndValidateCIDR(cidrStr string, ipVersion string) (string, error) {
+	prefix, err := netip.ParsePrefix(cidrStr)
+	if err != nil {
+		return "", grpcstatus.Errorf(grpccodes.InvalidArgument,
+			"invalid %s CIDR format '%s': %v", ipVersion, cidrStr, err)
+	}
+
+	isIPv4 := prefix.Addr().Is4()
+	if ipVersion == cidrIPv4 && !isIPv4 {
+		return "", grpcstatus.Errorf(grpccodes.InvalidArgument,
+			"field 'ipv4_cidr' contains IPv6 address: %s", cidrStr)
+	}
+	if ipVersion == cidrIPv6 && isIPv4 {
+		return "", grpcstatus.Errorf(grpccodes.InvalidArgument,
+			"field 'ipv6_cidr' contains IPv4 address: %s", cidrStr)
+	}
+
+	return prefix.Masked().String(), nil
+}
+
+// validateCIDR validates a CIDR string and checks if it matches the expected IP version.
+func validateCIDR(cidrStr string, ipVersion string) error {
+	_, err := parseAndValidateCIDR(cidrStr, ipVersion)
+	return err
+}

internal/servers/private_security_groups_server.go

@@ -341,15 +341,15 @@ func validateSecurityRule(rule *privatev1.SecurityRule, ruleType string, index i
 
 	// Validate IPv4 CIDR format if present
 	if rule.GetIpv4Cidr() != "" {
-		if err := validateCIDR(rule.GetIpv4Cidr(), "IPv4"); err != nil {
+		if err := validateCIDR(rule.GetIpv4Cidr(), cidrIPv4); err != nil {
 			return grpcstatus.Errorf(grpccodes.InvalidArgument,
 				"%s rule at index %d: invalid IPv4 CIDR: %v", ruleType, index, err)
 		}
 	}
 
 	// Validate IPv6 CIDR format if present
 	if rule.GetIpv6Cidr() != "" {
-		if err := validateCIDR(rule.GetIpv6Cidr(), "IPv6"); err != nil {
+		if err := validateCIDR(rule.GetIpv6Cidr(), cidrIPv6); err != nil {
 			return grpcstatus.Errorf(grpccodes.InvalidArgument,
 				"%s rule at index %d: invalid IPv6 CIDR: %v", ruleType, index, err)
 		}

internal/servers/private_subnets_server.go

@@ -228,16 +228,26 @@ func (s *PrivateSubnetsServer) validateSubnet(ctx context.Context,
 
 	// SUB-VAL-01: Validate IPv4 CIDR format
 	if spec.GetIpv4Cidr() != "" {
-		if err := validateCIDR(spec.GetIpv4Cidr(), "IPv4"); err != nil {
+		canonical, err := parseAndValidateCIDR(spec.GetIpv4Cidr(), cidrIPv4)
+		if err != nil {
 			return err
 		}
+		// Canonicalize on Create only; Update must not rewrite immutable CIDR fields (SUB-VAL-11+).
+		if existingSubnet == nil {
+			spec.SetIpv4Cidr(canonical)
+		}
 	}
 
 	// SUB-VAL-02: Validate IPv6 CIDR format
 	if spec.GetIpv6Cidr() != "" {
-		if err := validateCIDR(spec.GetIpv6Cidr(), "IPv6"); err != nil {
+		canonical, err := parseAndValidateCIDR(spec.GetIpv6Cidr(), cidrIPv6)
+		if err != nil {
 			return err
 		}
+		// Canonicalize on Create only; Update must not rewrite immutable CIDR fields (SUB-VAL-11+).
+		if existingSubnet == nil {
+			spec.SetIpv6Cidr(canonical)
+		}
 	}
 
 	// SUB-VAL-04, SUB-VAL-05, SUB-VAL-06, SUB-VAL-07, SUB-VAL-08: Validate parent VirtualNetwork
@@ -331,7 +341,7 @@ func (s *PrivateSubnetsServer) validateVirtualNetworkReference(ctx context.Conte
 				"subnet has IPv4 CIDR but parent VirtualNetwork does not support IPv4")
 		}
 		// SUB-VAL-06: Validate IPv4 CIDR is subset of parent
-		if err := validateCIDRSubset(spec.GetIpv4Cidr(), parentSpec.GetIpv4Cidr(), "IPv4"); err != nil {
+		if err := validateCIDRSubset(spec.GetIpv4Cidr(), parentSpec.GetIpv4Cidr(), cidrIPv4); err != nil {
 			return err
 		}
 	}
@@ -343,7 +353,7 @@ func (s *PrivateSubnetsServer) validateVirtualNetworkReference(ctx context.Conte
 				"subnet has IPv6 CIDR but parent VirtualNetwork does not support IPv6")
 		}
 		// SUB-VAL-06: Validate IPv6 CIDR is subset of parent
-		if err := validateCIDRSubset(spec.GetIpv6Cidr(), parentSpec.GetIpv6Cidr(), "IPv6"); err != nil {
+		if err := validateCIDRSubset(spec.GetIpv6Cidr(), parentSpec.GetIpv6Cidr(), cidrIPv6); err != nil {
 			return err
 		}
 	}

internal/servers/private_subnets_server_test.go

@@ -308,6 +308,75 @@ var _ = Describe("Private subnets server", func() {
 			})
 		})
 
+		Context("SUB-VAL-01: IPv4 CIDR canonicalization on Create", func() {
+			It("canonicalizes non-canonical IPv4 CIDR on Create", func() {
+				vn := createVirtualNetwork(ctx, "10.0.0.0/16", "")
+
+				subnet := privatev1.Subnet_builder{
+					Spec: privatev1.SubnetSpec_builder{
+						Ipv4Cidr:       new("10.0.1.5/24"),
+						VirtualNetwork: vn.GetId(),
+					}.Build(),
+				}.Build()
+
+				err := server.validateSubnet(ctx, subnet, nil)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(subnet.GetSpec().GetIpv4Cidr()).To(Equal("10.0.1.0/24"))
+			})
+
+			It("accepts non-canonical subnet CIDR within legacy non-canonical parent", func() {
+				vn := createVirtualNetwork(ctx, "10.0.0.5/16", "")
+
+				subnet := privatev1.Subnet_builder{
+					Spec: privatev1.SubnetSpec_builder{
+						Ipv4Cidr:       new("10.0.1.5/24"),
+						VirtualNetwork: vn.GetId(),
+					}.Build(),
+				}.Build()
+
+				err := server.validateSubnet(ctx, subnet, nil)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(subnet.GetSpec().GetIpv4Cidr()).To(Equal("10.0.1.0/24"))
+			})
+		})
+
+		Context("SUB-VAL-02: IPv6 CIDR canonicalization on Create", func() {
+			It("canonicalizes non-canonical IPv6 CIDR on Create", func() {
+				vn := createVirtualNetwork(ctx, "", "2001:db8::/32")
+
+				subnet := privatev1.Subnet_builder{
+					Spec: privatev1.SubnetSpec_builder{
+						Ipv6Cidr:       new("2001:db8:1::1/64"),
+						VirtualNetwork: vn.GetId(),
+					}.Build(),
+				}.Build()
+
+				err := server.validateSubnet(ctx, subnet, nil)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(subnet.GetSpec().GetIpv6Cidr()).To(Equal("2001:db8:1::/64"))
+			})
+		})
+
+		Context("Round-trip: server.Create canonicalizes subnet CIDRs", func() {
+			It("stores canonical IPv4 CIDR on Create", func() {
+				vn := createVirtualNetwork(ctx, "10.0.0.0/16", "")
+
+				createResponse, err := server.Create(ctx, privatev1.SubnetsCreateRequest_builder{
+					Object: privatev1.Subnet_builder{
+						Metadata: privatev1.Metadata_builder{
+							Tenant: auth.SharedTenant,
+						}.Build(),
+						Spec: privatev1.SubnetSpec_builder{
+							Ipv4Cidr:       new("10.0.1.5/24"),
+							VirtualNetwork: vn.GetId(),
+						}.Build(),
+					}.Build(),
+				}.Build())
+				Expect(err).ToNot(HaveOccurred())
+				Expect(createResponse.GetObject().GetSpec().GetIpv4Cidr()).To(Equal("10.0.1.0/24"))
+			})
+		})
+
 		Context("SUB-VAL-03: At least one CIDR required", func() {
 			It("rejects both IPv4 and IPv6 CIDRs empty", func() {
 				vn := createVirtualNetwork(ctx, "10.0.0.0/16", "2001:db8::/32")

internal/servers/private_virtual_networks_server.go

@@ -18,7 +18,6 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
-	"net/netip"
 
 	"github.com/prometheus/client_golang/prometheus"
 	grpccodes "google.golang.org/grpc/codes"
@@ -236,16 +235,28 @@ func (s *PrivateVirtualNetworksServer) validateVirtualNetwork(ctx context.Contex
 
 	// VN-VAL-01: Validate IPv4 CIDR format
 	if spec.GetIpv4Cidr() != "" {
-		if err = validateCIDR(spec.GetIpv4Cidr(), "IPv4"); err != nil {
+		var canonical string
+		canonical, err = parseAndValidateCIDR(spec.GetIpv4Cidr(), cidrIPv4)
+		if err != nil {
 			return
 		}
+		// Canonicalize on Create only; Update must not rewrite immutable CIDR fields (VN-VAL-11/12).
+		if existingVN == nil {
+			spec.SetIpv4Cidr(canonical)
+		}
 	}
 
 	// VN-VAL-02: Validate IPv6 CIDR format
 	if spec.GetIpv6Cidr() != "" {
-		if err = validateCIDR(spec.GetIpv6Cidr(), "IPv6"); err != nil {
+		var canonical string
+		canonical, err = parseAndValidateCIDR(spec.GetIpv6Cidr(), cidrIPv6)
+		if err != nil {
 			return
 		}
+		// Canonicalize on Create only; Update must not rewrite immutable CIDR fields (VN-VAL-11/12).
+		if existingVN == nil {
+			spec.SetIpv6Cidr(canonical)
+		}
 	}
 
 	// VN-VAL-04, VN-VAL-05, VN-VAL-06: Validate NetworkClass
@@ -261,28 +272,6 @@ func (s *PrivateVirtualNetworksServer) validateVirtualNetwork(ctx context.Contex
 	return
 }
 
-// validateCIDR validates a CIDR string and checks if it matches the expected IP version.
-func validateCIDR(cidrStr string, ipVersion string) error {
-	prefix, err := netip.ParsePrefix(cidrStr)
-	if err != nil {
-		return grpcstatus.Errorf(grpccodes.InvalidArgument,
-			"invalid %s CIDR format '%s': %v", ipVersion, cidrStr, err)
-	}
-
-	// Validate IP version matches field name
-	isIPv4 := prefix.Addr().Is4()
-	if ipVersion == "IPv4" && !isIPv4 {
-		return grpcstatus.Errorf(grpccodes.InvalidArgument,
-			"field 'ipv4_cidr' contains IPv6 address: %s", cidrStr)
-	}
-	if ipVersion == "IPv6" && isIPv4 {
-		return grpcstatus.Errorf(grpccodes.InvalidArgument,
-			"field 'ipv6_cidr' contains IPv4 address: %s", cidrStr)
-	}
-
-	return nil
-}
-
 // validateImmutableFields validates that immutable fields have not been changed.
 func validateImmutableFields(newVN *privatev1.VirtualNetwork, existingVN *privatev1.VirtualNetwork) error {
 	if existingVN == nil {

internal/servers/private_virtual_networks_server_test.go

@@ -276,6 +276,89 @@ var _ = Describe("Private virtual networks server", func() {
 			})
 		})
 
+		Context("VN-VAL-01: IPv4 CIDR canonicalization on Create", func() {
+			It("canonicalizes non-canonical IPv4 CIDR on Create", func() {
+				nc := createNetworkClass(ctx, privatev1.NetworkClassState_NETWORK_CLASS_STATE_READY)
+
+				vn := privatev1.VirtualNetwork_builder{
+					Spec: privatev1.VirtualNetworkSpec_builder{
+						Ipv4Cidr:     new("10.0.1.5/24"),
+						NetworkClass: nc.GetId(),
+						Region:       "us-west-1",
+					}.Build(),
+				}.Build()
+
+				_, err := server.validateVirtualNetwork(ctx, vn, nil)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(vn.GetSpec().GetIpv4Cidr()).To(Equal("10.0.1.0/24"))
+			})
+		})
+
+		Context("VN-VAL-02: IPv6 CIDR canonicalization on Create", func() {
+			It("canonicalizes non-canonical IPv6 CIDR on Create", func() {
+				nc := createNetworkClass(ctx, privatev1.NetworkClassState_NETWORK_CLASS_STATE_READY)
+
+				vn := privatev1.VirtualNetwork_builder{
+					Spec: privatev1.VirtualNetworkSpec_builder{
+						Ipv6Cidr:     new("2001:db8::1/32"),
+						NetworkClass: nc.GetId(),
+						Region:       "us-west-1",
+					}.Build(),
+				}.Build()
+
+				_, err := server.validateVirtualNetwork(ctx, vn, nil)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(vn.GetSpec().GetIpv6Cidr()).To(Equal("2001:db8::/32"))
+			})
+		})
+
+		Context("Round-trip: server.Create canonicalizes CIDRs", func() {
+			It("stores canonical IPv4 CIDR on Create", func() {
+				nc := createNetworkClass(ctx, privatev1.NetworkClassState_NETWORK_CLASS_STATE_READY)
+
+				createResponse, err := server.Create(ctx, privatev1.VirtualNetworksCreateRequest_builder{
+					Object: privatev1.VirtualNetwork_builder{
+						Metadata: privatev1.Metadata_builder{
+							Tenant: auth.SharedTenant,
+						}.Build(),
+						Spec: privatev1.VirtualNetworkSpec_builder{
+							Ipv4Cidr:     new("10.0.1.5/24"),
+							NetworkClass: nc.GetId(),
+							Region:       "us-west-1",
+						}.Build(),
+					}.Build(),
+				}.Build())
+				Expect(err).ToNot(HaveOccurred())
+				Expect(createResponse.GetObject().GetSpec().GetIpv4Cidr()).To(Equal("10.0.1.0/24"))
+			})
+		})
+
+		Context("Create-only: no canonicalization on Update", func() {
+			It("does not canonicalize ipv4_cidr on Update", func() {
+				nc := createNetworkClass(ctx, privatev1.NetworkClassState_NETWORK_CLASS_STATE_READY)
+
+				existing := privatev1.VirtualNetwork_builder{
+					Spec: privatev1.VirtualNetworkSpec_builder{
+						Region:       "us-west-1",
+						NetworkClass: nc.GetId(),
+						Ipv4Cidr:     new("10.0.1.5/24"),
+					}.Build(),
+				}.Build()
+
+				updated := privatev1.VirtualNetwork_builder{
+					Spec: privatev1.VirtualNetworkSpec_builder{
+						Region:       "us-west-1",
+						NetworkClass: nc.GetId(),
+						Ipv4Cidr:     new("10.0.1.5/24"),
+					}.Build(),
+				}.Build()
+
+				_, err := server.validateVirtualNetwork(ctx, updated, existing)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(updated.GetSpec().GetIpv4Cidr()).To(Equal("10.0.1.5/24"))
+			})
+		})
+
 		Context("VN-VAL-03: At least one CIDR required", func() {
 			It("rejects empty IPv4 and IPv6 CIDRs", func() {
 				vn := privatev1.VirtualNetwork_builder{