Skip to content

Commit b9cc1f8

Browse files
manoj-nutanixmanoj-nutanix
committed
fix: validates control plane and prism central IP are distinct
1 parent ce6738a commit b9cc1f8

File tree

6 files changed

+388
-17
lines changed

6 files changed

+388
-17
lines changed

‎api/v1alpha1/common_types.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,15 @@ type LocalObjectReference struct {
7777
// +kubebuilder:validation:MinLength=1
7878
Name string `json:"name"`
7979
}
80+
81+
func (s ControlPlaneEndpointSpec) VirtualIPAddress() string {
82+
// If specified, use the virtual IP address and/or port,
83+
// otherwise fall back to the control plane endpoint host and port.
84+
if s.VirtualIPSpec != nil &&
85+
s.VirtualIPSpec.Configuration != nil &&
86+
s.VirtualIPSpec.Configuration.Address != "" {
87+
return s.VirtualIPSpec.Configuration.Address
88+
}
89+
90+
return s.Host
91+
}

‎api/v1alpha1/common_types_test.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Copyright 2023 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
package v1alpha1
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestVirtualIPAddress(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
spec ControlPlaneEndpointSpec
15+
expected string
16+
}{
17+
{
18+
name: "Virtual IP specified",
19+
spec: ControlPlaneEndpointSpec{
20+
VirtualIPSpec: &ControlPlaneVirtualIPSpec{
21+
Configuration: &ControlPlaneVirtualIPConfiguration{
22+
Address: "192.168.1.1",
23+
},
24+
},
25+
Host: "192.168.1.2",
26+
},
27+
expected: "192.168.1.1",
28+
},
29+
{
30+
name: "VirtualIPSpec struct not specified",
31+
spec: ControlPlaneEndpointSpec{
32+
VirtualIPSpec: nil,
33+
Host: "192.168.1.2",
34+
},
35+
expected: "192.168.1.2",
36+
},
37+
{
38+
name: "ControlPlaneVirtualIPConfiguration struct not specified",
39+
spec: ControlPlaneEndpointSpec{
40+
VirtualIPSpec: &ControlPlaneVirtualIPSpec{
41+
Configuration: nil,
42+
},
43+
Host: "192.168.1.2",
44+
},
45+
expected: "192.168.1.2",
46+
},
47+
{
48+
name: "Virtual IP specified as empty string",
49+
spec: ControlPlaneEndpointSpec{
50+
VirtualIPSpec: &ControlPlaneVirtualIPSpec{
51+
Configuration: &ControlPlaneVirtualIPConfiguration{
52+
Address: "",
53+
},
54+
},
55+
Host: "192.168.1.2",
56+
},
57+
expected: "192.168.1.2",
58+
},
59+
}
60+
61+
for _, tt := range tests {
62+
t.Run(tt.name, func(t *testing.T) {
63+
result := tt.spec.VirtualIPAddress()
64+
assert.Equal(t, tt.expected, result)
65+
})
66+
}
67+
}

‎api/v1alpha1/nutanix_clusterconfig_types.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package v1alpha1
55

66
import (
77
"fmt"
8+
"net/netip"
89
"net/url"
910
"strconv"
1011
)
@@ -76,3 +77,17 @@ func (s NutanixPrismCentralEndpointSpec) ParseURL() (string, uint16, error) {
7677

7778
return hostname, uint16(port), nil
7879
}
80+
81+
func (s NutanixPrismCentralEndpointSpec) ParseIP() (netip.Addr, error) {
82+
pcHostname, _, err := s.ParseURL()
83+
if err != nil {
84+
return netip.Addr{}, err
85+
}
86+
87+
pcIP, err := netip.ParseAddr(pcHostname)
88+
if err != nil {
89+
return netip.Addr{}, fmt.Errorf("error parsing Prism Central IP: %w", err)
90+
}
91+
92+
return pcIP, nil
93+
}

