Merge pull request kubernetes-sigs#3099 from sbueringer/pr-implement-…

…vm-naming-strategy

✨ Implement VirtualMachine namingStrategy

apis/vmware/v1beta1/vspheremachine_types.go

@@ -87,6 +87,30 @@ type VSphereMachineSpec struct {
 	//
 	// +optional
 	MinHardwareVersion string `json:"minHardwareVersion,omitempty"`
+
+	// NamingStrategy allows configuring the naming strategy used when calculating the name of the VirtualMachine.
+	// +optional
+	NamingStrategy *VirtualMachineNamingStrategy `json:"namingStrategy,omitempty"`
+}
+
+// VirtualMachineNamingStrategy defines the naming strategy for the VirtualMachines.
+type VirtualMachineNamingStrategy struct {
+	// Template defines the template to use for generating the name of the VirtualMachine object.
+	// If not defined, it will fall back to `{{ .machine.name }}`.
+	// The templating has the following data available:
+	// * `.machine.name`: The name of the Machine object.
+	// The templating also has the following funcs available:
+	// * `trimSuffix`: same as strings.TrimSuffix
+	// * `trunc`: truncates a string, e.g. `trunc 2 "hello"` or `trunc -2 "hello"`
+	// Notes:
+	// * While the template offers some flexibility, we would like the name to link to the Machine name
+	//   to ensure better user experience when troubleshooting
+	// * Generated names must be valid Kubernetes names as they are used to create a VirtualMachine object
+	//   and usually also as the name of the Node object.
+	// * Names are automatically truncated at 63 characters. Please note that this can lead to name conflicts,
+	//   so we highly recommend to use a template which leads to a name shorter than 63 characters.
+	// +optional
+	Template *string `json:"template,omitempty"`
 }
 
 // VSphereMachineStatus defines the observed state of VSphereMachine.

config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml

@@ -76,6 +76,28 @@ spec:
                   of the VM is at least set to the specified value.
                   The expected format of the field is vmx-15.
                 type: string
+              namingStrategy:
+                description: NamingStrategy allows configuring the naming strategy
+                  used when calculating the name of the VirtualMachine.
+                properties:
+                  template:
+                    description: |-
+                      Template defines the template to use for generating the name of the VirtualMachine object.
+                      If not defined, it will fall back to `{{ .machine.name }}`.
+                      The templating has the following data available:
+                      * `.machine.name`: The name of the Machine object.
+                      The templating also has the following funcs available:
+                      * `trimSuffix`: same as strings.TrimSuffix
+                      * `trunc`: truncates a string, e.g. `trunc 2 "hello"` or `trunc -2 "hello"`
+                      Notes:
+                      * While the template offers some flexibility, we would like the name to link to the Machine name
+                        to ensure better user experience when troubleshooting
+                      * Generated names must be valid Kubernetes names as they are used to create a VirtualMachine object
+                        and usually also as the name of the Node object.
+                      * Names are automatically truncated at 63 characters. Please note that this can lead to name conflicts,
+                        so we highly recommend to use a template which leads to a name shorter than 63 characters.
+                    type: string
+                type: object
               powerOffMode:
                 default: hard
                 description: |-

config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml

@@ -72,6 +72,28 @@ spec:
                           of the VM is at least set to the specified value.
                           The expected format of the field is vmx-15.
                         type: string
+                      namingStrategy:
+                        description: NamingStrategy allows configuring the naming
+                          strategy used when calculating the name of the VirtualMachine.
+                        properties:
+                          template:
+                            description: |-
+                              Template defines the template to use for generating the name of the VirtualMachine object.
+                              If not defined, it will fall back to `{{ .machine.name }}`.
+                              The templating has the following data available:
+                              * `.machine.name`: The name of the Machine object.
+                              The templating also has the following funcs available:
+                              * `trimSuffix`: same as strings.TrimSuffix
+                              * `trunc`: truncates a string, e.g. `trunc 2 "hello"` or `trunc -2 "hello"`
+                              Notes:
+                              * While the template offers some flexibility, we would like the name to link to the Machine name
+                                to ensure better user experience when troubleshooting
+                              * Generated names must be valid Kubernetes names as they are used to create a VirtualMachine object
+                                and usually also as the name of the Node object.
+                              * Names are automatically truncated at 63 characters. Please note that this can lead to name conflicts,
+                                so we highly recommend to use a template which leads to a name shorter than 63 characters.
+                            type: string
+                        type: object
                       powerOffMode:
                         default: hard
                         description: |-

