-
Notifications
You must be signed in to change notification settings - Fork 5
feat(vm): validate cloud-init userdata #1937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 { | ||
| 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()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 == "" { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
|---|---|---|
|
|
@@ -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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). | ||
|
|
@@ -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())) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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())) | ||
|
|
@@ -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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| } | ||
There was a problem hiding this comment.
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?