‎api/v1alpha1/nutanix_clusterconfig_types_test.go‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
// Copyright 2024 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package v1alpha1
5+
6+
import (
7+
"fmt"
8+
"net/netip"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
func TestParseURL(t *testing.T) {
16+
tests := []struct {
17+
name string
18+
spec NutanixPrismCentralEndpointSpec
19+
expectedHost string
20+
expectedPort uint16
21+
expectedErr error
22+
}{
23+
{
24+
name: "Valid URL with port",
25+
spec: NutanixPrismCentralEndpointSpec{
26+
URL: "https://192.168.1.1:9440",
27+
},
28+
expectedHost: "192.168.1.1",
29+
expectedPort: 9440,
30+
expectedErr: nil,
31+
},
32+
{
33+
name: "Valid URL without port",
34+
spec: NutanixPrismCentralEndpointSpec{
35+
URL: "https://192.168.1.1",
36+
},
37+
expectedHost: "192.168.1.1",
38+
expectedPort: 9440,
39+
expectedErr: nil,
40+
},
41+
{
42+
name: "Invalid URL",
43+
spec: NutanixPrismCentralEndpointSpec{
44+
URL: "invalid-url",
45+
},
46+
expectedHost: "",
47+
expectedPort: 0,
48+
expectedErr: fmt.Errorf(
49+
"error parsing Prism Central URL: parse %q: invalid URI for request",
50+
"invalid-url",
51+
),
52+
},
53+
{
54+
name: "Invalid port",
55+
spec: NutanixPrismCentralEndpointSpec{
56+
URL: "https://192.168.1.1:invalid-port",
57+
},
58+
expectedHost: "",
59+
expectedPort: 0,
60+
expectedErr: fmt.Errorf(
61+
"error parsing Prism Central URL: parse %q: invalid port %q after host",
62+
"https://192.168.1.1:invalid-port",
63+
":invalid-port",
64+
),
65+
},
66+
}
67+
68+
for _, tt := range tests {
69+
t.Run(tt.name, func(t *testing.T) {
70+
host, port, err := tt.spec.ParseURL()
71+
if tt.expectedErr != nil {
72+
require.EqualError(t, err, tt.expectedErr.Error())
73+
} else {
74+
require.NoError(t, err)
75+
}
76+
assert.Equal(t, tt.expectedHost, host)
77+
assert.Equal(t, tt.expectedPort, port)
78+
})
79+
}
80+
}
81+
82+
func TestParseIP(t *testing.T) {
83+
tests := []struct {
84+
name string
85+
spec NutanixPrismCentralEndpointSpec
86+
expectedIP netip.Addr
87+
expectedErr error
88+
}{
89+
{
90+
name: "Valid IP",
91+
spec: NutanixPrismCentralEndpointSpec{
92+
URL: "https://192.168.1.1:9440",
93+
},
94+
expectedIP: netip.MustParseAddr("192.168.1.1"),
95+
expectedErr: nil,
96+
},
97+
{
98+
name: "Invalid URL",
99+
spec: NutanixPrismCentralEndpointSpec{
100+
URL: "invalid-url",
101+
},
102+
expectedIP: netip.Addr{},
103+
expectedErr: fmt.Errorf(
104+
"error parsing Prism Central URL: parse %q: invalid URI for request",
105+
"invalid-url",
106+
),
107+
},
108+
{
109+
name: "Invalid IP",
110+
spec: NutanixPrismCentralEndpointSpec{
111+
URL: "https://invalid-ip:9440",
112+
},
113+
expectedIP: netip.Addr{},
114+
expectedErr: fmt.Errorf(
115+
"error parsing Prism Central IP: ParseAddr(%q): unable to parse IP",
116+
"invalid-ip",
117+
),
118+
},
119+
}
120+
121+
for _, tt := range tests {
122+
t.Run(tt.name, func(t *testing.T) {
123+
ip, err := tt.spec.ParseIP()
124+
if tt.expectedErr != nil {
125+
require.EqualError(t, err, tt.expectedErr.Error())
126+
} else {
127+
require.NoError(t, err)
128+
}
129+
assert.Equal(t, tt.expectedIP, ip)
130+
})
131+
}
132+
}

‎pkg/webhook/cluster/nutanix_validator.go‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77
"context"
88
"errors"
99
"fmt"
10-
"net"
1110
"net/http"
11+
"net/netip"
1212

1313
v1 "k8s.io/api/admission/v1"
1414
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -70,15 +70,23 @@ func (a *nutanixValidator) validate(
7070
)
7171
}
7272

