Skip to content

Commit

Permalink
OCPBUGS-16189: Implemented primary IP Network stack validations
Browse files Browse the repository at this point in the history 
- Implemented a network validation mechanism to ensure proper
  correlation among the ClusterNetwork, ServiceNetwork, and
  MachineNetwork by utilising the same network stack.

Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
  • Loading branch information
@jparrill
jparrill committed Oct 13, 2023
1 parent 836cd27 commit f70d254
Show file tree
Hide file tree
Showing 2 changed files with 179 additions and 1 deletion.
60 changes: 60 additions & 0 deletions hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3977,6 +3977,7 @@ func (r *HostedClusterReconciler) validateHostedClusterSupport(hc *hyperv1.Hoste

func (r *HostedClusterReconciler) validateNetworks(hc *hyperv1.HostedCluster) error {
var errs field.ErrorList
errs = append(errs, validateNetworkStackAddresses(hc)...)
errs = append(errs, validateSliceNetworkCIDRs(hc)...)
errs = append(errs, checkAdvertiseAddressOverlapping(hc)...)

Expand Down Expand Up @@ -4019,6 +4020,65 @@ func findAdvertiseAddress(hc *hyperv1.HostedCluster) (net.IP, *field.Error) {
return advertiseAddress, nil
}

// validateNetworkStackAddresses validates that Networks defined in the HostedCluster are in the same network stack
// between each other against the primary IP using ClusterNetwork as a base.
func validateNetworkStackAddresses(hc *hyperv1.HostedCluster) field.ErrorList {
var errs field.ErrorList
ipv4 := make([]string, 0)
ipv6 := make([]string, 0)

networks := make(map[string]string, 0)

if len(hc.Spec.Networking.ClusterNetwork) > 0 {
networks["spec.networking.ClusterNetwork"] = hc.Spec.Networking.ClusterNetwork[0].CIDR.IP.String()
}

if len(hc.Spec.Networking.ServiceNetwork) > 0 {
networks["spec.networking.ServiceNetwork"] = hc.Spec.Networking.ServiceNetwork[0].CIDR.IP.String()
}

if len(hc.Spec.Networking.MachineNetwork) > 0 {
networks["spec.networking.MachineNetwork"] = hc.Spec.Networking.MachineNetwork[0].CIDR.IP.String()
}

advAddr, err := findAdvertiseAddress(hc)
if err != nil {
errs = append(errs, err)
}

networks["spec.networking.APIServerNetworking.AdvertiseAddress"] = advAddr.String()

for fieldpath, ipaddr := range networks {
checkIP := net.ParseIP(ipaddr)

if checkIP != nil && strings.Contains(ipaddr, ".") {
ipv4 = append(ipv4, ipaddr)
}

if checkIP != nil && strings.Contains(ipaddr, ":") {
ipv6 = append(ipv6, ipaddr)
}

// This check ensures that the IPv6 and IPv4 is a valid ip
if checkIP == nil {
errs = append(errs, field.Invalid(field.NewPath(fieldpath),
k8sutilspointer.String(ipaddr),
fmt.Sprintf("error checking network stack of %s with ip %s", fieldpath, ipaddr),
))
}
}

if len(ipv4) > 0 && len(ipv6) > 0 {
// Invalid result, means that there are mixed stacks in the primary position of the stack
errs = append(errs, field.Forbidden(field.NewPath("spec.networking"),
fmt.Sprintf("declare multiple network stacks as primary network in the cluster definition is not allowed, ipv4: %v, ipv6: %v", ipv4, ipv6),
))
}

return errs

}

// checkAdvertiseAddressOverlapping validates that the AdvertiseAddress defined does not overlap with
// the ClusterNetwork, ServiceNetwork and MachineNetwork
func checkAdvertiseAddressOverlapping(hc *hyperv1.HostedCluster) field.ErrorList {
Expand Down
120 changes: 119 additions & 1 deletion hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,8 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
cluster.Spec.PullSecret = corev1.LocalObjectReference{Name: "secret"}
cluster.Spec.InfraID = "infra-id"
cluster.Spec.Networking.ClusterNetwork = []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.1.0/24")}}
cluster.Spec.Networking.MachineNetwork = []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}}
cluster.Spec.Networking.ServiceNetwork = []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}}
objects = append(objects, cluster)
}

Expand Down Expand Up @@ -1148,7 +1150,11 @@ func TestValidateConfigAndClusterCapabilities(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster-4.14",
},
Spec: hyperv1.HostedClusterSpec{},
Spec: hyperv1.HostedClusterSpec{
Networking: hyperv1.ClusterNetworking{
ClusterNetwork: clusterNet,
},
},
},
expectedResult: errors.New(`hostedcluster name failed RFC1123 validation: a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')`),
},
Expand Down Expand Up @@ -3449,3 +3455,115 @@ func TestFindAdvertiseAddress(t *testing.T) {
})
}
}

func TestValidateNetworkStackAddresses(t *testing.T) {
tests := []struct {
name string
cn []hyperv1.ClusterNetworkEntry
mn []hyperv1.MachineNetworkEntry
sn []hyperv1.ServiceNetworkEntry
aa *hyperv1.APIServerNetworking
wantErr bool
}{
{
name: "given an IPv6 clusterNetwork and an IPv4 ServiceNetwork, it should fail",
aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("fd03::1")},
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd03::/64")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}},
wantErr: true,
},
{
name: "on IPv6 and IPv4 Advertise Address, it should fail",
aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("192.168.1.1")},
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd01::/64")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}},
wantErr: true,
},
{
name: "on IPv6 and defining Advertise Address, it should success",
aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("fd03::1")},
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd01::/64")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}},
wantErr: false,
},
{
name: "given an IPv4 clusterNetwork and an IPv6 ServiceNetwork, it should fail",
aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("192.168.1.1")},
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/16")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}},
wantErr: true,
},
{
name: "on IPv4 and defining IPv6 Advertise Address, it should fail",
aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("fd03::1")},
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.1.0/24")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}},
wantErr: true,
},
{
name: "on IPv4 and defining Advertise Address, it should success",
aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("192.168.1.1")},
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.0.0/24")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.1.0/24")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}},
wantErr: false,
},
{
name: "on IPv4, it should success",
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.1.0/24")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}},
wantErr: false,
},
{
name: "on IPv6, it should success",
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd01::/64")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}},
wantErr: false,
},
{
name: "given an IPv4 invalid advertise address, it should fail",
aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("192.168.1.1.2")},
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.1.0/24")}},
wantErr: true,
},
{
name: "given an IPv6 invalid advertise address, it should fail",
aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("fd03::1::32")},
mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}},
cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd03::/64")}},
sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
hc := &hyperv1.HostedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "hc",
Namespace: "any",
},
Spec: hyperv1.HostedClusterSpec{
Networking: hyperv1.ClusterNetworking{
ClusterNetwork: tt.cn,
ServiceNetwork: tt.sn,
MachineNetwork: tt.mn,
APIServer: tt.aa,
},
},
}
err := validateNetworkStackAddresses(hc)
if (err != nil) != tt.wantErr {
t.Errorf("validateNetworkStackAddresses() wantErr %v, err %v", tt.wantErr, err)
}
})
}
}

0 comments on commit f70d254

Please sign in to comment.