Skip to content

Commit

Permalink
Improve API and Ingress VIPs validation
Browse files Browse the repository at this point in the history 
• This commit enhances VIPs validation to ensure the primary IP
family of the VIPs matches the primary IP family of all the network fields.
• Code repurposed from prior closed PR (#7504)

Co-Authored-By: Maysa Macedo <[email protected]>
Co-Authored-By: Danny Kokkinos <[email protected]>
  • Loading branch information
@dkokkino @MaysaMacedo
dkokkino and MaysaMacedo committed Feb 7, 2025
1 parent 6fec4c8 commit 0ecf1c7
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 15 deletions.
36 changes: 26 additions & 10 deletions pkg/types/validation/installconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,19 +870,27 @@ func validateAPIAndIngressVIPs(vips vips, fieldNames vipFields, vipIsRequired, r
allErrs = append(allErrs, field.Required(fldPath.Child(fieldNames.IngressVIPs), "must specify VIP for ingress, when VIP for API is set"))
}

if len(vips.API) == 1 {
if len(vips.API) >= 1 {
hasIPv4, hasIPv6, presence, _ := inferIPVersionFromInstallConfig(n)

apiVIPIPFamily := corev1.IPv4Protocol
if utilsnet.IsIPv6String(vips.API[0]) {
apiVIPIPFamily = corev1.IPv6Protocol
}

if hasIPv4 && hasIPv6 && apiVIPIPFamily != presence["machineNetwork"].Primary {
allErrs = append(allErrs, field.Invalid(fldPath.Child(fieldNames.APIVIPs), vips.API[0], "VIP for the API must be of the same IP family with machine network's primary IP Family for dual-stack IPv4/IPv6"))
if hasIPv4 && hasIPv6 {
for k, v := range presence {
if v.Primary != apiVIPIPFamily {
allErrs = append(allErrs, field.Invalid(fldPath.Child(fieldNames.APIVIPs), vips.API[0], fmt.Sprintf("%s primary IP Family and primary IP family for the API VIP should match", k)))
}
}
}
} else if len(vips.API) == 2 {
if isDualStack, _ := utilsnet.IsDualStackIPStrings(vips.API); !isDualStack {
}
if len(vips.API) == 2 {
if isDualStack, err := utilsnet.IsDualStackIPStrings(vips.API); !isDualStack {
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, vips, err.Error()))
}
allErrs = append(allErrs, field.Invalid(fldPath.Child(fieldNames.APIVIPs), vips.API, "If two API VIPs are given, one must be an IPv4 address, the other an IPv6"))
}
}
Expand Down Expand Up @@ -913,19 +921,27 @@ func validateAPIAndIngressVIPs(vips vips, fieldNames vipFields, vipIsRequired, r
allErrs = append(allErrs, field.Required(fldPath.Child(fieldNames.APIVIPs), "must specify VIP for API, when VIP for ingress is set"))
}

if len(vips.Ingress) == 1 {
if len(vips.Ingress) >= 1 {
hasIPv4, hasIPv6, presence, _ := inferIPVersionFromInstallConfig(n)

ingressVIPIPFamily := corev1.IPv4Protocol
if utilsnet.IsIPv6String(vips.Ingress[0]) {
ingressVIPIPFamily = corev1.IPv6Protocol
}

if hasIPv4 && hasIPv6 && ingressVIPIPFamily != presence["machineNetwork"].Primary {
allErrs = append(allErrs, field.Invalid(fldPath.Child(fieldNames.IngressVIPs), vips.Ingress[0], "VIP for the Ingress must be of the same IP family with machine network's primary IP Family for dual-stack IPv4/IPv6"))
if hasIPv4 && hasIPv6 {
for k, v := range presence {
if v.Primary != ingressVIPIPFamily {
allErrs = append(allErrs, field.Invalid(fldPath.Child(fieldNames.IngressVIPs), vips.Ingress[0], fmt.Sprintf("%s primary IP Family and primary IP family for the Ingress VIP should match", k)))
}
}
}
} else if len(vips.Ingress) == 2 {
if isDualStack, _ := utilsnet.IsDualStackIPStrings(vips.Ingress); !isDualStack {
}
if len(vips.Ingress) == 2 {
if isDualStack, err := utilsnet.IsDualStackIPStrings(vips.Ingress); !isDualStack {
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, vips, err.Error()))
}
allErrs = append(allErrs, field.Invalid(fldPath.Child(fieldNames.IngressVIPs), vips.Ingress, "If two Ingress VIPs are given, one must be an IPv4 address, the other an IPv6"))
}
}
Expand Down
36 changes: 31 additions & 5 deletions pkg/types/validation/installconfig_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,33 @@ func validDualStackNetworkingConfig() *types.Networking {
},
}
}

func InvalidPrimaryV6DualStackNetworkingConfig() *types.Networking {
return &types.Networking{
NetworkType: "OVNKubernetes",
MachineNetwork: []types.MachineNetworkEntry{
{
CIDR: *ipnet.MustParseCIDR("ffd0::/48"),
},
{
CIDR: *ipnet.MustParseCIDR("10.0.0.0/16"),
},
},
ServiceNetwork: []ipnet.IPNet{
*ipnet.MustParseCIDR("172.30.0.0/16"),
*ipnet.MustParseCIDR("ffd1::/112"),
},
ClusterNetwork: []types.ClusterNetworkEntry{
{
CIDR: *ipnet.MustParseCIDR("ffd2::/48"),
HostPrefix: 64,
},
{
CIDR: *ipnet.MustParseCIDR("192.168.1.0/24"),
HostPrefix: 28,
},
},
}
}
func TestValidateInstallConfig(t *testing.T) {
cases := []struct {
name string
Expand Down Expand Up @@ -2109,27 +2135,27 @@ func TestValidateInstallConfig(t *testing.T) {
name: "baremetal API VIP set to an incorrect IP Family",
installConfig: func() *types.InstallConfig {
c := validInstallConfig()
c.Networking = validDualStackNetworkingConfig()
c.Networking = InvalidPrimaryV6DualStackNetworkingConfig()
c.Platform = types.Platform{
BareMetal: validBareMetalPlatform(),
}
c.Platform.BareMetal.APIVIPs = []string{"ffd0::"}
return c
}(),
expectedError: `platform.baremetal.apiVIPs: Invalid value: "ffd0::": VIP for the API must be of the same IP family with machine network's primary IP Family for dual-stack IPv4/IPv6`,
expectedError: `[platform.baremetal.apiVIPs: Invalid value: "ffd0::": serviceNetwork primary IP Family and primary IP family for the API VIP should match]`,
},
{
name: "baremetal Ingress VIP set to an incorrect IP Family",
installConfig: func() *types.InstallConfig {
c := validInstallConfig()
c.Networking = validDualStackNetworkingConfig()
c.Networking = InvalidPrimaryV6DualStackNetworkingConfig()
c.Platform = types.Platform{
BareMetal: validBareMetalPlatform(),
}
c.Platform.BareMetal.IngressVIPs = []string{"ffd0::"}
return c
}(),
expectedError: `platform.baremetal.ingressVIPs: Invalid value: "ffd0::": VIP for the Ingress must be of the same IP family with machine network's primary IP Family for dual-stack IPv4/IPv6`,
expectedError: `[platform.baremetal.ingressVIPs: Invalid value: "ffd0::": serviceNetwork primary IP Family and primary IP family for the Ingress VIP should match]`,
},
{
name: "should validate vips on baremetal (required)",
Expand Down

0 comments on commit 0ecf1c7

Please sign in to comment.