From 681e4f55646969fc962321c407ade93bc78f45e4 Mon Sep 17 00:00:00 2001 From: Dimitri Koshkin Date: Fri, 5 Apr 2024 15:17:20 -0700 Subject: [PATCH] refactor: combine PC host and port into a single url var This makes it simpler for clients to provide a single input field and not have to do any parsing to split the hostname and port. It also allows us to use API validation for bad input. --- api/v1alpha1/nutanix_clusterconfig_types.go | 26 ++---- .../controlplaneendpoint/variables_test.go | 12 +-- .../mutation/prismcentralendpoint/inject.go | 34 ++++++- .../prismcentralendpoint/inject_test.go | 40 ++++++++- .../prismcentralendpoint/variables_test.go | 90 +++++++++++++++++-- 5 files changed, 165 insertions(+), 37 deletions(-) diff --git a/api/v1alpha1/nutanix_clusterconfig_types.go b/api/v1alpha1/nutanix_clusterconfig_types.go index 6e39a6f88..5d014774b 100644 --- a/api/v1alpha1/nutanix_clusterconfig_types.go +++ b/api/v1alpha1/nutanix_clusterconfig_types.go @@ -7,12 +7,10 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - - "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/api/variables" ) const ( - PrismCentralPort = 9440 + DefaultPrismCentralPort = 9440 ) // NutanixSpec defines the desired state of NutanixCluster. @@ -39,11 +37,8 @@ func (NutanixSpec) VariableSchema() clusterv1.VariableSchema { } type NutanixPrismCentralEndpointSpec struct { - // host is the DNS name or IP address of the Nutanix Prism Central - Host string `json:"host"` - - // port is the port number to access the Nutanix Prism Central - Port int32 `json:"port"` + // The URL of Nutanix Prism Central, can be DNS name or an IP address + URL string `json:"url"` // use insecure connection to Prism Central endpoint // +optional @@ -65,17 +60,12 @@ func (NutanixPrismCentralEndpointSpec) VariableSchema() clusterv1.VariableSchema Description: "Nutanix Prism Central endpoint configuration", Type: "object", Properties: map[string]clusterv1.JSONSchemaProps{ - "host": { - Description: "the DNS name or IP address of the Nutanix Prism Central", + "url": { + Description: "The URL of Nutanix Prism Central, can be DNS name or an IP address", Type: "string", MinLength: ptr.To[int64](1), - }, - "port": { - Description: "The port number to access the Nutanix Prism Central", - Type: "integer", - Default: variables.MustMarshal(PrismCentralPort), - Minimum: ptr.To[int64](1), - Maximum: ptr.To[int64](65535), + Format: "uri", + Pattern: "^https://", }, "insecure": { Description: "Use insecure connection to Prism Central endpoint", @@ -103,7 +93,7 @@ func (NutanixPrismCentralEndpointSpec) VariableSchema() clusterv1.VariableSchema Required: []string{"name"}, }, }, - Required: []string{"host", "port", "credentials"}, + Required: []string{"url", "credentials"}, }, } } diff --git a/pkg/handlers/nutanix/mutation/controlplaneendpoint/variables_test.go b/pkg/handlers/nutanix/mutation/controlplaneendpoint/variables_test.go index 372e39564..eec2f61f9 100644 --- a/pkg/handlers/nutanix/mutation/controlplaneendpoint/variables_test.go +++ b/pkg/handlers/nutanix/mutation/controlplaneendpoint/variables_test.go @@ -4,6 +4,7 @@ package controlplaneendpoint import ( + "fmt" "testing" corev1 "k8s.io/api/core/v1" @@ -16,6 +17,8 @@ import ( nutanixclusterconfig "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/nutanix/clusterconfig" ) +var testPrismCentralURL = fmt.Sprintf("https://prism-central.nutanix.com:%d", v1alpha1.DefaultPrismCentralPort) + func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, @@ -33,8 +36,7 @@ func TestVariableValidation(t *testing.T) { }, // PrismCentralEndpoint is a required field and must always be set PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ - Host: "prism-central.nutanix.com", - Port: v1alpha1.PrismCentralPort, + URL: testPrismCentralURL, Credentials: corev1.LocalObjectReference{ Name: "credentials", }, @@ -52,8 +54,7 @@ func TestVariableValidation(t *testing.T) { }, // PrismCentralEndpoint is a required field and must always be set PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ - Host: "prism-central.nutanix.com", - Port: v1alpha1.PrismCentralPort, + URL: testPrismCentralURL, Credentials: corev1.LocalObjectReference{ Name: "credentials", }, @@ -72,8 +73,7 @@ func TestVariableValidation(t *testing.T) { }, // PrismCentralEndpoint is a required field and must always be set PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ - Host: "prism-central.nutanix.com", - Port: v1alpha1.PrismCentralPort, + URL: testPrismCentralURL, Credentials: corev1.LocalObjectReference{ Name: "credentials", }, diff --git a/pkg/handlers/nutanix/mutation/prismcentralendpoint/inject.go b/pkg/handlers/nutanix/mutation/prismcentralendpoint/inject.go index 808cb6fda..f5db5132d 100644 --- a/pkg/handlers/nutanix/mutation/prismcentralendpoint/inject.go +++ b/pkg/handlers/nutanix/mutation/prismcentralendpoint/inject.go @@ -7,6 +7,8 @@ import ( "context" "encoding/base64" "fmt" + "net/url" + "strconv" "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -97,9 +99,15 @@ func (h *nutanixPrismCentralEndpoint) Mutate( "patchedObjectName", client.ObjectKeyFromObject(obj), ).Info("setting prismCentralEndpoint in NutanixCluster spec") + var address string + var port int32 + address, port, err = parsePrismCentralURL(prismCentralEndpointVar.URL) + if err != nil { + return err + } prismCentral := &credentials.NutanixPrismEndpoint{ - Address: prismCentralEndpointVar.Host, - Port: prismCentralEndpointVar.Port, + Address: address, + Port: port, Insecure: prismCentralEndpointVar.Insecure, CredentialRef: &credentials.NutanixCredentialReference{ Kind: credentials.SecretKind, @@ -135,3 +143,25 @@ func (h *nutanixPrismCentralEndpoint) Mutate( }, ) } + +func parsePrismCentralURL(in string) (string, int32, error) { + var prismCentralURL *url.URL + prismCentralURL, err := url.Parse(in) + if err != nil { + return "", -1, fmt.Errorf("error parsing Prism Central URL: %w", err) + } + + hostname := prismCentralURL.Hostname() + + // return early with the default port if no port is specified + if prismCentralURL.Port() == "" { + return hostname, v1alpha1.DefaultPrismCentralPort, nil + } + + port, err := strconv.Atoi(prismCentralURL.Port()) + if err != nil { + return "", -1, fmt.Errorf("error converting port to int: %w", err) + } + + return hostname, int32(port), nil +} diff --git a/pkg/handlers/nutanix/mutation/prismcentralendpoint/inject_test.go b/pkg/handlers/nutanix/mutation/prismcentralendpoint/inject_test.go index 83d9f431e..0b977cc26 100644 --- a/pkg/handlers/nutanix/mutation/prismcentralendpoint/inject_test.go +++ b/pkg/handlers/nutanix/mutation/prismcentralendpoint/inject_test.go @@ -44,8 +44,7 @@ var _ = Describe("Generate Nutanix Prism Central Endpoint patches", func() { capitest.VariableWithValue( clusterconfig.MetaVariableName, v1alpha1.NutanixPrismCentralEndpointSpec{ - Host: "prism-central.nutanix.com", - Port: 9441, + URL: "https://prism-central.nutanix.com:9441", Insecure: true, Credentials: corev1.LocalObjectReference{ Name: "credentials", @@ -73,14 +72,47 @@ var _ = Describe("Generate Nutanix Prism Central Endpoint patches", func() { }, }, }, + { + Name: "all required fields set without port", + Vars: []runtimehooksv1.Variable{ + capitest.VariableWithValue( + clusterconfig.MetaVariableName, + v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://prism-central.nutanix.com", + Insecure: true, + Credentials: corev1.LocalObjectReference{ + Name: "credentials", + }, + }, + nutanixclusterconfig.NutanixVariableName, + VariableName, + ), + }, + RequestItem: request.NewNutanixClusterTemplateRequestItem(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "replace", + Path: "/spec/template/spec/prismCentral", + ValueMatcher: gomega.SatisfyAll( + gomega.HaveKeyWithValue( + "address", + gomega.BeEquivalentTo("prism-central.nutanix.com"), + ), + gomega.HaveKeyWithValue("port", gomega.BeEquivalentTo(v1alpha1.DefaultPrismCentralPort)), + gomega.HaveKeyWithValue("insecure", true), + gomega.HaveKey("credentialRef"), + gomega.Not(gomega.HaveKey("additionalTrustBundle")), + ), + }, + }, + }, { Name: "additional trust bundle is set", Vars: []runtimehooksv1.Variable{ capitest.VariableWithValue( clusterconfig.MetaVariableName, v1alpha1.NutanixPrismCentralEndpointSpec{ - Host: "prism-central.nutanix.com", - Port: 9441, + URL: "https://prism-central.nutanix.com:9441", Insecure: true, Credentials: corev1.LocalObjectReference{ Name: "credentials", diff --git a/pkg/handlers/nutanix/mutation/prismcentralendpoint/variables_test.go b/pkg/handlers/nutanix/mutation/prismcentralendpoint/variables_test.go index 318189001..0f8009218 100644 --- a/pkg/handlers/nutanix/mutation/prismcentralendpoint/variables_test.go +++ b/pkg/handlers/nutanix/mutation/prismcentralendpoint/variables_test.go @@ -4,6 +4,7 @@ package prismcentralendpoint import ( + "fmt" "testing" corev1 "k8s.io/api/core/v1" @@ -24,12 +25,11 @@ func TestVariableValidation(t *testing.T) { true, nutanixclusterconfig.NewVariable, capitest.VariableTestDef{ - Name: "valid PC address and port", + Name: "valid PC URL", Vals: v1alpha1.ClusterConfigSpec{ Nutanix: &v1alpha1.NutanixSpec{ PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ - Host: "prism-central.nutanix.com", - Port: v1alpha1.PrismCentralPort, + URL: fmt.Sprintf("https://prism-central.nutanix.com:%d", v1alpha1.DefaultPrismCentralPort), Insecure: false, Credentials: corev1.LocalObjectReference{ Name: "credentials", @@ -44,11 +44,88 @@ func TestVariableValidation(t *testing.T) { }, }, capitest.VariableTestDef{ - Name: "empty PC address", + Name: "valid PC URL as an IP", Vals: v1alpha1.ClusterConfigSpec{ Nutanix: &v1alpha1.NutanixSpec{ PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ - Port: v1alpha1.PrismCentralPort, + URL: fmt.Sprintf("https://10.0.0.1:%d", v1alpha1.DefaultPrismCentralPort), + Insecure: false, + Credentials: corev1.LocalObjectReference{ + Name: "credentials", + }, + }, + // ControlPlaneEndpoint is a required field and must always be set + ControlPlaneEndpoint: clusterv1.APIEndpoint{ + Host: "10.20.100.10", + Port: 6443, + }, + }, + }, + }, + capitest.VariableTestDef{ + Name: "valid PC URL without a port", + Vals: v1alpha1.ClusterConfigSpec{ + Nutanix: &v1alpha1.NutanixSpec{ + PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "https://prism-central.nutanix.com", + Insecure: false, + Credentials: corev1.LocalObjectReference{ + Name: "credentials", + }, + }, + // ControlPlaneEndpoint is a required field and must always be set + ControlPlaneEndpoint: clusterv1.APIEndpoint{ + Host: "10.20.100.10", + Port: 6443, + }, + }, + }, + }, + capitest.VariableTestDef{ + Name: "empty PC URL", + Vals: v1alpha1.ClusterConfigSpec{ + Nutanix: &v1alpha1.NutanixSpec{ + PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + Insecure: false, + Credentials: corev1.LocalObjectReference{ + Name: "credentials", + }, + }, + // ControlPlaneEndpoint is a required field and must always be set + ControlPlaneEndpoint: clusterv1.APIEndpoint{ + Host: "10.20.100.10", + Port: 6443, + }, + }, + }, + ExpectError: true, + }, + capitest.VariableTestDef{ + Name: "http is not a valid PC URL", + Vals: v1alpha1.ClusterConfigSpec{ + Nutanix: &v1alpha1.NutanixSpec{ + PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "http://prism-central.nutanix.com", + Insecure: false, + Credentials: corev1.LocalObjectReference{ + Name: "credentials", + }, + }, + // ControlPlaneEndpoint is a required field and must always be set + ControlPlaneEndpoint: clusterv1.APIEndpoint{ + Host: "10.20.100.10", + Port: 6443, + }, + }, + }, + ExpectError: true, + }, + capitest.VariableTestDef{ + Name: "not a valid PC URL", + Vals: v1alpha1.ClusterConfigSpec{ + Nutanix: &v1alpha1.NutanixSpec{ + PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ + URL: "not-a-valid-url", Insecure: false, Credentials: corev1.LocalObjectReference{ Name: "credentials", @@ -68,8 +145,7 @@ func TestVariableValidation(t *testing.T) { Vals: v1alpha1.ClusterConfigSpec{ Nutanix: &v1alpha1.NutanixSpec{ PrismCentralEndpoint: v1alpha1.NutanixPrismCentralEndpointSpec{ - Host: "prism-central.nutanix.com", - Port: v1alpha1.PrismCentralPort, + URL: fmt.Sprintf("https://prism-central.nutanix.com:%d", v1alpha1.DefaultPrismCentralPort), Insecure: false, }, // ControlPlaneEndpoint is a required field and must always be set