config/supervisor/webhook/manifests.yaml

@@ -52,3 +52,24 @@ webhooks:
     resources:
     - vspheremachines
   sideEffects: None
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachinetemplate
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: validation.vspheremachinetemplate.vmware.infrastructure.cluster.x-k8s.io
+  rules:
+  - apiGroups:
+    - vmware.infrastructure.cluster.x-k8s.io
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - vspheremachinetemplates
+  sideEffects: None

controllers/vmware/test/controllers_test.go

@@ -252,6 +252,9 @@ func getManager(cfg *rest.Config, networkProvider string, withWebhooks bool) man
 		}
 
 		if withWebhooks {
+			if err := (&vmwarewebhooks.VSphereMachineTemplateWebhook{}).SetupWebhookWithManager(mgr); err != nil {
+				return err
+			}
 			if err := (&vmwarewebhooks.VSphereMachineWebhook{}).SetupWebhookWithManager(mgr); err != nil {
 				return err
 			}

internal/webhooks/vmware/vspheremachine_test.go

@@ -26,8 +26,6 @@ import (
 )
 
 func TestVSphereMachine_ValidateUpdate(t *testing.T) {
-	g := NewWithT(t)
-
 	fakeProviderID := "fake-000000"
 	tests := []struct {
 		name              string
@@ -67,7 +65,9 @@ func TestVSphereMachine_ValidateUpdate(t *testing.T) {
 		},
 	}
 	for _, tc := range tests {
-		t.Run(tc.name, func(_ *testing.T) {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+
 			webhook := &VSphereMachineWebhook{}
 			_, err := webhook.ValidateUpdate(context.Background(), tc.oldVSphereMachine, tc.vsphereMachine)
 			if tc.wantErr {

internal/webhooks/vmware/vspheremachinetemplate.go

@@ -0,0 +1,108 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package vmware is the package for webhooks of vmware resources.
+package vmware
+
+import (
+	"context"
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/services/vmoperator"
+)
+
+// +kubebuilder:webhook:verbs=create;update,path=/validate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachinetemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=vmware.infrastructure.cluster.x-k8s.io,resources=vspheremachinetemplates,versions=v1beta1,name=validation.vspheremachinetemplate.vmware.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
+
+// VSphereMachineTemplateWebhook implements a validation webhook for VSphereMachineTemplate.
+type VSphereMachineTemplateWebhook struct{}
+
+var _ webhook.CustomValidator = &VSphereMachineTemplateWebhook{}
+
+func (webhook *VSphereMachineTemplateWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(&vmwarev1.VSphereMachineTemplate{}).
+		WithValidator(webhook).
+		Complete()
+}
+
+// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
+func (webhook *VSphereMachineTemplateWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	vSphereMachineTemplate, ok := obj.(*vmwarev1.VSphereMachineTemplate)
+	if !ok {
+		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereMachineTemplate but got a %T", obj))
+	}
+	return webhook.validate(ctx, nil, vSphereMachineTemplate)
+}
+
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
+func (webhook *VSphereMachineTemplateWebhook) ValidateUpdate(ctx context.Context, _, newRaw runtime.Object) (admission.Warnings, error) {
+	vSphereMachineTemplate, ok := newRaw.(*vmwarev1.VSphereMachineTemplate)
+	if !ok {
+		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereMachineTemplate but got a %T", newRaw))
+	}
+	return webhook.validate(ctx, nil, vSphereMachineTemplate)
+}
+
+func (webhook *VSphereMachineTemplateWebhook) validate(_ context.Context, _, newVSphereMachineTemplate *vmwarev1.VSphereMachineTemplate) (admission.Warnings, error) {
+	var allErrs field.ErrorList
+
+	// Validate namingStrategy
+	namingStrategy := newVSphereMachineTemplate.Spec.Template.Spec.NamingStrategy
+	if namingStrategy != nil &&
+		namingStrategy.Template != nil {
+		name, err := vmoperator.GenerateVirtualMachineName("machine", namingStrategy)
+		templateFldPath := field.NewPath("spec", "template", "spec", "namingStrategy", "template")
+		if err != nil {
+			allErrs = append(allErrs,
+				field.Invalid(
+					templateFldPath,
+					*namingStrategy.Template,
+					fmt.Sprintf("invalid VirtualMachine name template: %v", err),
+				),
+			)
+		} else {
+			// Note: This validates that the resulting name is a valid Kubernetes object name.
+			for _, err := range validation.IsDNS1123Subdomain(name) {
+				allErrs = append(allErrs,
+					field.Invalid(
+						templateFldPath,
+						*namingStrategy.Template,
+						fmt.Sprintf("invalid VirtualMachine name template, generated name is not a valid Kubernetes object name: %v", err),
+					),
+				)
+			}
+		}
+	}
+
+	if len(allErrs) > 0 {
+		return nil, apierrors.NewInvalid(vmwarev1.GroupVersion.WithKind("VSphereMachineTemplate").GroupKind(), newVSphereMachineTemplate.Name, allErrs)
+	}
+	return nil, nil
+}
+
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
+func (webhook *VSphereMachineTemplateWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
+	return nil, nil
+}

internal/webhooks/vmware/vspheremachinetemplate_test.go

@@ -0,0 +1,99 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package vmware
+
+import (
+	"context"
+	"testing"
+
+	. "github.com/onsi/gomega"
+	"k8s.io/utils/ptr"
+
+	vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
+)
+
+func TestVSphereMachineTemplate_Validate(t *testing.T) {
+	tests := []struct {
+		name           string
+		namingStrategy *vmwarev1.VirtualMachineNamingStrategy
+		wantErr        bool
+	}{
+		{
+			name:           "Should succeed if namingStrategy not set",
+			namingStrategy: nil,
+			wantErr:        false,
+		},
+		{
+			name: "Should succeed if namingStrategy.template not set",
+			namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{
+				Template: nil,
+			},
+			wantErr: false,
+		},
+		{
+			name: "Should succeed if namingStrategy.template is set to the fallback value",
+			namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{
+				Template: ptr.To[string]("{{ .machine.name }}"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "Should succeed if namingStrategy.template is set to the Windows example",
+			namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{
+				Template: ptr.To[string]("{{ if le (len .machine.name) 20 }}{{ .machine.name }}{{else}}{{ trimSuffix \"-\" (trunc 14 .machine.name) }}-{{ trunc -5 .machine.name }}{{end}}"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "Should fail if namingStrategy.template is set to an invalid template",
+			namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{
+				Template: ptr.To[string]("{{ invalid"),
+			},
+			wantErr: true,
+		},
+		{
+			name: "Should fail if namingStrategy.template is set to a valid template that renders an invalid name",
+			namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{
+				Template: ptr.To[string]("-{{ .machine.name }}"), // Leading - is not valid for names.
+			},
+			wantErr: true,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			vSphereMachineTemplate := &vmwarev1.VSphereMachineTemplate{
+				Spec: vmwarev1.VSphereMachineTemplateSpec{
+					Template: vmwarev1.VSphereMachineTemplateResource{
+						Spec: vmwarev1.VSphereMachineSpec{
+							NamingStrategy: tc.namingStrategy,
+						},
+					},
+				},
+			}
+
+			webhook := &VSphereMachineTemplateWebhook{}
+			_, err := webhook.validate(context.Background(), nil, vSphereMachineTemplate)
+			if tc.wantErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+			}
+		})
+	}
+}

main.go

@@ -379,6 +379,9 @@ func setupVAPIControllers(ctx context.Context, controllerCtx *capvcontext.Contro
 }
 
 func setupSupervisorControllers(ctx context.Context, controllerCtx *capvcontext.ControllerManagerContext, mgr ctrlmgr.Manager, tracker *remote.ClusterCacheTracker) error {
+	if err := (&vmwarewebhooks.VSphereMachineTemplateWebhook{}).SetupWebhookWithManager(mgr); err != nil {
+		return err
+	}
 	if err := (&vmwarewebhooks.VSphereMachineWebhook{}).SetupWebhookWithManager(mgr); err != nil {
 		return err
 	}