From 66cec1486665c30028a6c6f171bf6e56cbbd1289 Mon Sep 17 00:00:00 2001 From: Sankalp Rangare Date: Tue, 13 Dec 2022 16:49:29 +0100 Subject: [PATCH] Add validation for image-cloning and custom-images for kubevirt Signed-off-by: Sankalp Rangare --- .../provider/kubevirt/provider.go | 115 +++++++++++++++- .../provider/kubevirt/provider_test.go | 123 ++++++++++++++++++ 2 files changed, 231 insertions(+), 7 deletions(-) diff --git a/pkg/cloudprovider/provider/kubevirt/provider.go b/pkg/cloudprovider/provider/kubevirt/provider.go index 9850f3603b..92415b5212 100644 --- a/pkg/cloudprovider/provider/kubevirt/provider.go +++ b/pkg/cloudprovider/provider/kubevirt/provider.go @@ -58,6 +58,9 @@ func init() { if err := kubevirtv1.AddToScheme(scheme.Scheme); err != nil { klog.Fatalf("failed to add kubevirtv1 to scheme: %v", err) } + if err := cdiv1beta1.AddToScheme(scheme.Scheme); err != nil { + klog.Fatalf("failed to add cdiv1beta1 to scheme: %v", err) + } } type imageSource string @@ -72,15 +75,24 @@ const ( httpSource imageSource = "http" // pvcSource defines the pvc source type for VM Disk Image. pvcSource imageSource = "pvc" + // kubeVirtImagesNamespace namespace contains globally available custom images and cached standard images. + kubeVirtImagesNamespace = "kubevirt-images" + dataVolumeStandardImageAnnotation = "kubevirt-initialization.k8c.io/standard-image" + osAnnotationForCustomDisk = "cdi.kubevirt.io/os-type" ) -var supportedOS = map[providerconfigtypes.OperatingSystem]*struct{}{ - providerconfigtypes.OperatingSystemCentOS: nil, - providerconfigtypes.OperatingSystemUbuntu: nil, - providerconfigtypes.OperatingSystemRHEL: nil, - providerconfigtypes.OperatingSystemFlatcar: nil, - providerconfigtypes.OperatingSystemRockyLinux: nil, -} +var ( + supportedOS = map[providerconfigtypes.OperatingSystem]*struct{}{ + providerconfigtypes.OperatingSystemCentOS: nil, + providerconfigtypes.OperatingSystemUbuntu: nil, + providerconfigtypes.OperatingSystemRHEL: nil, + providerconfigtypes.OperatingSystemFlatcar: nil, + providerconfigtypes.OperatingSystemRockyLinux: nil, + } + errInvalidOsImage = fmt.Errorf("invalid primaryDisk.osImage") + errCustomImage = fmt.Errorf("custom-image cloning not allowed") + errStandardImage = fmt.Errorf("standard-image cloning not allowed") +) type provider struct { configVarResolver *providerconfig.ConfigVarResolver @@ -108,6 +120,8 @@ type Config struct { SecondaryDisks []SecondaryDisks NodeAffinityPreset NodeAffinityPreset TopologySpreadConstraints []corev1.TopologySpreadConstraint + AllowPVCClone bool + AllowCustomImages bool } type AffinityType string @@ -317,6 +331,16 @@ func (p *provider) getConfig(provSpec clusterv1alpha1.ProviderSpec) (*Config, *p return nil, nil, fmt.Errorf(`failed to parse "topologySpreadConstraints" field: %w`, err) } + config.AllowPVCClone, err = isImageCloningAllowed() + if err != nil { + return nil, nil, fmt.Errorf(`failed to parse "KUBEVIRT_ALLOW_PVC_CLONE" environment variable: %w`, err) + } + + config.AllowCustomImages, err = isCustomImageAllowed() + if err != nil { + return nil, nil, fmt.Errorf(`failed to parse "KUBEVIRT_ALLOW_CUSTOM_IMAGES" environment variable: %w`, err) + } + return &config, pconfig, nil } @@ -412,6 +436,34 @@ func getNamespace() string { return ns } +// isImageCloningAllowed returns whether image-cloning is allowed or not. +// Default value is `true`. +func isImageCloningAllowed() (bool, error) { + value := os.Getenv("KUBEVIRT_ALLOW_PVC_CLONE") + if value == "" { + return true, nil + } + isImageCloningEnabled, err := strconv.ParseBool(value) + if err != nil { + return false, err + } + return isImageCloningEnabled, nil +} + +// isCustomImageAllowed returns whether custom-image for cloning is allowed or not. +// Default value is `true`. +func isCustomImageAllowed() (bool, error) { + value := os.Getenv("KUBEVIRT_ALLOW_CUSTOM_IMAGES") + if value == "" { + return true, nil + } + isCustomImagesEnabled, err := strconv.ParseBool(value) + if err != nil { + return false, err + } + return isCustomImagesEnabled, nil +} + func (p *provider) Get(ctx context.Context, machine *clusterv1alpha1.Machine, _ *cloudprovidertypes.ProviderData) (instance.Instance, error) { c, _, err := p.getConfig(machine.Spec.ProviderSpec) if err != nil { @@ -503,6 +555,9 @@ func (p *provider) Validate(ctx context.Context, spec clusterv1alpha1.MachineSpe return fmt.Errorf("failed to request VirtualMachineInstances: %w", err) } + if c.OSImageSource.PVC != nil { + return validateOsImage(ctx, c, sigClient) + } return nil } @@ -920,3 +975,49 @@ func getTopologySpreadConstraints(config *Config, matchLabels map[string]string) }, } } + +// validateOsImage with PVC as source. +func validateOsImage(ctx context.Context, c *Config, sigClient client.Client) error { + switch c.OSImageSource.PVC.Namespace { + case c.Namespace: + if !c.AllowCustomImages { + return errCustomImage + } + + case kubeVirtImagesNamespace: + existingDiskList := cdiv1beta1.DataVolumeList{} + listOption := client.ListOptions{ + Namespace: kubeVirtImagesNamespace, + } + if err := sigClient.List(ctx, &existingDiskList, &listOption); client.IgnoreNotFound(err) != nil { + return fmt.Errorf("failed to request DataVolumeList: %w", err) + } + return validateKubeVirtImages(c.OSImageSource.PVC.Name, existingDiskList, c) + + default: + return errInvalidOsImage + } + return nil +} + +// validateKubeVirtImages from kubeVirtImagesNamespace. +func validateKubeVirtImages(sourcePVC string, existingDiskList cdiv1beta1.DataVolumeList, config *Config) error { + for _, existingDV := range existingDiskList.Items { + if sourcePVC == existingDV.Name { + if existingDV.Annotations[dataVolumeStandardImageAnnotation] == "true" { + if !config.AllowPVCClone { + return errStandardImage + } + return nil + } + if existingDV.Annotations[osAnnotationForCustomDisk] != "" { + if !config.AllowCustomImages { + return errCustomImage + } + return nil + } + break + } + } + return errInvalidOsImage +} diff --git a/pkg/cloudprovider/provider/kubevirt/provider_test.go b/pkg/cloudprovider/provider/kubevirt/provider_test.go index 261ac347e7..19cb95126c 100644 --- a/pkg/cloudprovider/provider/kubevirt/provider_test.go +++ b/pkg/cloudprovider/provider/kubevirt/provider_test.go @@ -20,12 +20,14 @@ import ( "bytes" "context" "embed" + "errors" "html/template" "path" "reflect" "testing" kubevirtv1 "kubevirt.io/api/core/v1" + cdiv1beta1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" cloudprovidertesting "github.com/kubermatic/machine-controller/pkg/cloudprovider/testing" "github.com/kubermatic/machine-controller/pkg/providerconfig" @@ -36,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/client-go/kubernetes/scheme" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" fakectrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -320,3 +323,123 @@ func TestTopologySpreadConstraint(t *testing.T) { }) } } + +func TestValidateOsImage(t *testing.T) { + testClient := fakectrlruntimeclient. + NewClientBuilder(). + WithScheme(scheme.Scheme). + WithObjects(&cdiv1beta1.DataVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "standardDV", + Namespace: kubeVirtImagesNamespace, + Annotations: map[string]string{dataVolumeStandardImageAnnotation: "true"}}, + }, + &cdiv1beta1.DataVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "customDVByAdmin", + Namespace: kubeVirtImagesNamespace, + Annotations: map[string]string{osAnnotationForCustomDisk: "ubuntu"}}, + }, + ).Build() + + tests := []struct { + desc string + config Config + expectedErr error + }{ + { + desc: "valid osImage with cloned standard DataVolume as pvc source, cloning enabled", + config: Config{ + OSImageSource: &cdiv1beta1.DataVolumeSource{ + PVC: &cdiv1beta1.DataVolumeSourcePVC{ + Name: "standardDV", + Namespace: kubeVirtImagesNamespace, + }, + }, + AllowPVCClone: true, + }, + expectedErr: nil, + }, + { + desc: "invalid osImage with cloned standard DataVolume as pvc source, cloning disabled", + config: Config{ + OSImageSource: &cdiv1beta1.DataVolumeSource{ + PVC: &cdiv1beta1.DataVolumeSourcePVC{ + Name: "standardDV", + Namespace: kubeVirtImagesNamespace, + }, + }, + AllowPVCClone: false, + }, + expectedErr: errStandardImage, + }, + { + desc: "valid osImage with custom-image-by-admin as pvc source, custom-images enabled", + config: Config{ + OSImageSource: &cdiv1beta1.DataVolumeSource{ + PVC: &cdiv1beta1.DataVolumeSourcePVC{ + Name: "customDVByAdmin", + Namespace: kubeVirtImagesNamespace, + }, + }, + AllowCustomImages: true, + }, + expectedErr: nil, + }, + { + desc: "invalid osImage with custom-image-by-admin as pvc source, custom-images disabled", + config: Config{ + OSImageSource: &cdiv1beta1.DataVolumeSource{ + PVC: &cdiv1beta1.DataVolumeSourcePVC{ + Name: "customDVByAdmin", + Namespace: kubeVirtImagesNamespace, + }, + }, + AllowCustomImages: false, + }, + expectedErr: errCustomImage, + }, + { + desc: "invalid osImage with custom-image-by-user as pvc source, custom-images disabled", + config: Config{ + Namespace: "cluster-test", + OSImageSource: &cdiv1beta1.DataVolumeSource{ + PVC: &cdiv1beta1.DataVolumeSourcePVC{ + Name: "customDVByUser", + Namespace: "cluster-test", + }, + }, + AllowCustomImages: false, + }, + expectedErr: errCustomImage, + }, + { + desc: "invalid osImage with non-existent pvc source, cloning enabled", + config: Config{ + OSImageSource: &cdiv1beta1.DataVolumeSource{ + PVC: &cdiv1beta1.DataVolumeSourcePVC{ + Name: "non-existent-DV", + Namespace: kubeVirtImagesNamespace, + }, + }, + AllowPVCClone: true, + }, + expectedErr: errInvalidOsImage, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + actualErr := validateOsImage(context.Background(), &test.config, testClient) + if test.expectedErr != nil { + if !errors.Is(actualErr, test.expectedErr) { + t.Errorf("expected error: %q, got: %q", test.expectedErr, actualErr) + } + } else { + if actualErr != nil { + t.Errorf("expected success, but got error: %q", actualErr) + } + } + }) + } +}