diff --git a/api/v1alpha1/condition_consts.go b/api/v1alpha1/condition_consts.go index 3af2565d7..a6bff3ac3 100644 --- a/api/v1alpha1/condition_consts.go +++ b/api/v1alpha1/condition_consts.go @@ -11,30 +11,6 @@ const ( // Conditions and condition Reasons for the VirtualMachine object. -const ( - // VirtualMachineMetadataReadyCondition documents the state of the metadata passed to the VirtualMachine. - VirtualMachineMetadataReadyCondition ConditionType = "VirtualMachineMetadataReady" - - // VirtualMachineMetadataDuplicatedReason (Severity=Error) documents that the mutually exclusive Secret and ConfigMap are - // specified in the VirtualMachineMetadata. - // - // This validation is performed by the validation webhook, defensively adding the reason. - VirtualMachineMetadataDuplicatedReason = "VirtualMachineMetadataDuplicated" - - // VirtualMachineMetadataNotFoundReason (Severity=Error) documents that the Secret or ConfigMap specified in the VirtualMachineSpec - // cannot be found. - VirtualMachineMetadataNotFoundReason = "VirtualMachineMetadataNotFound" - - // VirtualMachineMetadataFormatInvalidReason (Severity=Error) documents that the format of the supplied metadata is invalid. - // - // It is used to indicate issues with the formatting of the metadata irrespective of the Transport in use. - VirtualMachineMetadataFormatInvalidReason = "VirtualMachineMetadataFormatInvalid" - - // VirtualMachineGuestInfoSizeExceededReason (Severity=Error) documents that the supplied Cloud Init metadata exceeds the - // maximum allowed capacity. - VirtualMachineGuestInfoSizeExceededReason = "VirtualMachineGuestInfoSizeExceeded" -) - const ( // VirtualMachinePrereqReadyCondition documents that all of a VirtualMachine's prerequisites declared in the spec // (e.g. VirtualMachineClass) are satisfied. diff --git a/api/v1alpha2/virtualmachine_types.go b/api/v1alpha2/virtualmachine_types.go index 0096a9b5e..4773b8fca 100644 --- a/api/v1alpha2/virtualmachine_types.go +++ b/api/v1alpha2/virtualmachine_types.go @@ -10,32 +10,6 @@ import ( "github.com/vmware-tanzu/vm-operator/api/v1alpha2/common" ) -// TODO: Since this API version replaces the concept of metadata to BootstrapSpec, rename the types once -// these reasons before using them in Conditions. Keeping the reasons the same for the sake of consistency. -const ( - // VirtualMachineMetadataReadyCondition documents the state of the metadata passed to the VirtualMachine. - VirtualMachineMetadataReadyCondition = "VirtualMachineMetadataReady" - - // VirtualMachineMetadataDuplicatedReason (Severity=Error) documents that the mutually exclusive Secret and ConfigMap are - // specified in the VirtualMachineMetadata. - // - // This validation is performed by the validation webhook, defensively adding the reason. - VirtualMachineMetadataDuplicatedReason = "VirtualMachineMetadataDuplicated" - - // VirtualMachineMetadataNotFoundReason (Severity=Error) documents that the Secret or ConfigMap specified in the VirtualMachineSpec - // cannot be found. - VirtualMachineMetadataNotFoundReason = "VirtualMachineMetadataNotFound" - - // VirtualMachineMetadataFormatInvalidReason (Severity=Error) documents that the format of the supplied metadata is invalid. - // - // It is used to indicate issues with the formatting of the metadata irrespective of the Transport in use. - VirtualMachineMetadataFormatInvalidReason = "VirtualMachineMetadataFormatInvalid" - - // VirtualMachineGuestInfoSizeExceededReason (Severity=Error) documents that the supplied Cloud Init metadata exceeds the - // maximum allowed capacity. - VirtualMachineGuestInfoSizeExceededReason = "VirtualMachineGuestInfoSizeExceeded" -) - const ( // VirtualMachineConditionClassReady indicates that a referenced // VirtualMachineClass is ready. diff --git a/go.mod b/go.mod index e133dd9d2..fa68ca8eb 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,6 @@ replace ( ) require ( - github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 github.com/davecgh/go-spew v1.1.1 github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1 github.com/go-logr/logr v1.2.3 diff --git a/go.sum b/go.sum index e6a545e6d..70f60ede0 100644 --- a/go.sum +++ b/go.sum @@ -54,8 +54,6 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= -github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 h1:s6gZFSlWYmbqAuRjVTiNNhvNRfY2Wxp9nhfyel4rklc= -github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137/go.mod h1:OMCwj8VM1Kc9e19TLln2VL61YJF0x1XFtfdL4JdbSyE= github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o= github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY= github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= diff --git a/pkg/vmprovider/providers/vsphere/constants/constants.go b/pkg/vmprovider/providers/vsphere/constants/constants.go index 192ddc8d5..660d152b8 100644 --- a/pkg/vmprovider/providers/vsphere/constants/constants.go +++ b/pkg/vmprovider/providers/vsphere/constants/constants.go @@ -4,8 +4,6 @@ package constants import ( - "github.com/alecthomas/units" - "github.com/vmware-tanzu/vm-operator/pkg" ) @@ -74,12 +72,6 @@ const ( CloudInitGuestInfoUserdata = "guestinfo.userdata" CloudInitGuestInfoUserdataEncoding = "guestinfo.userdata.encoding" - // CloudInitGuestInfoMaxSize the maximum allowed size for CloudInit metadata. - // As defined in the https://github.com/vmware/open-vm-tools/blob/42ce53f39be45b7795a9f1adf892af84629b4a19/open-vm-tools/lib/include/guest_msg_def.h#L100-L104 - // - // The human-readable value for the constant equates to 64 Kibibytes, or 64KiB. - CloudInitGuestInfoMaxSize = 64 * units.KiB - // InstanceStoragePVCNamePrefix prefix of auto-generated PVC names. InstanceStoragePVCNamePrefix = "instance-pvc-" // InstanceStorageLabelKey identifies resources related to instance storage. diff --git a/pkg/vmprovider/providers/vsphere/session/session_vm_customization.go b/pkg/vmprovider/providers/vsphere/session/session_vm_customization.go index 9ddab7ae8..5c7e38251 100644 --- a/pkg/vmprovider/providers/vsphere/session/session_vm_customization.go +++ b/pkg/vmprovider/providers/vsphere/session/session_vm_customization.go @@ -17,7 +17,6 @@ import ( apiEquality "k8s.io/apimachinery/pkg/api/equality" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" - "github.com/vmware-tanzu/vm-operator/pkg/conditions" "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/lib" "github.com/vmware-tanzu/vm-operator/pkg/util" @@ -115,16 +114,9 @@ func GetCloudInitMetadata(vm *vmopv1.VirtualMachine, return string(metadataBytes), nil } -// ErrWithReason encapsulates the error type as well as a human-readable reason that will be -// used to set the Reason field on the Condition set on the object. -type ErrWithReason struct { - error - conditionReason string -} - func GetCloudInitPrepCustSpec( cloudInitMetadata string, - updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, *vimTypes.CustomizationSpec, *ErrWithReason) { + updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, *vimTypes.CustomizationSpec, error) { userdata := updateArgs.VMMetadata.Data["user-data"] @@ -132,10 +124,7 @@ func GetCloudInitPrepCustSpec( // Ensure the data is normalized first to plain-text. plainText, err := util.TryToDecodeBase64Gzip([]byte(userdata)) if err != nil { - return nil, nil, &ErrWithReason{ - error: fmt.Errorf("decoding cloud-init prep userdata failed %v", err), - conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason, - } + return nil, nil, fmt.Errorf("decoding cloud-init prep userdata failed %v", err) } userdata = plainText } @@ -158,27 +147,17 @@ func GetCloudInitPrepCustSpec( func GetCloudInitGuestInfoCustSpec( cloudInitMetadata string, config *vimTypes.VirtualMachineConfigInfo, - updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, *ErrWithReason) { + updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, error) { extraConfig := map[string]string{} encodedMetadata, err := EncodeGzipBase64(cloudInitMetadata) if err != nil { - return nil, &ErrWithReason{ - error: fmt.Errorf("encoding cloud-init metadata failed %v", err), - conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason, - } + return nil, fmt.Errorf("encoding cloud-init metadata failed %v", err) } extraConfig[constants.CloudInitGuestInfoMetadata] = encodedMetadata extraConfig[constants.CloudInitGuestInfoMetadataEncoding] = "gzip+base64" - if len(encodedMetadata) > int(constants.CloudInitGuestInfoMaxSize) { - return nil, &ErrWithReason{ - error: fmt.Errorf("size of cloud-init guest info exceeds the maximum allowed size of %s", constants.CloudInitGuestInfoMaxSize.String()), - conditionReason: vmopv1.VirtualMachineGuestInfoSizeExceededReason, - } - } - var data string // Check for the 'user-data' key as per official contract and API documentation. // Additionally, To support the cluster bootstrap data supplied by CAPBK's secret, @@ -194,30 +173,16 @@ func GetCloudInitGuestInfoCustSpec( // Ensure the data is normalized first to plain-text. plainText, err := util.TryToDecodeBase64Gzip([]byte(data)) if err != nil { - return nil, &ErrWithReason{ - error: fmt.Errorf("decoding cloud-init userdata failed %v", err), - conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason, - } + return nil, fmt.Errorf("decoding cloud-init userdata failed %v", err) } encodedUserdata, err := EncodeGzipBase64(plainText) if err != nil { - return nil, &ErrWithReason{ - error: fmt.Errorf("encoding cloud-init userdata failed %v", err), - conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason, - } + return nil, fmt.Errorf("encoding cloud-init userdata failed %v", err) } extraConfig[constants.CloudInitGuestInfoUserdata] = encodedUserdata extraConfig[constants.CloudInitGuestInfoUserdataEncoding] = "gzip+base64" - - if len(encodedUserdata) > int(constants.CloudInitGuestInfoMaxSize) || - len(encodedUserdata)+len(encodedMetadata) > int(constants.CloudInitGuestInfoMaxSize) { - return nil, &ErrWithReason{ - error: fmt.Errorf("size of cloud-init guest info exceeds the maximum allowed size of %s", constants.CloudInitGuestInfoMaxSize.String()), - conditionReason: vmopv1.VirtualMachineGuestInfoSizeExceededReason, - } - } } configSpec := &vimTypes.VirtualMachineConfigSpec{} @@ -280,11 +245,6 @@ func customizeCloudInit( cloudInitMetadata, err := GetCloudInitMetadata(vmCtx.VM, netplan, updateArgs.VMMetadata.Data) if err != nil { - conditions.MarkFalse(vmCtx.VM, - vmopv1.VirtualMachineMetadataReadyCondition, - vmopv1.VirtualMachineMetadataFormatInvalidReason, - vmopv1.ConditionSeverityError, - err.Error()) return nil, nil, err } @@ -301,12 +261,6 @@ func customizeCloudInit( } if err != nil { - errWithReason := err.(ErrWithReason) - conditions.MarkFalse(vmCtx.VM, - vmopv1.VirtualMachineMetadataReadyCondition, - errWithReason.conditionReason, - vmopv1.ConditionSeverityError, - errWithReason.Error()) return nil, nil, err } @@ -353,7 +307,6 @@ func (s *Session) customize( if err != nil { return err } - conditions.MarkTrue(vmCtx.VM, vmopv1.VirtualMachineMetadataReadyCondition) if configSpec != nil { defaultConfigSpec := &vimTypes.VirtualMachineConfigSpec{} diff --git a/pkg/vmprovider/providers/vsphere/session/session_vm_customization_test.go b/pkg/vmprovider/providers/vsphere/session/session_vm_customization_test.go index cc29982d6..d79a02b9b 100644 --- a/pkg/vmprovider/providers/vsphere/session/session_vm_customization_test.go +++ b/pkg/vmprovider/providers/vsphere/session/session_vm_customization_test.go @@ -7,7 +7,6 @@ import ( goctx "context" "encoding/base64" "fmt" - "math/rand" "strings" . "github.com/onsi/ginkgo" @@ -403,59 +402,6 @@ var _ = Describe("Cloud-Init Customization", func() { }) }) - Context("With cloud init guest info exceeding the maximum size", func() { - generateDataFunc := func(length int) string { - charset := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" - - sb := strings.Builder{} - sb.Grow(length) - for i := 0; i < length; i++ { - sb.WriteByte(charset[rand.Intn(len(charset))]) //nolint:gosec - } - return sb.String() - } - - Context("With cloud init user data exceeding the maximum size", func() { - BeforeEach(func() { - // Doubling the input to the max allowed size to make sure the compressed and encoded - // output is still above the allowed limit. - data, err := session.EncodeGzipBase64(generateDataFunc(128 * 1000)) - Expect(err).ToNot(HaveOccurred()) - updateArgs.VMMetadata.Data["user-data"] = data - }) - It("will return a limit exceeded error", func() { - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("exceeds the maximum allowed size of 64KiB")) - }) - }) - - Context("With cloud init metadata exceeding the maximum size", func() { - BeforeEach(func() { - // Doubling the input to the max allowed size to make sure the compressed and encoded - // output is still above the allowed limit. - data, err := session.EncodeGzipBase64(generateDataFunc(128 * 1000)) - Expect(err).ToNot(HaveOccurred()) - cloudInitMetadata = data - }) - It("will return a limit exceeded error", func() { - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("exceeds the maximum allowed size of 64KiB")) - }) - }) - - Context("With cloud init metadata + userdata exceeding the maximum size", func() { - BeforeEach(func() { - data, err := session.EncodeGzipBase64(generateDataFunc(64 * 1000)) - Expect(err).ToNot(HaveOccurred()) - cloudInitMetadata = data - updateArgs.VMMetadata.Data["user-data"] = data - }) - It("will return a limit exceeded error", func() { - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("exceeds the maximum allowed size of 64KiB")) - }) - }) - }) }) Context("GetCloudInitPrepCustSpec", func() { diff --git a/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go b/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go index 61bd15778..32965ceff 100644 --- a/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go +++ b/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go @@ -229,13 +229,6 @@ func GetVMMetadata( // The VM's MetaData ConfigMapName and SecretName are mutually exclusive. Validation webhook checks // this during create/update but double check it here. if metadata.ConfigMapName != "" && metadata.SecretName != "" { - conditions.MarkFalse(vmCtx.VM, - vmopv1.VirtualMachineMetadataReadyCondition, - vmopv1.VirtualMachineMetadataDuplicatedReason, - vmopv1.ConditionSeverityError, - "Both ConfigMapName %s/%s and SecretName %s/%s are specified", - metadata.ConfigMapName, vmCtx.VM.Namespace, - metadata.SecretName, vmCtx.VM.Namespace) return vmMD, errors.New("invalid VM Metadata: both ConfigMapName and SecretName are specified") } @@ -243,11 +236,7 @@ func GetVMMetadata( cm := &corev1.ConfigMap{} err := k8sClient.Get(vmCtx, ctrlclient.ObjectKey{Name: metadata.ConfigMapName, Namespace: vmCtx.VM.Namespace}, cm) if err != nil { - conditions.MarkFalse(vmCtx.VM, - vmopv1.VirtualMachineMetadataReadyCondition, - vmopv1.VirtualMachineMetadataNotFoundReason, - vmopv1.ConditionSeverityError, - "Unable to get metadata ConfigMap %s/%s", metadata.ConfigMapName, vmCtx.VM.Namespace) + // TODO: Condition return vmMD, errors.Wrap(err, "Failed to get VM Metadata ConfigMap") } @@ -257,11 +246,7 @@ func GetVMMetadata( secret := &corev1.Secret{} err := k8sClient.Get(vmCtx, ctrlclient.ObjectKey{Name: metadata.SecretName, Namespace: vmCtx.VM.Namespace}, secret) if err != nil { - conditions.MarkFalse(vmCtx.VM, - vmopv1.VirtualMachineMetadataReadyCondition, - vmopv1.VirtualMachineMetadataNotFoundReason, - vmopv1.ConditionSeverityError, - "Unable to get metadata Secret %s/%s", metadata.SecretName, vmCtx.VM.Namespace) + // TODO: Condition return vmMD, errors.Wrap(err, "Failed to get VM Metadata Secret") }