From b519340fe4d934c955224b20590acd015597503b Mon Sep 17 00:00:00 2001 From: Manoj Surudwad Date: Thu, 12 Dec 2024 23:48:14 +0530 Subject: [PATCH 1/3] fix: validates NUTANIX_ENDPOINT is outside the Load Balancer IP Range --- api/v1alpha1/nutanix_clusterconfig_types.go | 2 +- pkg/helpers/helpers.go | 26 ++++ pkg/helpers/helpers_test.go | 116 ++++++++++++++++ pkg/webhook/cluster/nutanix_validator.go | 127 ++++++++++++++++++ pkg/webhook/cluster/nutanix_validator_test.go | 114 ++++++++++++++++ .../cluster/{vaiidator.go => validator.go} | 1 + 6 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 pkg/helpers/helpers.go create mode 100644 pkg/helpers/helpers_test.go create mode 100644 pkg/webhook/cluster/nutanix_validator.go create mode 100644 pkg/webhook/cluster/nutanix_validator_test.go rename pkg/webhook/cluster/{vaiidator.go => validator.go} (89%) diff --git a/api/v1alpha1/nutanix_clusterconfig_types.go b/api/v1alpha1/nutanix_clusterconfig_types.go index 3240eac52..66190a556 100644 --- a/api/v1alpha1/nutanix_clusterconfig_types.go +++ b/api/v1alpha1/nutanix_clusterconfig_types.go @@ -57,7 +57,7 @@ type NutanixPrismCentralEndpointCredentials struct { //nolint:gocritic // No need for named return values func (s NutanixPrismCentralEndpointSpec) ParseURL() (string, uint16, error) { var prismCentralURL *url.URL - prismCentralURL, err := url.Parse(s.URL) + prismCentralURL, err := url.ParseRequestURI(s.URL) if err != nil { return "", 0, fmt.Errorf("error parsing Prism Central URL: %w", err) } diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go new file mode 100644 index 000000000..9abde3f04 --- /dev/null +++ b/pkg/helpers/helpers.go @@ -0,0 +1,26 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +package helpers + +import ( + "fmt" + "net/netip" +) + +// IsIPInRange checks if the target IP falls within the start and end IP range (inclusive). +func IsIPInRange(startIP, endIP, targetIP string) (bool, error) { + start, err := netip.ParseAddr(startIP) + if err != nil { + return false, fmt.Errorf("invalid start IP: %w", err) + } + end, err := netip.ParseAddr(endIP) + if err != nil { + return false, fmt.Errorf("invalid end IP: %w", err) + } + target, err := netip.ParseAddr(targetIP) + if err != nil { + return false, fmt.Errorf("invalid target IP: %w", err) + } + + return start.Compare(target) <= 0 && end.Compare(target) >= 0, nil +} diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go new file mode 100644 index 000000000..360cd1dee --- /dev/null +++ b/pkg/helpers/helpers_test.go @@ -0,0 +1,116 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package helpers + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsIPInRange(t *testing.T) { + tests := []struct { + name string + startIP string + endIP string + targetIP string + expectedInRange bool + expectedErr error + }{ + { + name: "Valid range - target within range", + startIP: "192.168.1.1", + endIP: "192.168.1.10", + targetIP: "192.168.1.5", + expectedInRange: true, + expectedErr: nil, + }, + { + name: "Valid range - target same as start IP", + startIP: "192.168.1.1", + endIP: "192.168.1.10", + targetIP: "192.168.1.1", + expectedInRange: true, + expectedErr: nil, + }, + { + name: "Valid range - target same as end IP", + startIP: "192.168.1.1", + endIP: "192.168.1.10", + targetIP: "192.168.1.10", + expectedInRange: true, + expectedErr: nil, + }, + { + name: "Valid range - target outside range", + startIP: "192.168.1.1", + endIP: "192.168.1.10", + targetIP: "192.168.1.15", + expectedInRange: false, + expectedErr: nil, + }, + { + name: "Invalid start IP", + startIP: "invalid-ip", + endIP: "192.168.1.10", + targetIP: "192.168.1.5", + expectedInRange: false, + expectedErr: fmt.Errorf( + "invalid start IP: ParseAddr(%q): unable to parse IP", + "invalid-ip", + ), + }, + { + name: "Invalid end IP", + startIP: "192.168.1.1", + endIP: "invalid-ip", + targetIP: "192.168.1.5", + expectedInRange: false, + expectedErr: fmt.Errorf( + "invalid end IP: ParseAddr(%q): unable to parse IP", + "invalid-ip", + ), + }, + { + name: "Invalid target IP", + startIP: "192.168.1.1", + endIP: "192.168.1.10", + targetIP: "invalid-ip", + expectedInRange: false, + expectedErr: fmt.Errorf( + "invalid target IP: ParseAddr(%q): unable to parse IP", + "invalid-ip", + ), + }, + { + name: "IPv6 range - target within range", + startIP: "2001:db8::1", + endIP: "2001:db8::10", + targetIP: "2001:db8::5", + expectedInRange: true, + expectedErr: nil, + }, + { + name: "IPv6 range - target outside range", + startIP: "2001:db8::1", + endIP: "2001:db8::10", + targetIP: "2001:db8::11", + expectedInRange: false, + expectedErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := IsIPInRange(tt.startIP, tt.endIP, tt.targetIP) + assert.Equal(t, tt.expectedInRange, got) + if tt.expectedErr != nil { + assert.EqualError(t, err, tt.expectedErr.Error()) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/webhook/cluster/nutanix_validator.go b/pkg/webhook/cluster/nutanix_validator.go new file mode 100644 index 000000000..f77aa742c --- /dev/null +++ b/pkg/webhook/cluster/nutanix_validator.go @@ -0,0 +1,127 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cluster + +import ( + "context" + "fmt" + "net" + "net/http" + + v1 "k8s.io/api/admission/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/utils" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/helpers" +) + +type nutanixValidator struct { + client ctrlclient.Client + decoder admission.Decoder +} + +func NewNutanixValidator( + client ctrlclient.Client, decoder admission.Decoder, +) *nutanixValidator { + return &nutanixValidator{ + client: client, + decoder: decoder, + } +} + +func (a *nutanixValidator) Validator() admission.HandlerFunc { + return a.validate +} + +func (a *nutanixValidator) validate( + ctx context.Context, + req admission.Request, +) admission.Response { + if req.Operation == v1.Delete { + return admission.Allowed("") + } + + cluster := &clusterv1.Cluster{} + err := a.decoder.Decode(req, cluster) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if cluster.Spec.Topology == nil { + return admission.Allowed("") + } + + if utils.GetProvider(cluster) != "nutanix" { + return admission.Allowed("") + } + + clusterConfig, err := variables.UnmarshalClusterConfigVariable(cluster.Spec.Topology.Variables) + if err != nil { + return admission.Denied( + fmt.Errorf("failed to unmarshal cluster topology variable %q: %w", + v1alpha1.ClusterConfigVariableName, + err).Error(), + ) + } + + if clusterConfig.Nutanix != nil && + clusterConfig.Addons != nil { + // Check if Prism Central IP is in MetalLB Load Balancer IP range. + if err := checkIfPrismCentralIPInLoadBalancerIPRange( + clusterConfig.Nutanix.PrismCentralEndpoint, + clusterConfig.Addons.ServiceLoadBalancer, + ); err != nil { + return admission.Denied(err.Error()) + } + } + + return admission.Allowed("") +} + +// checkIfPrismCentralIPInLoadBalancerIPRange checks if the Prism Central IP is in the MetalLB Load Balancer IP range. +// Errors out if Prism Central IP is in the Load Balancer IP range. +func checkIfPrismCentralIPInLoadBalancerIPRange( + pcEndpoint v1alpha1.NutanixPrismCentralEndpointSpec, + serviceLoadBalancerConfiguration *v1alpha1.ServiceLoadBalancer, +) error { + if serviceLoadBalancerConfiguration == nil || + serviceLoadBalancerConfiguration.Provider != v1alpha1.ServiceLoadBalancerProviderMetalLB || + serviceLoadBalancerConfiguration.Configuration == nil { + return nil + } + + pcHostname, _, err := pcEndpoint.ParseURL() + 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 { + return nil + } + + for _, pool := range serviceLoadBalancerConfiguration.Configuration.AddressRanges { + isIPInRange, err := helpers.IsIPInRange(pool.Start, pool.End, pcIP.String()) + if err != nil { + return fmt.Errorf( + "error while checking if Prism Central IP %q is part of MetalLB address range %q-%q: %w", + pcIP, + pool.Start, + pool.End, + err, + ) + } + if isIPInRange { + return fmt.Errorf("prism central IP %q must not be part of MetalLB address range %q-%q", + pcIP, pool.Start, pool.End) + } + } + + return nil +} diff --git a/pkg/webhook/cluster/nutanix_validator_test.go b/pkg/webhook/cluster/nutanix_validator_test.go new file mode 100644 index 000000000..3a4923d3a --- /dev/null +++ b/pkg/webhook/cluster/nutanix_validator_test.go @@ -0,0 +1,114 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cluster + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" +) + +func TestCheckIfPrismCentralIPInLoadBalancerIPRange(t *testing.T) { + tests := []struct { + name string + pcEndpoint v1alpha1.NutanixPrismCentralEndpointSpec + serviceLoadBalancerConfiguration *v1alpha1.ServiceLoadBalancer + expectedErr error + }{ + { + name: "PC IP not in range", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + serviceLoadBalancerConfiguration: &v1alpha1.ServiceLoadBalancer{ + Provider: v1alpha1.ServiceLoadBalancerProviderMetalLB, + Configuration: &v1alpha1.ServiceLoadBalancerConfiguration{ + AddressRanges: []v1alpha1.AddressRange{ + {Start: "192.168.1.10", End: "192.168.1.20"}, + }, + }, + }, + expectedErr: nil, + }, + { + name: "PC IP in range", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.15:9440", + }, + serviceLoadBalancerConfiguration: &v1alpha1.ServiceLoadBalancer{ + Provider: v1alpha1.ServiceLoadBalancerProviderMetalLB, + Configuration: &v1alpha1.ServiceLoadBalancerConfiguration{ + AddressRanges: []v1alpha1.AddressRange{ + {Start: "192.168.1.10", End: "192.168.1.20"}, + }, + }, + }, + expectedErr: fmt.Errorf( + "prism central IP %q must not be part of MetalLB address range %q-%q", + "192.168.1.15", + "192.168.1.10", + "192.168.1.20", + ), + }, + { + name: "Invalid Prism Central URL", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "invalid-url", + }, + serviceLoadBalancerConfiguration: &v1alpha1.ServiceLoadBalancer{ + Provider: v1alpha1.ServiceLoadBalancerProviderMetalLB, + Configuration: &v1alpha1.ServiceLoadBalancerConfiguration{ + AddressRanges: []v1alpha1.AddressRange{ + {Start: "192.168.1.10", End: "192.168.1.20"}, + }, + }, + }, + expectedErr: fmt.Errorf( + "error parsing Prism Central URL: parse %q: invalid URI for request", + "invalid-url", + ), + }, + { + name: "Service Load Balancer Configuration is nil", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + serviceLoadBalancerConfiguration: nil, + expectedErr: nil, + }, + { + name: "Provider is not MetalLB", + pcEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://192.168.1.1:9440", + }, + serviceLoadBalancerConfiguration: &v1alpha1.ServiceLoadBalancer{ + Provider: "other-provider", + Configuration: &v1alpha1.ServiceLoadBalancerConfiguration{ + AddressRanges: []v1alpha1.AddressRange{ + {Start: "192.168.1.10", End: "192.168.1.20"}, + }, + }, + }, + expectedErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkIfPrismCentralIPInLoadBalancerIPRange( + tt.pcEndpoint, + tt.serviceLoadBalancerConfiguration, + ) + + if tt.expectedErr != nil { + assert.Equal(t, tt.expectedErr.Error(), err.Error()) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/webhook/cluster/vaiidator.go b/pkg/webhook/cluster/validator.go similarity index 89% rename from pkg/webhook/cluster/vaiidator.go rename to pkg/webhook/cluster/validator.go index 3eae0a7dc..adbddae27 100644 --- a/pkg/webhook/cluster/vaiidator.go +++ b/pkg/webhook/cluster/validator.go @@ -11,5 +11,6 @@ import ( func NewValidator(client ctrlclient.Client, decoder admission.Decoder) admission.Handler { return admission.MultiValidatingHandler( NewClusterUUIDLabeler(client, decoder).Validator(), + NewNutanixValidator(client, decoder).Validator(), ) } From ce6738aab0894d5314e880bc716bab7e871c8da1 Mon Sep 17 00:00:00 2001 From: Manoj Surudwad Date: Tue, 17 Dec 2024 12:47:48 +0530 Subject: [PATCH 2/3] refactor: renames func and error messages --- pkg/webhook/cluster/nutanix_validator.go | 20 ++++++++++++------- pkg/webhook/cluster/nutanix_validator_test.go | 6 +++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/webhook/cluster/nutanix_validator.go b/pkg/webhook/cluster/nutanix_validator.go index f77aa742c..dd91da0ea 100644 --- a/pkg/webhook/cluster/nutanix_validator.go +++ b/pkg/webhook/cluster/nutanix_validator.go @@ -5,6 +5,7 @@ package cluster import ( "context" + "errors" "fmt" "net" "net/http" @@ -72,7 +73,7 @@ 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 := checkIfPrismCentralIPInLoadBalancerIPRange( + if err := validatePrismCentralIPNotInLoadBalancerIPRange( clusterConfig.Nutanix.PrismCentralEndpoint, clusterConfig.Addons.ServiceLoadBalancer, ); err != nil { @@ -83,9 +84,9 @@ func (a *nutanixValidator) validate( return admission.Allowed("") } -// checkIfPrismCentralIPInLoadBalancerIPRange checks if the Prism Central IP is in the MetalLB Load Balancer IP range. -// Errors out if Prism Central IP is in the Load Balancer IP range. -func checkIfPrismCentralIPInLoadBalancerIPRange( +// validatePrismCentralIPNotInLoadBalancerIPRange checks if the Prism Central IP is not +// in the MetalLB Load Balancer IP range and error out if it is. +func validatePrismCentralIPNotInLoadBalancerIPRange( pcEndpoint v1alpha1.NutanixPrismCentralEndpointSpec, serviceLoadBalancerConfiguration *v1alpha1.ServiceLoadBalancer, ) error { @@ -110,7 +111,7 @@ func checkIfPrismCentralIPInLoadBalancerIPRange( isIPInRange, err := helpers.IsIPInRange(pool.Start, pool.End, pcIP.String()) if err != nil { return fmt.Errorf( - "error while checking if Prism Central IP %q is part of MetalLB address range %q-%q: %w", + "failed to check if Prism Central IP %q is part of MetalLB address range %q-%q: %w", pcIP, pool.Start, pool.End, @@ -118,8 +119,13 @@ func checkIfPrismCentralIPInLoadBalancerIPRange( ) } if isIPInRange { - return fmt.Errorf("prism central IP %q must not be part of MetalLB address range %q-%q", - pcIP, pool.Start, pool.End) + errMsg := fmt.Sprintf( + "Prism Central IP %q must not be part of MetalLB address range %q-%q", + pcIP, + pool.Start, + pool.End, + ) + return errors.New(errMsg) } } diff --git a/pkg/webhook/cluster/nutanix_validator_test.go b/pkg/webhook/cluster/nutanix_validator_test.go index 3a4923d3a..f769795b7 100644 --- a/pkg/webhook/cluster/nutanix_validator_test.go +++ b/pkg/webhook/cluster/nutanix_validator_test.go @@ -12,7 +12,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" ) -func TestCheckIfPrismCentralIPInLoadBalancerIPRange(t *testing.T) { +func TestValidatePrismCentralIPNotInLoadBalancerIPRange(t *testing.T) { tests := []struct { name string pcEndpoint v1alpha1.NutanixPrismCentralEndpointSpec @@ -48,7 +48,7 @@ func TestCheckIfPrismCentralIPInLoadBalancerIPRange(t *testing.T) { }, }, expectedErr: fmt.Errorf( - "prism central IP %q must not be part of MetalLB address range %q-%q", + "Prism Central IP %q must not be part of MetalLB address range %q-%q", "192.168.1.15", "192.168.1.10", "192.168.1.20", @@ -99,7 +99,7 @@ func TestCheckIfPrismCentralIPInLoadBalancerIPRange(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := checkIfPrismCentralIPInLoadBalancerIPRange( + err := validatePrismCentralIPNotInLoadBalancerIPRange( tt.pcEndpoint, tt.serviceLoadBalancerConfiguration, ) From b9cc1f8340d4450ea7dea6e1eb635e798e7fd85f Mon Sep 17 00:00:00 2001 From: Manoj Surudwad Date: Wed, 18 Dec 2024 12:22:04 +0530 Subject: [PATCH 3/3] fix: validates control plane and prism central IP are distinct --- api/v1alpha1/common_types.go | 12 ++ api/v1alpha1/common_types_test.go | 67 +++++++++ api/v1alpha1/nutanix_clusterconfig_types.go | 15 ++ .../nutanix_clusterconfig_types_test.go | 132 ++++++++++++++++++ pkg/webhook/cluster/nutanix_validator.go | 62 ++++++-- pkg/webhook/cluster/nutanix_validator_test.go | 117 +++++++++++++++- 6 files changed, 388 insertions(+), 17 deletions(-) create mode 100644 api/v1alpha1/common_types_test.go create mode 100644 api/v1alpha1/nutanix_clusterconfig_types_test.go 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) + } + }) + } +}