From 9e5bcad61bc2c13f066d0e1b3e16bae36bad1197 Mon Sep 17 00:00:00 2001 From: Fangchi Wang Date: Thu, 27 Aug 2026 10:51:41 +0800 Subject: [PATCH 1/2] Add validation markers and CEL tests to AviLoadBalancerConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. AviLoadBalancerConfig validation markers: - spec.cloudName: add MinLength=1 constraint (+kubebuilder:validation:MinLength=1) - spec.credentialSecretRef.name: add canonical DNS-1123 subdomain validation (+kubebuilder:validation:MinLength=1, MaxLength=253, Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) 2. envtest test suite: - add test/cel/aviloadbalancerconfig_test.go covering valid admission, empty server admission, cloudName defaulting vs. MinLength=1 rejection, and credentialSecretRef.name pattern validation. Testing Done: - make generate — regenerated CRD manifests and client code cleanly - make test-cel — all CEL envtest cases pass against kube-apiserver - make test-unit — all unit tests pass Signed-off-by: Fangchi Wang --- api/v1alpha1/aviloadbalancerconfig_types.go | 2 + api/v1alpha1/loadbalancerconfig_types.go | 5 + test/cel/aviloadbalancerconfig_test.go | 191 ++++++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 test/cel/aviloadbalancerconfig_test.go diff --git a/api/v1alpha1/aviloadbalancerconfig_types.go b/api/v1alpha1/aviloadbalancerconfig_types.go index 27c12cb..d20d681 100644 --- a/api/v1alpha1/aviloadbalancerconfig_types.go +++ b/api/v1alpha1/aviloadbalancerconfig_types.go @@ -50,7 +50,9 @@ type AviLoadBalancerConfigSpec struct { // CloudName is used by the Avi Kubernetes Operator (AKO) when querying // properties via the Avi REST API, ex. /api/cloud/?name=CLOUD_NAME. // Defaults to Default-Cloud. + // // +kubebuilder:default:=Default-Cloud + // +kubebuilder:validation:MinLength=1 CloudName string `json:"cloudName,omitempty"` // AdvancedL4 is a flag that enables support for WCP in AKO. diff --git a/api/v1alpha1/loadbalancerconfig_types.go b/api/v1alpha1/loadbalancerconfig_types.go index c00c393..70fcfce 100644 --- a/api/v1alpha1/loadbalancerconfig_types.go +++ b/api/v1alpha1/loadbalancerconfig_types.go @@ -13,6 +13,11 @@ import ( // which contains credential specifications for a load balancer. type ClientSecretReference struct { // Name is the name of resource being referenced. + // It must conform to DNS-1123 subdomain format. + // + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` Name string `json:"name"` // Namespace of the resource being referenced. If empty, cluster scoped resource is assumed. // +kubebuilder:default:=default diff --git a/test/cel/aviloadbalancerconfig_test.go b/test/cel/aviloadbalancerconfig_test.go new file mode 100644 index 0000000..e199879 --- /dev/null +++ b/test/cel/aviloadbalancerconfig_test.go @@ -0,0 +1,191 @@ +// © Broadcom. All Rights Reserved. +// The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. +// SPDX-License-Identifier: Apache-2.0 + +package cel_test + +import ( + "strings" + "testing" + + netv1alpha1 "github.com/vmware-tanzu/net-operator-api/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func validAviLoadBalancerConfig(name string) *netv1alpha1.AviLoadBalancerConfig { + return &netv1alpha1.AviLoadBalancerConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: netv1alpha1.AviLoadBalancerConfigSpec{ + Server: "https://10.0.0.1", + CloudName: "Default-Cloud", + CredentialSecretRef: netv1alpha1.ClientSecretReference{ + Name: "avi-creds", + Namespace: "default", + }, + }, + } +} + +func TestAviLoadBalancerConfig_Valid_Admitted(t *testing.T) { + obj := validAviLoadBalancerConfig("avic-valid") + if err := k8sClient.Create(testCtx, obj); err != nil { + t.Fatalf("expected admission, got: %v", err) + } + _ = k8sClient.Delete(testCtx, obj) +} + +// TestAviLoadBalancerConfig_EmptyServer_Admitted verifies that API admits an empty Server string. +// spec.server has no omitempty tag, so "" is sent even if field is unset by a structured/typed client. +func TestAviLoadBalancerConfig_EmptyServer_Admitted(t *testing.T) { + obj := validAviLoadBalancerConfig("avic-empty-server") + obj.Spec.Server = "" + if err := k8sClient.Create(testCtx, obj); err != nil { + t.Fatalf("expected admission for empty server, got: %v", err) + } + _ = k8sClient.Delete(testCtx, obj) +} + +// TestAviLoadBalancerConfig_OmittedCloudName_DefaultedOnReadback verifies that omitting cloudName +// via the typed Go struct (CloudName zero value + omitempty tag) is admitted, and that the field +// is defaulted to "Default-Cloud" on readback. The Go marshaler omits a zero-value CloudName from +// the JSON payload, so the API server applies the kubebuilder default before persisting. +func TestAviLoadBalancerConfig_OmittedCloudName_DefaultedOnReadback(t *testing.T) { + obj := &netv1alpha1.AviLoadBalancerConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "avic-omitted-cloud"}, + Spec: netv1alpha1.AviLoadBalancerConfigSpec{ + Server: "https://10.0.0.1", + // CloudName is intentionally the zero value; omitempty omits it from the JSON + // payload so the server applies the "Default-Cloud" default. + CredentialSecretRef: netv1alpha1.ClientSecretReference{ + Name: "avi-creds", + Namespace: "default", + }, + }, + } + if err := k8sClient.Create(testCtx, obj); err != nil { + t.Fatalf("expected admission for omitted cloudName, got: %v", err) + } + t.Cleanup(func() { _ = k8sClient.Delete(testCtx, obj) }) + + got := &netv1alpha1.AviLoadBalancerConfig{} + if err := k8sClient.Get(testCtx, client.ObjectKeyFromObject(obj), got); err != nil { + t.Fatalf("failed to read back object: %v", err) + } + if got.Spec.CloudName != "Default-Cloud" { + t.Errorf("expected cloudName defaulted to %q, got %q", "Default-Cloud", got.Spec.CloudName) + } +} + +// TestAviLoadBalancerConfig_ExplicitEmptyCloudName_DefaultedOnReadback verifies that a typed Go +// client that explicitly sets CloudName = "" is admitted and receives the "Default-Cloud" default +// on readback. Setting "" is indistinguishable from omitting the field: omitempty drops both from +// the JSON payload, so MinLength=1 never fires and the API server applies the kubebuilder default. +func TestAviLoadBalancerConfig_ExplicitEmptyCloudName_DefaultedOnReadback(t *testing.T) { + obj := validAviLoadBalancerConfig("avic-explicit-empty-cloud") + obj.Spec.CloudName = "" // explicitly zeroed — omitempty will omit this from the wire + if err := k8sClient.Create(testCtx, obj); err != nil { + t.Fatalf("expected admission for explicit empty cloudName via typed client, got: %v", err) + } + t.Cleanup(func() { _ = k8sClient.Delete(testCtx, obj) }) + + got := &netv1alpha1.AviLoadBalancerConfig{} + if err := k8sClient.Get(testCtx, client.ObjectKeyFromObject(obj), got); err != nil { + t.Fatalf("failed to read back object: %v", err) + } + if got.Spec.CloudName != "Default-Cloud" { + t.Errorf("expected cloudName defaulted to %q, got %q", "Default-Cloud", got.Spec.CloudName) + } +} + +// TestAviLoadBalancerConfig_EmptyCloudName_Rejected verifies that an explicit empty cloudName string +// is rejected by MinLength=1. Because the Go struct field carries omitempty, +// the Go JSON marshaler omits an empty CloudName and the server applies the +// "Default-Cloud" default instead. We use an Unstructured object to bypass this +// and send "cloudName": "" explicitly so that the server's default is not applied. +func TestAviLoadBalancerConfig_EmptyCloudName_Rejected(t *testing.T) { + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "netoperator.vmware.com/v1alpha1", + "kind": "AviLoadBalancerConfig", + "metadata": map[string]interface{}{ + "name": "avic-empty-cloud", + }, + "spec": map[string]interface{}{ + "server": "https://10.0.0.1", + "cloudName": "", + "credentialSecretRef": map[string]interface{}{ + "name": "avi-creds", + "namespace": "default", + }, + }, + }, + } + if err := k8sClient.Create(testCtx, obj); !isRejected(err) { + t.Fatalf("expected rejection for explicit empty cloudName, got: %v", err) + _ = k8sClient.Delete(testCtx, obj) + } +} + +// TestAviLoadBalancerConfig_ValidCloudNameAndServer_Admitted verifies a typical non-default cloudName +// and explicit server URL are admitted. +func TestAviLoadBalancerConfig_ValidCloudNameAndServer_Admitted(t *testing.T) { + obj := validAviLoadBalancerConfig("avic-named-cloud") + obj.Spec.CloudName = "my-cloud" + obj.Spec.Server = "https://10.1.2.3" + if err := k8sClient.Create(testCtx, obj); err != nil { + t.Fatalf("expected admission for valid cloudName and server, got: %v", err) + } + _ = k8sClient.Delete(testCtx, obj) +} + +func TestAviLoadBalancerConfig_ValidCredentialSecretRefName_Admitted(t *testing.T) { + obj := validAviLoadBalancerConfig("avic-dot-secret") + obj.Spec.CredentialSecretRef.Name = "avi.creds.v1" + if err := k8sClient.Create(testCtx, obj); err != nil { + t.Fatalf("expected admission for dot-separated DNS-1123 secret name, got: %v", err) + } + _ = k8sClient.Delete(testCtx, obj) +} + +func TestAviLoadBalancerConfig_InvalidCredentialSecretRefName_Rejected(t *testing.T) { + testCases := []struct { + name string + secretName string + }{ + { + name: "uppercase characters", + secretName: "Invalid-Secret-Name", + }, + { + name: "trailing hyphen", + secretName: "bad-secret-", + }, + { + name: "leading hyphen", + secretName: "-bad-secret", + }, + { + name: "consecutive dots", + secretName: "bad..secret", + }, + { + name: "empty name", + secretName: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + obj := validAviLoadBalancerConfig("avic-bad-secret-" + strings.ReplaceAll(tc.name, " ", "-")) + obj.Spec.CredentialSecretRef.Name = tc.secretName + if err := k8sClient.Create(testCtx, obj); !isRejected(err) { + t.Fatalf("expected rejection for invalid credentialSecretRef.name (%s), got: %v", tc.name, err) + _ = k8sClient.Delete(testCtx, obj) + } + }) + } +} From 54adbd4ba063840ff45295eb5600a49de43b81b3 Mon Sep 17 00:00:00 2001 From: Fangchi Wang Date: Tue, 8 Sep 2026 14:45:04 +0800 Subject: [PATCH 2/2] api/avic: add certificateAuthorityData to AviLoadBalancerConfigSpec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add optional CertificateAuthorityData field to AviLoadBalancerConfigSpec to store PEM-encoded CA certificates used to verify the Avi Controller's TLS certificate. - Enables synchronous certificate format validation in admission webhooks on both Create and Update operations without depending on external Secret lifecycle timing. - Maintains backward compatibility: if empty or omitted, controllers fall back to reading the CA certificate from the Secret referenced by CredentialSecretRef. Testing Done: - make generate — regenerated CRD manifests and deepcopy stubs - make test-unit — unit tests pass Signed-off-by: Fangchi Wang --- api/v1alpha1/aviloadbalancerconfig_types.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/api/v1alpha1/aviloadbalancerconfig_types.go b/api/v1alpha1/aviloadbalancerconfig_types.go index d20d681..e51f3a0 100644 --- a/api/v1alpha1/aviloadbalancerconfig_types.go +++ b/api/v1alpha1/aviloadbalancerconfig_types.go @@ -70,6 +70,14 @@ type AviLoadBalancerConfigSpec struct { // +kubebuilder:validation:Enum=controller;supervisor IPAMType AviLoadBalancerIPAMType `json:"ipamType,omitempty"` + // CertificateAuthorityData contains PEM-encoded certificate authority + // certificates used to verify the Avi Controller's TLS certificate. + // If empty, the certificate authority certificate stored in the Secret + // referenced by CredentialSecretRef will be used. + // + // +optional + CertificateAuthorityData string `json:"certificateAuthorityData,omitempty"` + // CredentialSecretRef points to a Secret resource used to access and // configure the Avi Controller. //