Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions crds/virtualmachines.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
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

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
}

func NewProvisioningService(reader client.Reader) *ProvisioningService {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this here and not inside the vm directory?

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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overengineering. The error looks cool, but you’re not using its advantages. You’re just putting it into a condition afterward. Just return a simple fmt.Errorf

Keep It Simple

}
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the specified provisioning secret %q must have at least one of the following fields: %v: %w"

}
return nil
}

func (p *ProvisioningService) validateSysprepSecret(_ *corev1.Secret) error {
Copy link
Contributor

@Isteb4k Isteb4k Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overengineering. Code that does nothing. Remove everything unnecessary

Keep It Simple

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
Copy link
Contributor

@Isteb4k Isteb4k Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable is named incorrectly, and you can actually get rid of it

Keep It Simple

break
}
}
return validate
}

func (p *ProvisioningService) ValidateUserDataLen(userData string) error {
if userData == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trim it before validation

return ErrUserDataEmpty
}

if len(userData) > cloudInitUserMaxLen {
return ErrUserDataTooLong
}

return nil
}
Original file line number Diff line number Diff line change
@@ -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,
),
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err = fmt.Errorf("failed to validate userdata length: %w", err)

No need to create one more variable

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).
Expand Down Expand Up @@ -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()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should already be clear from err.Error() that the secret failed validation. No need to duplicate this "Invalid secret"

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()))
Expand All @@ -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
}
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you can use CEL rules instead

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
}
Loading
Loading