Skip to content

Commit

Permalink
fix: validates PC IP is outside Load Balancer IP Range (#1001)
Browse files Browse the repository at this point in the history
**What problem does this PR solve?**:

webhook errors out if NUTANIX_ENDPOINT IP falls in Load Balancer IP
Range. It only implements dumb check which compares PC IP with Load
Balancer IP Range. It's complex to achieve with CEL so going with
webhook as we'll need to use regex(isIP() func is not working for
cluster variables) to extract IP from PC URL and do string compare which
not robust as we can do error handling through webhook.

**Which issue(s) this PR fixes**:
Fixes #
https://jira.nutanix.com/browse/NCN-102628

**How Has This Been Tested?**:
<!--
Please describe the tests that you ran to verify your changes.
Provide output from the tests and any manual steps needed to replicate
the tests.
-->

**Special notes for your reviewer**:
<!--
Use this to provide any additional information to the reviewers.
This may include:
- Best way to review the PR.
- Where the author wants the most review attention on.
- etc.
-->

```
✗ clusterctl generate cluster ${CLUSTER_NAME} \  --from ${CLUSTER_FILE} \
  --kubernetes-version ${KUBERNETES_VERSION} \
  --worker-machine-count 1 | \
  kubectl apply --server-side -f -
secret/nutanix-cluster-cilium-helm-addon-dockerhub-credentials serverside-applied
secret/nutanix-cluster-cilium-helm-addon-pc-creds-for-csi serverside-applied
Warning: Detected changes to resource nutanix-cluster-cilium-helm-addon-pc-creds which is currently being deleted.
secret/nutanix-cluster-cilium-helm-addon-pc-creds serverside-applied
Error from server (Forbidden): admission webhook "cluster-validator.caren.nutanix.com" denied the request: Prism Central IP "198.18.1.1" must not be part of MetalLB address range "198.18.1.1"-"198.18.1.10"
```
  • Loading branch information
manoj-nutanix authored Dec 18, 2024
1 parent 8c18f88 commit ffb7bb2
Show file tree
Hide file tree
Showing 9 changed files with 762 additions and 1 deletion.
12 changes: 12 additions & 0 deletions api/v1alpha1/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
67 changes: 67 additions & 0 deletions api/v1alpha1/common_types_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
17 changes: 16 additions & 1 deletion api/v1alpha1/nutanix_clusterconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package v1alpha1

import (
"fmt"
"net/netip"
"net/url"
"strconv"
)
Expand Down Expand Up @@ -57,7 +58,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)
}
Expand All @@ -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
}
132 changes: 132 additions & 0 deletions api/v1alpha1/nutanix_clusterconfig_types_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
26 changes: 26 additions & 0 deletions pkg/helpers/helpers.go
Original file line number Diff line number Diff line change
@@ -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
}
116 changes: 116 additions & 0 deletions pkg/helpers/helpers_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
Loading

0 comments on commit ffb7bb2

Please sign in to comment.