Skip to content

OCPBUGS-98102: Add validations Azure Dualstack #10677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sadasu wants to merge 2 commits into
base: main
Choose a base branch
from
+279 −15
Open

OCPBUGS-98102: Add validations Azure Dualstack #10677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
185c2c0
OCPBUGS-98102: Azure Dualstack: Validation for machine network prefix…
sadasu Jul 8, 2026
9462cd0
Fix linter error and associated code review comments
sadasu Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 47 additions & 14 deletions pkg/asset/tls/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tls
package tls //nolint:revive // pre-existing package name

import (
"crypto/ecdsa"
Expand All @@ -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
Expand Down Expand Up @@ -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")
})
}
}
Expand All @@ -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)
Comment on lines 91 to 96

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant test file and surrounding lines.
git ls-files 'pkg/asset/tls/utils_test.go'
wc -l pkg/asset/tls/utils_test.go
cat -n pkg/asset/tls/utils_test.go | sed -n '1,170p'

Repository: openshift/installer

Length of output: 4474

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the implementation behind the test helpers.
ast-grep outline pkg/asset/tls/utils.go --view expanded
printf '\n---\n'
cat -n pkg/asset/tls/utils.go | sed -n '1,220p'

Repository: openshift/installer

Length of output: 4478

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline pkg/asset/tls/utils.go --view expanded
printf '\n---\n'
cat -n pkg/asset/tls/utils.go | sed -n '1,220p'

Repository: openshift/installer

Length of output: 4478

Use explicit RSA and EC PEM encodings in these subtests. The RSA and EC cases still go through PrivateKeyToPem, so they only exercise the PKCS#8 PRIVATE KEY path. Marshal RSA with x509.MarshalPKCS1PrivateKey and EC with x509.MarshalECPrivateKey, and encode them as RSA PRIVATE KEY / EC PRIVATE KEY; keep PrivateKeyToPem for the PKCS#8 case.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/asset/tls/utils_test.go` around lines 91 - 96, Update the RSA and EC
subtests around PrivateKeyToPem to construct PEM data directly using
x509.MarshalPKCS1PrivateKey with an “RSA PRIVATE KEY” block and
x509.MarshalECPrivateKey with an “EC PRIVATE KEY” block. Retain PrivateKeyToPem
only in the PKCS#8 test case, then continue decoding each PEM value through
PemToPrivateKey.

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")
})
Expand Down
91 changes: 91 additions & 0 deletions pkg/types/azure/validation/platform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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 {
Comment thread
sadasu marked this conversation as resolved.
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
Expand Down
142 changes: 141 additions & 1 deletion pkg/types/azure/validation/platform_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}{
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down