From 9b93aa5e0b567403c9fc6967123110933d84a164 Mon Sep 17 00:00:00 2001 From: Roman Sysoev Date: Fri, 30 Jan 2026 02:00:29 +0300 Subject: [PATCH 1/2] feat(vm): validate cloud init userdata Signed-off-by: Roman Sysoev --- crds/virtualmachines.yaml | 2 - .../service/provisioning_service.go | 114 ++++++++++++++++++ .../controller/vm/internal/provisioning.go | 97 +++------------ .../validators/provisioning_validator.go | 62 ++++++++++ .../pkg/controller/vm/vm_controller.go | 3 +- .../pkg/controller/vm/vm_webhook.go | 5 +- 6 files changed, 195 insertions(+), 88 deletions(-) create mode 100644 images/virtualization-artifact/pkg/controller/service/provisioning_service.go create mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/validators/provisioning_validator.go diff --git a/crds/virtualmachines.yaml b/crds/virtualmachines.yaml index c674078a22..ce9c242ffe 100644 --- a/crds/virtualmachines.yaml +++ b/crds/virtualmachines.yaml @@ -72,8 +72,6 @@ spec: [More information about `cloud-init` and configuration examples](https://cloudinit.readthedocs.io/en/latest/reference/examples.html). type: string - maxLength: 2048 - userDataRef: description: | Link to an existing resource with the `cloud-init` scenario. diff --git a/images/virtualization-artifact/pkg/controller/service/provisioning_service.go b/images/virtualization-artifact/pkg/controller/service/provisioning_service.go new file mode 100644 index 0000000000..ad8d723544 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/service/provisioning_service.go @@ -0,0 +1,114 @@ +/* +Copyright 2026 Flant JSC + +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 service + +import ( + "context" + "errors" + "fmt" + + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +const cloudInitUserMaxLen = 2048 + +type ProvisioningService struct { + reader client.Reader +} + +func NewProvisioningService(reader client.Reader) *ProvisioningService { + return &ProvisioningService{ + reader: reader, + } +} + +var ErrSecretIsNotValid = errors.New("secret is not valid") + +type SecretNotFoundError string + +func (e SecretNotFoundError) Error() string { + return fmt.Sprintf("secret %s not found", string(e)) +} + +type UnexpectedSecretTypeError string + +func (e UnexpectedSecretTypeError) Error() string { + return fmt.Sprintf("unexpected secret type: %s", string(e)) +} + +var cloudInitCheckKeys = []string{ + "userdata", + "userData", +} + +func (p *ProvisioningService) Validate(ctx context.Context, key types.NamespacedName) error { + secret := &corev1.Secret{} + err := p.reader.Get(ctx, key, secret) + if err != nil { + if k8serrors.IsNotFound(err) { + return SecretNotFoundError(key.String()) + } + return err + } + switch secret.Type { + case v1alpha2.SecretTypeCloudInit: + return p.validateCloudInitSecret(secret) + case v1alpha2.SecretTypeSysprep: + return p.validateSysprepSecret(secret) + default: + return UnexpectedSecretTypeError(secret.Type) + } +} + +func (p *ProvisioningService) validateCloudInitSecret(secret *corev1.Secret) error { + if !p.hasOneOfKeys(secret, cloudInitCheckKeys...) { + return fmt.Errorf("the secret should have one of data fields %v: %w", cloudInitCheckKeys, ErrSecretIsNotValid) + } + return nil +} + +func (p *ProvisioningService) validateSysprepSecret(_ *corev1.Secret) error { + return nil +} + +func (p *ProvisioningService) hasOneOfKeys(secret *corev1.Secret, checkKeys ...string) bool { + validate := len(checkKeys) == 0 + for _, key := range checkKeys { + if _, ok := secret.Data[key]; ok { + validate = true + break + } + } + return validate +} + +func (p *ProvisioningService) ValidateUserDataLen(userData string) error { + if userData == "" { + return errors.New("provisioning userdata is defined, but it is empty") + } + + if len(userData) > cloudInitUserMaxLen { + return fmt.Errorf("userdata exceeds %d byte limit; should use userDataRef for larger data", cloudInitUserMaxLen) + } + + return nil +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/provisioning.go b/images/virtualization-artifact/pkg/controller/vm/internal/provisioning.go index b3511b33a2..780adcfeda 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/provisioning.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/provisioning.go @@ -21,8 +21,6 @@ import ( "errors" "fmt" - corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -38,12 +36,13 @@ import ( const nameProvisioningHandler = "ProvisioningHandler" func NewProvisioningHandler(client client.Client) *ProvisioningHandler { - return &ProvisioningHandler{client: client, validator: newProvisioningValidator(client)} + return &ProvisioningHandler{ + provisioningService: service.NewProvisioningService(client), + } } type ProvisioningHandler struct { - client client.Client - validator *provisioningValidator + provisioningService *service.ProvisioningService } func (h *ProvisioningHandler) Handle(ctx context.Context, s state.VirtualMachineState) (reconcile.Result, error) { @@ -73,13 +72,15 @@ func (h *ProvisioningHandler) Handle(ctx context.Context, s state.VirtualMachine p := current.Spec.Provisioning switch p.Type { case v1alpha2.ProvisioningTypeUserData: - if p.UserData != "" { - cb.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonProvisioningReady) - } else { + err := h.provisioningService.ValidateUserDataLen(p.UserData) + if err != nil { + errMsg := fmt.Errorf("failed to validate userdata length: %w", err) cb.Status(metav1.ConditionFalse). Reason(vmcondition.ReasonProvisioningNotReady). - Message("Provisioning is defined but it is empty.") + Message(service.CapitalizeFirstLetter(errMsg.Error() + ".")) + return reconcile.Result{}, errMsg } + cb.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonProvisioningReady) case v1alpha2.ProvisioningTypeUserDataRef: if p.UserDataRef == nil || p.UserDataRef.Kind != v1alpha2.UserDataRefKindSecret { cb.Status(metav1.ConditionFalse). @@ -119,25 +120,25 @@ func (h *ProvisioningHandler) Name() string { } func (h *ProvisioningHandler) genConditionFromSecret(ctx context.Context, builder *conditions.ConditionBuilder, secretKey types.NamespacedName) error { - err := h.validator.Validate(ctx, secretKey) + err := h.provisioningService.Validate(ctx, secretKey) switch { case err == nil: builder.Reason(vmcondition.ReasonProvisioningReady).Status(metav1.ConditionTrue) return nil - case errors.As(err, new(secretNotFoundError)): + case errors.As(err, new(service.SecretNotFoundError)): builder.Status(metav1.ConditionFalse). Reason(vmcondition.ReasonProvisioningNotReady). Message(service.CapitalizeFirstLetter(err.Error())) return nil - case errors.Is(err, errSecretIsNotValid): + case errors.Is(err, service.ErrSecretIsNotValid): builder.Status(metav1.ConditionFalse). Reason(vmcondition.ReasonProvisioningNotReady). Message(fmt.Sprintf("Invalid secret %q: %s", secretKey.String(), err.Error())) return nil - case errors.As(err, new(unexpectedSecretTypeError)): + case errors.As(err, new(service.UnexpectedSecretTypeError)): builder.Status(metav1.ConditionFalse). Reason(vmcondition.ReasonProvisioningNotReady). Message(service.CapitalizeFirstLetter(err.Error())) @@ -147,73 +148,3 @@ func (h *ProvisioningHandler) genConditionFromSecret(ctx context.Context, builde return err } } - -var errSecretIsNotValid = errors.New("secret is not valid") - -type secretNotFoundError string - -func (e secretNotFoundError) Error() string { - return fmt.Sprintf("secret %s not found", string(e)) -} - -type unexpectedSecretTypeError string - -func (e unexpectedSecretTypeError) Error() string { - return fmt.Sprintf("unexpected secret type: %s", string(e)) -} - -var cloudInitCheckKeys = []string{ - "userdata", - "userData", -} - -func newProvisioningValidator(reader client.Reader) *provisioningValidator { - return &provisioningValidator{ - reader: reader, - } -} - -type provisioningValidator struct { - reader client.Reader -} - -func (v provisioningValidator) Validate(ctx context.Context, key types.NamespacedName) error { - secret := &corev1.Secret{} - err := v.reader.Get(ctx, key, secret) - if err != nil { - if k8serrors.IsNotFound(err) { - return secretNotFoundError(key.String()) - } - return err - } - switch secret.Type { - case v1alpha2.SecretTypeCloudInit: - return v.validateCloudInitSecret(secret) - case v1alpha2.SecretTypeSysprep: - return v.validateSysprepSecret(secret) - default: - return unexpectedSecretTypeError(secret.Type) - } -} - -func (v provisioningValidator) validateCloudInitSecret(secret *corev1.Secret) error { - if !v.hasOneOfKeys(secret, cloudInitCheckKeys...) { - return fmt.Errorf("the secret should have one of data fields %v: %w", cloudInitCheckKeys, errSecretIsNotValid) - } - return nil -} - -func (v provisioningValidator) validateSysprepSecret(_ *corev1.Secret) error { - return nil -} - -func (v provisioningValidator) hasOneOfKeys(secret *corev1.Secret, checkKeys ...string) bool { - validate := len(checkKeys) == 0 - for _, key := range checkKeys { - if _, ok := secret.Data[key]; ok { - validate = true - break - } - } - return validate -} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/provisioning_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/provisioning_validator.go new file mode 100644 index 0000000000..80a3709cea --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/provisioning_validator.go @@ -0,0 +1,62 @@ +/* +Copyright 2026 Flant JSC + +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 validators + +import ( + "context" + + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +type ProvisioningValidator struct { + provisioningService *service.ProvisioningService +} + +func NewProvisioningValidator(provisioningService *service.ProvisioningService) *ProvisioningValidator { + return &ProvisioningValidator{provisioningService: provisioningService} +} + +func (p *ProvisioningValidator) ValidateCreate(_ context.Context, vm *v1alpha2.VirtualMachine) (admission.Warnings, error) { + err := p.validateUserDataLen(vm) + if err != nil { + return admission.Warnings{}, err + } + + return nil, nil +} + +func (p *ProvisioningValidator) ValidateUpdate(_ context.Context, _, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) { + err := p.validateUserDataLen(newVM) + if err != nil { + return admission.Warnings{}, err + } + + return nil, nil +} + +func (p *ProvisioningValidator) validateUserDataLen(vm *v1alpha2.VirtualMachine) error { + if vm.Spec.Provisioning != nil && vm.Spec.Provisioning.Type == v1alpha2.ProvisioningTypeUserData { + err := p.provisioningService.ValidateUserDataLen(vm.Spec.Provisioning.UserData) + if err != nil { + return err + } + } + return nil +} diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go index 1cd2ad4433..13cc4190f1 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go @@ -54,6 +54,7 @@ func SetupController( mgrCache := mgr.GetCache() client := mgr.GetClient() blockDeviceService := service.NewBlockDeviceService(client) + provisioningService := service.NewProvisioningService(client) vmClassService := service.NewVirtualMachineClassService(client) migrateVolumesService := vmservice.NewMigrationVolumesService(client, internal.MakeKVVMFromVMSpec, 10*time.Second) @@ -100,7 +101,7 @@ func SetupController( if err = builder.WebhookManagedBy(mgr). For(&v1alpha2.VirtualMachine{}). - WithValidator(NewValidator(client, blockDeviceService, featuregates.Default(), log)). + WithValidator(NewValidator(client, blockDeviceService, provisioningService, featuregates.Default(), log)). WithDefaulter(NewDefaulter(client, vmClassService, log)). Complete(); err != nil { return err diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go b/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go index 34b6ac0db4..5f40ea2a10 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go @@ -42,19 +42,20 @@ type Validator struct { log *log.Logger } -func NewValidator(client client.Client, service *service.BlockDeviceService, featureGate featuregate.FeatureGate, log *log.Logger) *Validator { +func NewValidator(client client.Client, blockDeviceservice *service.BlockDeviceService, provisioningService *service.ProvisioningService, featureGate featuregate.FeatureGate, log *log.Logger) *Validator { return &Validator{ validators: []VirtualMachineValidator{ validators.NewMetaValidator(client), validators.NewIPAMValidator(client), validators.NewBlockDeviceSpecRefsValidator(), validators.NewSizingPolicyValidator(client), - validators.NewBlockDeviceLimiterValidator(service, log), + validators.NewBlockDeviceLimiterValidator(blockDeviceservice, log), validators.NewAffinityValidator(), validators.NewTopologySpreadConstraintValidator(), validators.NewCPUCountValidator(), validators.NewNetworksValidator(featureGate), validators.NewFirstDiskValidator(client), + validators.NewProvisioningValidator(provisioningService), }, log: log.With("webhook", "validation"), } From c907d8068613321b98b80f15d4bfd1dcf1bd9d94 Mon Sep 17 00:00:00 2001 From: Roman Sysoev Date: Fri, 30 Jan 2026 17:26:20 +0300 Subject: [PATCH 2/2] test(vm): add userdata validation unit test Signed-off-by: Roman Sysoev --- .../service/provisioning_service.go | 9 ++- .../service/provisioning_service_test.go | 57 +++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 images/virtualization-artifact/pkg/controller/service/provisioning_service_test.go diff --git a/images/virtualization-artifact/pkg/controller/service/provisioning_service.go b/images/virtualization-artifact/pkg/controller/service/provisioning_service.go index ad8d723544..c81f2bbe74 100644 --- a/images/virtualization-artifact/pkg/controller/service/provisioning_service.go +++ b/images/virtualization-artifact/pkg/controller/service/provisioning_service.go @@ -31,6 +31,11 @@ import ( const cloudInitUserMaxLen = 2048 +var ( + ErrUserDataEmpty = errors.New("provisioning userdata is defined, but it is empty") + ErrUserDataTooLong = fmt.Errorf("userdata exceeds %d byte limit; should use userDataRef for larger data", cloudInitUserMaxLen) +) + type ProvisioningService struct { reader client.Reader } @@ -103,11 +108,11 @@ func (p *ProvisioningService) hasOneOfKeys(secret *corev1.Secret, checkKeys ...s func (p *ProvisioningService) ValidateUserDataLen(userData string) error { if userData == "" { - return errors.New("provisioning userdata is defined, but it is empty") + return ErrUserDataEmpty } if len(userData) > cloudInitUserMaxLen { - return fmt.Errorf("userdata exceeds %d byte limit; should use userDataRef for larger data", cloudInitUserMaxLen) + return ErrUserDataTooLong } return nil diff --git a/images/virtualization-artifact/pkg/controller/service/provisioning_service_test.go b/images/virtualization-artifact/pkg/controller/service/provisioning_service_test.go new file mode 100644 index 0000000000..4f16438f26 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/service/provisioning_service_test.go @@ -0,0 +1,57 @@ +/* +Copyright 2026 Flant JSC + +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 service + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ProvisioningService", func() { + var provisioningService *ProvisioningService + + BeforeEach(func() { + provisioningService = NewProvisioningService(nil) + }) + + DescribeTable("ValidateUserDataLen", + func(userData string, expectedErr error) { + err := provisioningService.ValidateUserDataLen(userData) + if expectedErr != nil { + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(expectedErr)) + } else { + Expect(err).ToNot(HaveOccurred()) + } + }, + Entry( + "empty userdata", + "", + ErrUserDataEmpty, + ), + Entry( + "userdata exceeds max limit", + string(make([]byte, cloudInitUserMaxLen+1)), + ErrUserDataTooLong, + ), + Entry( + "userdata is within limit", + string(make([]byte, cloudInitUserMaxLen-1)), + nil, + ), + ) +})