diff --git a/api/v1alpha1/common_types.go b/api/v1alpha1/common_types.go index 25feff8b2..b09fdd27f 100644 --- a/api/v1alpha1/common_types.go +++ b/api/v1alpha1/common_types.go @@ -77,3 +77,15 @@ type LocalObjectReference struct { // +kubebuilder:validation:MinLength=1 Name string `json:"name"` } + +func (s ControlPlaneEndpointSpec) VirtualIPAddress() string { + // If specified, use the virtual IP address and/or port, + // otherwise fall back to the control plane endpoint host and port. + if s.VirtualIPSpec != nil && + s.VirtualIPSpec.Configuration != nil && + s.VirtualIPSpec.Configuration.Address != "" { + return s.VirtualIPSpec.Configuration.Address + } + + return s.Host +} diff --git a/api/v1alpha1/common_types_test.go b/api/v1alpha1/common_types_test.go new file mode 100644 index 000000000..657f3a746 --- /dev/null +++ b/api/v1alpha1/common_types_test.go @@ -0,0 +1,67 @@ +// Copyright 2023 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +package v1alpha1 + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestVirtualIPAddress(t *testing.T) { + tests := []struct { + name string + spec ControlPlaneEndpointSpec + expected string + }{ + { + name: "Virtual IP specified", + spec: ControlPlaneEndpointSpec{ + VirtualIPSpec: &ControlPlaneVirtualIPSpec{ + Configuration: &ControlPlaneVirtualIPConfiguration{ + Address: "192.168.1.1", + }, + }, + Host: "192.168.1.2", + }, + expected: "192.168.1.1", + }, + { + name: "VirtualIPSpec struct not specified", + spec: ControlPlaneEndpointSpec{ + VirtualIPSpec: nil, + Host: "192.168.1.2", + }, + expected: "192.168.1.2", + }, + { + name: "ControlPlaneVirtualIPConfiguration struct not specified", + spec: ControlPlaneEndpointSpec{ + VirtualIPSpec: &ControlPlaneVirtualIPSpec{ + Configuration: nil, + }, + Host: "192.168.1.2", + }, + expected: "192.168.1.2", + }, + { + name: "Virtual IP specified as empty string", + spec: ControlPlaneEndpointSpec{ + VirtualIPSpec: &ControlPlaneVirtualIPSpec{ + Configuration: &ControlPlaneVirtualIPConfiguration{ + Address: "", + }, + }, + Host: "192.168.1.2", + }, + expected: "192.168.1.2", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.spec.VirtualIPAddress() + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/api/v1alpha1/nutanix_clusterconfig_types.go b/api/v1alpha1/nutanix_clusterconfig_types.go index 66190a556..83f0cd325 100644 --- a/api/v1alpha1/nutanix_clusterconfig_types.go +++ b/api/v1alpha1/nutanix_clusterconfig_types.go @@ -5,6 +5,7 @@ package v1alpha1 import ( "fmt" + "net/netip" "net/url" "strconv" ) @@ -76,3 +77,17 @@ func (s NutanixPrismCentralEndpointSpec) ParseURL() (string, uint16, error) { return hostname, uint16(port), nil } + +func (s NutanixPrismCentralEndpointSpec) ParseIP() (netip.Addr, error) { + pcHostname, _, err := s.ParseURL() + if err != nil { + return netip.Addr{}, err + } + + pcIP, err := netip.ParseAddr(pcHostname) + if err != nil { + return netip.Addr{}, fmt.Errorf("error parsing Prism Central IP: %w", err) + } + + return pcIP, nil +} diff --git a/api/v1alpha1/nutanix_clusterconfig_types_test.go b/api/v1alpha1/nutanix_clusterconfig_types_test.go new file mode 100644 index 000000000..5bc72a510 --- /dev/null +++ b/api/v1alpha1/nutanix_clusterconfig_types_test.go @@ -0,0 +1,132 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import ( + "fmt" + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseURL(t *testing.T) { + tests := []struct { + name string + spec NutanixPrismCentralEndpointSpec + expectedHost string + expectedPort uint16 + expectedErr error + }{ + { + name: "Valid URL with port", + spec: NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + expectedHost: "192.168.1.1", + expectedPort: 9440, + expectedErr: nil, + }, + { + name: "Valid URL without port", + spec: NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1", + }, + expectedHost: "192.168.1.1", + expectedPort: 9440, + expectedErr: nil, + }, + { + name: "Invalid URL", + spec: NutanixPrismCentralEndpointSpec{ + URL: "invalid-url", + }, + expectedHost: "", + expectedPort: 0, + expectedErr: fmt.Errorf( + "error parsing Prism Central URL: parse %q: invalid URI for request", + "invalid-url", + ), + }, + { + name: "Invalid port", + spec: NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:invalid-port", + }, + expectedHost: "", + expectedPort: 0, + expectedErr: fmt.Errorf( + "error parsing Prism Central URL: parse %q: invalid port %q after host", + "https://192.168.1.1:invalid-port", + ":invalid-port", + ), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + host, port, err := tt.spec.ParseURL() + if tt.expectedErr != nil { + require.EqualError(t, err, tt.expectedErr.Error()) + } else { + require.NoError(t, err) + } + assert.Equal(t, tt.expectedHost, host) + assert.Equal(t, tt.expectedPort, port) + }) + } +} + +func TestParseIP(t *testing.T) { + tests := []struct { + name string + spec NutanixPrismCentralEndpointSpec + expectedIP netip.Addr + expectedErr error + }{ + { + name: "Valid IP", + spec: NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + expectedIP: netip.MustParseAddr("192.168.1.1"), + expectedErr: nil, + }, + { + name: "Invalid URL", + spec: NutanixPrismCentralEndpointSpec{ + URL: "invalid-url", + }, + expectedIP: netip.Addr{}, + expectedErr: fmt.Errorf( + "error parsing Prism Central URL: parse %q: invalid URI for request", + "invalid-url", + ), + }, + { + name: "Invalid IP", + spec: NutanixPrismCentralEndpointSpec{ + URL: "https://invalid-ip:9440", + }, + expectedIP: netip.Addr{}, + expectedErr: fmt.Errorf( + "error parsing Prism Central IP: ParseAddr(%q): unable to parse IP", + "invalid-ip", + ), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ip, err := tt.spec.ParseIP() + if tt.expectedErr != nil { + require.EqualError(t, err, tt.expectedErr.Error()) + } else { + require.NoError(t, err) + } + assert.Equal(t, tt.expectedIP, ip) + }) + } +} diff --git a/pkg/webhook/cluster/nutanix_validator.go b/pkg/webhook/cluster/nutanix_validator.go index dd91da0ea..3f3499a5e 100644 --- a/pkg/webhook/cluster/nutanix_validator.go +++ b/pkg/webhook/cluster/nutanix_validator.go @@ -7,8 +7,8 @@ import ( "context" "errors" "fmt" - "net" "net/http" + "net/netip" v1 "k8s.io/api/admission/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -70,15 +70,23 @@ func (a *nutanixValidator) validate( ) } - if clusterConfig.Nutanix != nil && - clusterConfig.Addons != nil { - // Check if Prism Central IP is in MetalLB Load Balancer IP range. - if err := validatePrismCentralIPNotInLoadBalancerIPRange( + if clusterConfig.Nutanix != nil { + if err := validatePrismCentralIPDoesNotEqualControlPlaneIP( clusterConfig.Nutanix.PrismCentralEndpoint, - clusterConfig.Addons.ServiceLoadBalancer, + clusterConfig.Nutanix.ControlPlaneEndpoint, ); err != nil { return admission.Denied(err.Error()) } + + if clusterConfig.Addons != nil { + // Check if Prism Central IP is in MetalLB Load Balancer IP range. + if err := validatePrismCentralIPNotInLoadBalancerIPRange( + clusterConfig.Nutanix.PrismCentralEndpoint, + clusterConfig.Addons.ServiceLoadBalancer, + ); err != nil { + return admission.Denied(err.Error()) + } + } } return admission.Allowed("") @@ -96,14 +104,10 @@ func validatePrismCentralIPNotInLoadBalancerIPRange( return nil } - pcHostname, _, err := pcEndpoint.ParseURL() + pcIP, err := pcEndpoint.ParseIP() if err != nil { - return err - } - - pcIP := net.ParseIP(pcHostname) - // PC URL can contain IP/FQDN, so compare only if PC is an IP address. - if pcIP == nil { + // If it's not able to parse IP correctly then, ignore the error as + // we want to compare only IP addresses. return nil } @@ -131,3 +135,35 @@ func validatePrismCentralIPNotInLoadBalancerIPRange( return nil } + +// validatePrismCentralIPDoesNotEqualControlPlaneIP checks if Prism Central and Control Plane IP are same, +// error out if they are same. +// It strictly compares IP addresses(no FQDN) and doesn't involve any network calls. +func validatePrismCentralIPDoesNotEqualControlPlaneIP( + pcEndpoint v1alpha1.NutanixPrismCentralEndpointSpec, + controlPlaneEndpointSpec v1alpha1.ControlPlaneEndpointSpec, +) error { + controlPlaneVIP, err := netip.ParseAddr(controlPlaneEndpointSpec.VirtualIPAddress()) + if err != nil { + // If controlPlaneEndpointIP is a hostname, we cannot compare it with PC IP + // so return directly. + return nil + } + + pcIP, err := pcEndpoint.ParseIP() + if err != nil { + // If it's not able to parse IP correctly then, ignore the error as + // we want to compare only IP addresses. + return nil + } + + if pcIP.Compare(controlPlaneVIP) == 0 { + errMsg := fmt.Sprintf( + "Prism Central and control plane endpoint cannot have the same IP %q", + pcIP.String(), + ) + return errors.New(errMsg) + } + + return nil +} diff --git a/pkg/webhook/cluster/nutanix_validator_test.go b/pkg/webhook/cluster/nutanix_validator_test.go index f769795b7..f53aada2b 100644 --- a/pkg/webhook/cluster/nutanix_validator_test.go +++ b/pkg/webhook/cluster/nutanix_validator_test.go @@ -67,10 +67,7 @@ func TestValidatePrismCentralIPNotInLoadBalancerIPRange(t *testing.T) { }, }, }, - expectedErr: fmt.Errorf( - "error parsing Prism Central URL: parse %q: invalid URI for request", - "invalid-url", - ), + expectedErr: nil, }, { name: "Service Load Balancer Configuration is nil", @@ -112,3 +109,115 @@ func TestValidatePrismCentralIPNotInLoadBalancerIPRange(t *testing.T) { }) } } + +func TestValidatePrismCentralIPDoesNotEqualControlPlaneIP(t *testing.T) { + tests := []struct { + name string + pcEndpoint v1alpha1.NutanixPrismCentralEndpointSpec + controlPlaneEndpointSpec v1alpha1.ControlPlaneEndpointSpec + expectedErr error + }{ + { + name: "Different IPs", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{ + Host: "192.168.1.2", + }, + expectedErr: nil, + }, + { + name: "Same IPs", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{ + Host: "192.168.1.1", + }, + expectedErr: fmt.Errorf( + "Prism Central and control plane endpoint cannot have the same IP %q", + "192.168.1.1", + ), + }, + { + name: "Control Plane IP specified as hostname", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{ + Host: "dummy-hostname", + }, + expectedErr: nil, + }, + { + name: "Invalid Prism Central URL", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "invalid-url", + }, + controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{ + Host: "192.168.1.2", + }, + expectedErr: nil, + }, + { + name: "Prism Central URL is FQDN", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://example.com:9440", + }, + controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{ + Host: "192.168.1.2", + }, + expectedErr: nil, + }, + { + name: "With KubeVIP ovveride and same PC and Control Plane IPs", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{ + Host: "192.168.1.2", + VirtualIPSpec: &v1alpha1.ControlPlaneVirtualIPSpec{ + Provider: "KubeVIP", + Configuration: &v1alpha1.ControlPlaneVirtualIPConfiguration{ + Address: "192.168.1.1", + }, + }, + }, + expectedErr: fmt.Errorf( + "Prism Central and control plane endpoint cannot have the same IP %q", + "192.168.1.1", + ), + }, + { + name: "With KubeVIP override and different PC and Control Plane IPs", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.2:9440", + }, + controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{ + Host: "192.168.1.2", + VirtualIPSpec: &v1alpha1.ControlPlaneVirtualIPSpec{ + Provider: "KubeVIP", + Configuration: &v1alpha1.ControlPlaneVirtualIPConfiguration{ + Address: "192.168.1.1", + }, + }, + }, + expectedErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validatePrismCentralIPDoesNotEqualControlPlaneIP( + tt.pcEndpoint, + tt.controlPlaneEndpointSpec, + ) + if tt.expectedErr != nil { + assert.Equal(t, tt.expectedErr.Error(), err.Error()) + } else { + assert.NoError(t, err) + } + }) + } +}