diff --git a/pkg/asset/tls/utils_test.go b/pkg/asset/tls/utils_test.go index 8eb13c9ccef..fecdfab1deb 100644 --- a/pkg/asset/tls/utils_test.go +++ b/pkg/asset/tls/utils_test.go @@ -1,4 +1,4 @@ -package tls +package tls //nolint:revive // pre-existing package name import ( "crypto/ecdsa" @@ -12,6 +12,15 @@ import ( "github.com/openshift/installer/pkg/types" ) +func marshalPKCS8(t *testing.T, key interface{}) []byte { + t.Helper() + der, err := x509.MarshalPKCS8PrivateKey(key) + if !assert.NoError(t, err, "failed to marshal key to PKCS#8") { + return nil + } + return der +} + func TestPrivateKeyToPemRoundtrip(t *testing.T) { cases := []struct { name string @@ -44,15 +53,21 @@ func TestPrivateKeyToPemRoundtrip(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { key, err := tc.genFunc() - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } pemBytes, err := PrivateKeyToPem(key) - assert.NoError(t, err) - assert.NotEmpty(t, pemBytes) + if !assert.NoError(t, err) || !assert.NotEmpty(t, pemBytes) { + return + } decoded, err := PemToPrivateKey(pemBytes) - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } assert.IsType(t, tc.expectType, decoded) + assert.Equal(t, marshalPKCS8(t, key), marshalPKCS8(t, decoded), "round-tripped key material must match original") }) } } @@ -70,40 +85,58 @@ func TestPemToPrivateKeyFormats(t *testing.T) { t.Run("RSA PEM block", func(t *testing.T) { key, err := GenerateRSAPrivateKey(2048) - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } pemBytes, pemErr := PrivateKeyToPem(key) - assert.NoError(t, pemErr) + if !assert.NoError(t, pemErr) { + return + } decoded, err := PemToPrivateKey(pemBytes) - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } _, ok := decoded.(*rsa.PrivateKey) assert.True(t, ok, "expected *rsa.PrivateKey") }) t.Run("EC PEM block", func(t *testing.T) { key, err := GenerateECDSAPrivateKey(types.ECDSACurveP256) - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } pemBytes, pemErr := PrivateKeyToPem(key) - assert.NoError(t, pemErr) + if !assert.NoError(t, pemErr) { + return + } decoded, err := PemToPrivateKey(pemBytes) - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } _, ok := decoded.(*ecdsa.PrivateKey) assert.True(t, ok, "expected *ecdsa.PrivateKey") }) t.Run("PKCS#8 PEM block", func(t *testing.T) { key, err := GenerateRSAPrivateKey(2048) - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } pkcs8Bytes, err := x509.MarshalPKCS8PrivateKey(key) - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } pemBytes := pem.EncodeToMemory(&pem.Block{ Type: "PRIVATE KEY", Bytes: pkcs8Bytes, }) decoded, err := PemToPrivateKey(pemBytes) - assert.NoError(t, err) + if !assert.NoError(t, err) { + return + } _, ok := decoded.(*rsa.PrivateKey) assert.True(t, ok, "expected *rsa.PrivateKey") }) diff --git a/pkg/types/azure/validation/platform.go b/pkg/types/azure/validation/platform.go index 2827a414e0d..c375cd0d0bc 100644 --- a/pkg/types/azure/validation/platform.go +++ b/pkg/types/azure/validation/platform.go @@ -9,6 +9,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" capz "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/azure" "github.com/openshift/installer/pkg/types/network" @@ -162,6 +163,7 @@ func ValidatePlatform(p *azure.Platform, publish types.PublishingStrategy, fldPa } allErrs = append(allErrs, validateIPFamily(p.IPFamily, fldPath.Child("ipFamily"))...) + allErrs = append(allErrs, validateDualStackMachineNetworks(ic, p.IPFamily)...) if p.CloudName == azure.StackCloud && p.AllowSharedKeyAccess != nil && !*p.AllowSharedKeyAccess { allErrs = append(allErrs, field.Invalid(fldPath.Child("allowSharedAccessKey"), p.AllowSharedKeyAccess, "disabling shared access key creation is unsupported in Azure stack hub")) @@ -189,6 +191,95 @@ func validateIPFamily(ipFamily network.IPFamily, fldPath *field.Path) field.Erro return allErrs } +// validateDualStackMachineNetworks validates Azure IPv6 networking configuration. +// Azure does not support single-stack IPv6 (IPv6-only). IPv6 can only be used in dual-stack mode with IPv4. +// Additionally, Azure's subnet splitting logic requires /64 subnets for IPv6, so the parent CIDR must have a broader prefix (prefix length less than /64). +func validateDualStackMachineNetworks(ic *types.InstallConfig, ipFamily network.IPFamily) field.ErrorList { + if ic == nil || ic.Networking == nil { + return field.ErrorList{} + } + + fldPath := field.NewPath("networking") + var allErrs field.ErrorList + + machineNetworks := make([]ipnet.IPNet, len(ic.MachineNetwork)) + for i, mn := range ic.MachineNetwork { + machineNetworks[i] = mn.CIDR + } + + if len(machineNetworks) == 0 { + return allErrs + } + + hasIPv4 := false + hasIPv6 := false + ipv6Indices := []int{} + ipv6TooLongIndices := []int{} + ipv6NotNibbleBoundaryIndices := []int{} + + for i, machineNetwork := range machineNetworks { + ip := machineNetwork.IP + if len(ip) == 0 { + continue + } + + if ip.To4() != nil { + hasIPv4 = true + } else { + hasIPv6 = true + ipv6Indices = append(ipv6Indices, i) + + prefixLen, _ := machineNetwork.Mask.Size() + if prefixLen >= 64 { + ipv6TooLongIndices = append(ipv6TooLongIndices, i) + } else if prefixLen%4 != 0 { + ipv6NotNibbleBoundaryIndices = append(ipv6NotNibbleBoundaryIndices, i) + } + } + } + + if hasIPv6 && !hasIPv4 { + for _, i := range ipv6Indices { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("machineNetwork").Index(i).Child("cidr"), + machineNetworks[i].String(), + "single-stack IPv6 is not supported on Azure. IPv6 may only be used with dual-stack networking (both IPv4 and IPv6)", + )) + } + return allErrs + } + + if ipFamily.DualStackEnabled() && !hasIPv6 { + allErrs = append(allErrs, field.Required( + fldPath.Child("machineNetwork"), + "at least one IPv6 machine network must be specified when dual-stack is enabled", + )) + return allErrs + } + + if len(ipv6NotNibbleBoundaryIndices) > 0 { + for _, i := range ipv6NotNibbleBoundaryIndices { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("machineNetwork").Index(i).Child("cidr"), + machineNetworks[i].String(), + "IPv6 CIDR prefix length must be on a nibble boundary (multiples of 4). Valid prefixes less than /64 are /48, /52, /56, /60", + )) + } + } + + if hasIPv4 && len(ipv6TooLongIndices) > 0 { + for _, i := range ipv6TooLongIndices { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("machineNetwork").Index(i).Child("cidr"), + machineNetworks[i].String(), + "in dual-stack configurations, IPv6 machine network CIDRs require prefix lengths shorter than /64 on nibble boundaries (e.g., /48, /52, /56, /60). Azure recommends /56 to allow splitting into multiple /64 subnets", + )) + } + } + + return allErrs +} + // validateCustomerManagedKeys validates the key vault id. func validateCustomerManagedKeys(cloudName azure.CloudEnvironment, s azure.CustomerManagedKey, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList diff --git a/pkg/types/azure/validation/platform_test.go b/pkg/types/azure/validation/platform_test.go index 2b638256179..4542bdf6a80 100644 --- a/pkg/types/azure/validation/platform_test.go +++ b/pkg/types/azure/validation/platform_test.go @@ -7,6 +7,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/azure" "github.com/openshift/installer/pkg/types/network" @@ -55,6 +56,7 @@ func TestValidatePlatform(t *testing.T) { cases := []struct { name string platform *azure.Platform + ic *types.InstallConfig wantSkip func(p *azure.Platform) bool expected string }{ @@ -289,6 +291,140 @@ func TestValidatePlatform(t *testing.T) { }(), expected: `^test-path\.userProvisionedDNS: Invalid value: "Enabled": userProvisionedDNS is not supported on Azure Stack Hub$`, }, + { + name: "no machine networks specified", + platform: validPlatform(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{}, + }, + }, + expected: "", + }, + { + name: "invalid single-stack IPv6 with /64", + platform: validPlatform(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("fd00::/64")}, + }, + }, + }, + expected: `^\Qnetworking.machineNetwork[0].cidr: Invalid value: "fd00::/64": single-stack IPv6 is not supported on Azure. IPv6 may only be used with dual-stack networking (both IPv4 and IPv6)\E$`, + }, + { + name: "invalid single-stack IPv6 with /56", + platform: validPlatform(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("fd00::/56")}, + }, + }, + }, + expected: `^\Qnetworking.machineNetwork[0].cidr: Invalid value: "fd00::/56": single-stack IPv6 is not supported on Azure. IPv6 may only be used with dual-stack networking (both IPv4 and IPv6)\E$`, + }, + { + name: "dual-stack enabled but missing IPv4", + platform: func() *azure.Platform { + p := validPlatform() + p.IPFamily = network.DualStackIPv4Primary + return p + }(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("fd00::/56")}, + }, + }, + }, + expected: `^\Qnetworking.machineNetwork[0].cidr: Invalid value: "fd00::/56": single-stack IPv6 is not supported on Azure. IPv6 may only be used with dual-stack networking (both IPv4 and IPv6)\E$`, + }, + { + name: "dual-stack enabled but missing IPv6", + platform: func() *azure.Platform { + p := validPlatform() + p.IPFamily = network.DualStackIPv4Primary + return p + }(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")}, + }, + }, + }, + expected: `^\Qnetworking.machineNetwork: Required value: at least one IPv6 machine network must be specified when dual-stack is enabled\E$`, + }, + { + name: "dual-stack IPv6 primary enabled but missing IPv6", + platform: func() *azure.Platform { + p := validPlatform() + p.IPFamily = network.DualStackIPv6Primary + return p + }(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")}, + }, + }, + }, + expected: `^\Qnetworking.machineNetwork: Required value: at least one IPv6 machine network must be specified when dual-stack is enabled\E$`, + }, + { + name: "valid dual-stack with IPv6 /56", + platform: validPlatform(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")}, + {CIDR: *ipnet.MustParseCIDR("fd00::/56")}, + }, + }, + }, + }, + { + name: "invalid IPv6 prefix not on nibble boundary /63", + platform: validPlatform(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")}, + {CIDR: *ipnet.MustParseCIDR("fd00::/63")}, + }, + }, + }, + expected: `^\Qnetworking.machineNetwork[1].cidr: Invalid value: "fd00::/63": IPv6 CIDR prefix length must be on a nibble boundary (multiples of 4). Valid prefixes less than /64 are /48, /52, /56, /60\E$`, + }, + { + name: "invalid dual-stack with IPv6 /64", + platform: validPlatform(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")}, + {CIDR: *ipnet.MustParseCIDR("fd00::/64")}, + }, + }, + }, + expected: `^\Qnetworking.machineNetwork[1].cidr: Invalid value: "fd00::/64": in dual-stack configurations, IPv6 machine network CIDRs require prefix lengths shorter than /64 on nibble boundaries (e.g., /48, /52, /56, /60). Azure recommends /56 to allow splitting into multiple /64 subnets\E$`, + }, + { + name: "invalid dual-stack with valid and invalid IPv6", + platform: validPlatform(), + ic: &types.InstallConfig{ + Networking: &types.Networking{ + MachineNetwork: []types.MachineNetworkEntry{ + {CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")}, + {CIDR: *ipnet.MustParseCIDR("fd00::/56")}, + {CIDR: *ipnet.MustParseCIDR("fd00::1/128")}, + }, + }, + }, + expected: `^\Qnetworking.machineNetwork[2].cidr: Invalid value: "fd00::1/128": in dual-stack configurations, IPv6 machine network CIDRs require prefix lengths shorter than /64 on nibble boundaries (e.g., /48, /52, /56, /60). Azure recommends /56 to allow splitting into multiple /64 subnets\E$`, + }, } ic := types.InstallConfig{} for _, tc := range cases { @@ -297,7 +433,11 @@ func TestValidatePlatform(t *testing.T) { t.Skip() } - err := ValidatePlatform(tc.platform, types.ExternalPublishingStrategy, field.NewPath("test-path"), &ic).ToAggregate() + testIC := tc.ic + if testIC == nil { + testIC = &ic + } + err := ValidatePlatform(tc.platform, types.ExternalPublishingStrategy, field.NewPath("test-path"), testIC).ToAggregate() if tc.expected == "" { assert.NoError(t, err) } else {