Move key defaulting to CRDs

This patch moves the defaulting logic of the secret selector keys to the CRDs instead of relying on a mutating webhook Signed-off-by: Sagar Muchhal <[email protected]>
vmware-tanzu · Dec 15, 2023 · 31ae125 · 31ae125
1 parent 8f7f7b7
commit 31ae125
Show file tree

Hide file tree

Showing 8 changed files with 94 additions and 133 deletions.
diff --git a/api/v1alpha2/sysprep/sysprep.go b/api/v1alpha2/sysprep/sysprep.go
@@ -5,10 +5,6 @@
 
 package sysprep
 
-import (
-	"github.com/vmware-tanzu/vm-operator/api/v1alpha2/common"
-)
-
 // Sysprep describes the object representation of a Windows sysprep.xml answer
 // file.
 //
@@ -94,7 +90,7 @@ type GUIUnattended struct {
 	// `password`.
 	//
 	// +optional
-	Password *common.SecretKeySelector `json:"password,omitempty"`
+	Password *PasswordSecretKeySelector `json:"password,omitempty"`
 
 	// TimeZone is the time zone index for the virtual machine.
 	//
@@ -105,6 +101,16 @@ type GUIUnattended struct {
 	TimeZone int32 `json:"timeZone,omitempty"`
 }
 
+// PasswordSecretKeySelector references the password value from a Secret resource
+type PasswordSecretKeySelector struct {
+	// Name is the name of the secret.
+	Name string `json:"name"`
+
+	// Key is the key in the secret that specifies the requested data.
+	// +kubebuilder:default=password
+	Key string `json:"key"`
+}
+
 // Identification maps to the Identification key in the sysprep.xml answer file
 // and provides information needed to join a workgroup or domain.
 type Identification struct {
@@ -124,7 +130,7 @@ type Identification struct {
 	// `domain_admin_password`.
 	//
 	// +optional
-	DomainAdminPassword *common.SecretKeySelector `json:"domainAdminPassword,omitempty"`
+	DomainAdminPassword *DomainPasswordSecretKeySelector `json:"domainAdminPassword,omitempty"`
 
 	// JoinDomain is the domain that the virtual machine should join. If this
 	// value is supplied, then DomainAdmin and DomainAdminPassword must also be
@@ -141,6 +147,16 @@ type Identification struct {
 	JoinWorkgroup string `json:"joinWorkgroup,omitempty"`
 }
 
+// DomainPasswordSecretKeySelector references the password value from a Secret resource
+type DomainPasswordSecretKeySelector struct {
+	// Name is the name of the secret.
+	Name string `json:"name"`
+
+	// Key is the key in the secret that specifies the requested data.
+	// +kubebuilder:default=domain_admin_password
+	Key string `json:"key"`
+}
+
 // CustomizationLicenseDataMode is an enumeration of the different license
 // modes.
 //
@@ -198,5 +214,15 @@ type UserData struct {
 	// `domain_admin_password`.
 	//
 	// +optional
-	ProductID *common.SecretKeySelector `json:"productID,omitempty"`
+	ProductID *ProductIDSecretKeySelector `json:"productID,omitempty"`
+}
+
+// ProductIDSecretKeySelector references the ProductID value from a Secret resource
+type ProductIDSecretKeySelector struct {
+	// Name is the name of the secret.
+	Name string `json:"name"`
+
+	// Key is the key in the secret that specifies the requested data.
+	// +kubebuilder:default=product_id
+	Key string `json:"key"`
 }
diff --git a/api/v1alpha2/sysprep/zz_generated.deepcopy.go b/api/v1alpha2/sysprep/zz_generated.deepcopy.go
diff --git a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml
@@ -1121,6 +1121,7 @@ spec:
                                   to `password`."
                                 properties:
                                   key:
+                                    default: password
                                     description: Key is the key in the secret that
                                       specifies the requested data.
                                     type: string
@@ -1158,6 +1159,7 @@ spec:
                                   selector defaults to `domain_admin_password`."
                                 properties:
                                   key:
+                                    default: domain_admin_password
                                     description: Key is the key in the secret that
                                       specifies the requested data.
                                     type: string
@@ -1224,6 +1226,7 @@ spec:
                                   selector defaults to `domain_admin_password`."
                                 properties:
                                   key:
+                                    default: product_id
                                     description: Key is the key in the secret that
                                       specifies the requested data.
                                     type: string

diff --git a/pkg/vmprovider/providers/vsphere2/sysprep/secret_test.go b/pkg/vmprovider/providers/vsphere2/sysprep/secret_test.go
@@ -16,7 +16,6 @@ import (
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
-	"github.com/vmware-tanzu/vm-operator/api/v1alpha2/common"
 	vmopv1sysprep "github.com/vmware-tanzu/vm-operator/api/v1alpha2/sysprep"
 	"github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere2/sysprep"
 )
@@ -55,7 +54,7 @@ var _ = Describe("CloudConfig GetCloudConfigSecretData", func() {
 		BeforeEach(func() {
 			inlineSysprep = vmopv1sysprep.Sysprep{
 				UserData: &vmopv1sysprep.UserData{
-					ProductID: &common.SecretKeySelector{
+					ProductID: &vmopv1sysprep.ProductIDSecretKeySelector{
 						Name: productIDSecretName,
 						Key:  "product_id",
 					},
@@ -120,7 +119,7 @@ var _ = Describe("CloudConfig GetCloudConfigSecretData", func() {
 			inlineSysprep = vmopv1sysprep.Sysprep{
 				GUIUnattended: &vmopv1sysprep.GUIUnattended{
 					AutoLogon: true,
-					Password: &common.SecretKeySelector{
+					Password: &vmopv1sysprep.PasswordSecretKeySelector{
 						Name: pwdSecretName,
 						Key:  "password",
 					},
@@ -173,7 +172,7 @@ var _ = Describe("CloudConfig GetCloudConfigSecretData", func() {
 			inlineSysprep = vmopv1sysprep.Sysprep{
 				Identification: &vmopv1sysprep.Identification{
 					JoinDomain: "foo",
-					DomainAdminPassword: &common.SecretKeySelector{
+					DomainAdminPassword: &vmopv1sysprep.DomainPasswordSecretKeySelector{
 						Name: pwdSecretName,
 						Key:  "domain_password",
 					},

diff --git a/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep_test.go b/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep_test.go
@@ -106,7 +106,7 @@ var _ = Describe("SysPrep Bootstrap", func() {
 					GUIUnattended: &vmopv1sysprep.GUIUnattended{
 						AutoLogon:      true,
 						AutoLogonCount: 2,
-						Password: &common.SecretKeySelector{
+						Password: &vmopv1sysprep.PasswordSecretKeySelector{
 							// omitting the name of the secret, since it does not get used
 							// in this function
 							Key: "pwd_key",
@@ -116,7 +116,7 @@ var _ = Describe("SysPrep Bootstrap", func() {
 					UserData: &vmopv1sysprep.UserData{
 						FullName:  "foo-bar",
 						OrgName:   "foo-org",
-						ProductID: &common.SecretKeySelector{Key: "product_id_key"},
+						ProductID: &vmopv1sysprep.ProductIDSecretKeySelector{Key: "product_id_key"},
 					},
 					GUIRunOnce: vmopv1sysprep.GUIRunOnce{
 						Commands: []string{"blah", "boom"},
@@ -125,7 +125,7 @@ var _ = Describe("SysPrep Bootstrap", func() {
 						DomainAdmin:         "[Foo/Administrator]",
 						JoinDomain:          "foo.local",
 						JoinWorkgroup:       "foo.local.wg",
-						DomainAdminPassword: &common.SecretKeySelector{Key: "admin_pwd_key"},
+						DomainAdminPassword: &vmopv1sysprep.DomainPasswordSecretKeySelector{Key: "admin_pwd_key"},
 					},
 					LicenseFilePrintData: &vmopv1sysprep.LicenseFilePrintData{
 						AutoMode:  vmopv1sysprep.CustomizationLicenseDataModePerServer,

diff --git a/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils_test.go b/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils_test.go
@@ -379,7 +379,7 @@ func vmUtilTests() {
 				BeforeEach(func() {
 					vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep = &sysprep.Sysprep{
 						UserData: &sysprep.UserData{
-							ProductID: &common.SecretKeySelector{
+							ProductID: &sysprep.ProductIDSecretKeySelector{
 								Name: productIDSecretName,
 								Key:  "product_id",
 							},
@@ -449,7 +449,7 @@ func vmUtilTests() {
 					vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep = &sysprep.Sysprep{
 						GUIUnattended: &sysprep.GUIUnattended{
 							AutoLogon: true,
-							Password: &common.SecretKeySelector{
+							Password: &sysprep.PasswordSecretKeySelector{
 								Name: pwdSecretName,
 								Key:  "password",
 							},
@@ -507,7 +507,7 @@ func vmUtilTests() {
 					vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep = &sysprep.Sysprep{
 						Identification: &sysprep.Identification{
 							JoinDomain: "foo",
-							DomainAdminPassword: &common.SecretKeySelector{
+							DomainAdminPassword: &sysprep.DomainPasswordSecretKeySelector{
 								Name: pwdSecretName,
 								Key:  "domain_password",
 							},

diff --git a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go
@@ -119,9 +119,6 @@ func (m mutator) Mutate(ctx *context.WebhookRequestContext) admission.Response {
 		} else if mutated {
 			wasMutated = true
 		}
-		if modified.Spec.Bootstrap != nil && SetDefaultSysprepKeys(modified) {
-			wasMutated = true
-		}
 	case admissionv1.Update:
 		oldVM, err := m.vmFromUnstructured(ctx.OldObj)
 		if err != nil {
@@ -330,36 +327,3 @@ func ResolveImageName(
 	vm.Spec.ImageName = determinedImageName
 	return true, nil
 }
-
-func SetDefaultSysprepKeys(vm *vmopv1.VirtualMachine) bool {
-	if vm.Spec.Bootstrap.Sysprep == nil || vm.Spec.Bootstrap.Sysprep.Sysprep == nil {
-		return false
-	}
-
-	wasMutated := false
-	inlineSysprep := vm.Spec.Bootstrap.Sysprep.Sysprep
-	if inlineSysprep.GUIUnattended != nil {
-		if inlineSysprep.GUIUnattended.Password != nil && inlineSysprep.GUIUnattended.Password.Key == "" {
-			inlineSysprep.GUIUnattended.Password.Key = "password"
-			wasMutated = true
-		}
-	}
-
-	if inlineSysprep.Identification != nil {
-		if inlineSysprep.Identification.JoinDomain != "" &&
-			inlineSysprep.Identification.DomainAdminPassword != nil &&
-			inlineSysprep.Identification.DomainAdminPassword.Key == "" {
-			inlineSysprep.Identification.DomainAdminPassword.Key = "domain_admin_password"
-			wasMutated = true
-		}
-	}
-
-	if inlineSysprep.UserData != nil && inlineSysprep.UserData.ProductID != nil {
-		if inlineSysprep.UserData.ProductID.Key == "" {
-			inlineSysprep.UserData.ProductID.Key = "product_id"
-			wasMutated = true
-		}
-	}
-
-	return wasMutated
-}