73-
if clusterConfig.Nutanix != nil &&
74-
clusterConfig.Addons != nil {
75-
// Check if Prism Central IP is in MetalLB Load Balancer IP range.
76-
if err := validatePrismCentralIPNotInLoadBalancerIPRange(
73+
if clusterConfig.Nutanix != nil {
74+
if err := validatePrismCentralIPDoesNotEqualControlPlaneIP(
7775
clusterConfig.Nutanix.PrismCentralEndpoint,
78-
clusterConfig.Addons.ServiceLoadBalancer,
76+
clusterConfig.Nutanix.ControlPlaneEndpoint,
7977
); err != nil {
8078
return admission.Denied(err.Error())
8179
}
80+
81+
if clusterConfig.Addons != nil {
82+
// Check if Prism Central IP is in MetalLB Load Balancer IP range.
83+
if err := validatePrismCentralIPNotInLoadBalancerIPRange(
84+
clusterConfig.Nutanix.PrismCentralEndpoint,
85+
clusterConfig.Addons.ServiceLoadBalancer,
86+
); err != nil {
87+
return admission.Denied(err.Error())
88+
}
89+
}
8290
}
8391

8492
return admission.Allowed("")
@@ -96,14 +104,10 @@ func validatePrismCentralIPNotInLoadBalancerIPRange(
96104
return nil
97105
}
98106

99-
pcHostname, _, err := pcEndpoint.ParseURL()
107+
pcIP, err := pcEndpoint.ParseIP()
100108
if err != nil {
101-
return err
102-
}
103-
104-
pcIP := net.ParseIP(pcHostname)
105-
// PC URL can contain IP/FQDN, so compare only if PC is an IP address.
106-
if pcIP == nil {
109+
// If it's not able to parse IP correctly then, ignore the error as
110+
// we want to compare only IP addresses.
107111
return nil
108112
}
109113

@@ -131,3 +135,35 @@ func validatePrismCentralIPNotInLoadBalancerIPRange(
131135

132136
return nil
133137
}
138+
139+
// validatePrismCentralIPDoesNotEqualControlPlaneIP checks if Prism Central and Control Plane IP are same,
140+
// error out if they are same.
141+
// It strictly compares IP addresses(no FQDN) and doesn't involve any network calls.
142+
func validatePrismCentralIPDoesNotEqualControlPlaneIP(
143+
pcEndpoint v1alpha1.NutanixPrismCentralEndpointSpec,
144+
controlPlaneEndpointSpec v1alpha1.ControlPlaneEndpointSpec,
145+
) error {
146+
controlPlaneVIP, err := netip.ParseAddr(controlPlaneEndpointSpec.VirtualIPAddress())
147+
if err != nil {
148+
// If controlPlaneEndpointIP is a hostname, we cannot compare it with PC IP
149+
// so return directly.
150+
return nil
151+
}
152+
153+
pcIP, err := pcEndpoint.ParseIP()
154+
if err != nil {
155+
// If it's not able to parse IP correctly then, ignore the error as
156+
// we want to compare only IP addresses.
157+
return nil
158+
}
159+
160+
if pcIP.Compare(controlPlaneVIP) == 0 {
161+
errMsg := fmt.Sprintf(
162+
"Prism Central and control plane endpoint cannot have the same IP %q",
163+
pcIP.String(),
164+
)
165+
return errors.New(errMsg)
166+
}
167+
168+
return nil
169+
}

0 commit comments

Comments
 (0)