Skip to content

Commit

Permalink
Merge pull request kubevirt#9680 from germag/remove-virtiofs-featuregate
Browse files Browse the repository at this point in the history
virtiofs: Remove feature gate for config volumes
  • Loading branch information
kubevirt-bot authored May 8, 2023
2 parents e701677 + 95e2569 commit 2a22efe
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ go_library(
"//pkg/virt-api/webhooks:go_default_library",
"//pkg/virt-config:go_default_library",
"//pkg/virt-handler/cmd-client:go_default_library",
"//pkg/virtiofs:go_default_library",
"//staging/src/kubevirt.io/api/clone:go_default_library",
"//staging/src/kubevirt.io/api/clone/v1alpha1:go_default_library",
"//staging/src/kubevirt.io/api/core:go_default_library",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
webhookutils "kubevirt.io/kubevirt/pkg/util/webhooks"
"kubevirt.io/kubevirt/pkg/virt-api/webhooks"
virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
"kubevirt.io/kubevirt/pkg/virtiofs"
)

const requiredFieldFmt = "%s is a required field"
Expand Down Expand Up @@ -806,8 +807,35 @@ func validateGPUsWithPassthroughEnabled(field *k8sfield.Path, spec *v1.VirtualMa
return causes
}

// Returns true if any of the filesystems requires a virtiofs container to run as root
func requireAnyVirtiofsPrivilegedContainer(spec *v1.VirtualMachineInstanceSpec) bool {
if spec.Domain.Devices.Filesystems == nil {
return false
}

volumes := make(map[string]v1.Volume)
for _, volume := range spec.Volumes {
volumes[volume.Name] = volume
}

// we assume that all filesystems are of type virtiofs
for _, fs := range spec.Domain.Devices.Filesystems {
volume, ok := volumes[fs.Name]
if !ok {
continue
}

if virtiofs.RequiresRootPrivileges(&volume) {
return true
}
}

return false
}

func validateFilesystemsWithVirtIOFSEnabled(field *k8sfield.Path, spec *v1.VirtualMachineInstanceSpec, config *virtconfig.ClusterConfig) (causes []metav1.StatusCause) {
if spec.Domain.Devices.Filesystems != nil && !config.VirtiofsEnabled() {
// The feature gate is mandatory only for a privileged container
if requireAnyVirtiofsPrivilegedContainer(spec) && !config.VirtiofsEnabled() {
causes = append(causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Message: "virtiofs feature gate is not enabled in kubevirt-config",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2072,7 +2072,7 @@ var _ = Describe("Validating VMICreate Admitter", func() {
Expect(causes).To(HaveLen(1))
Expect(causes[0].Field).To(Equal("fake.GPUs"))
})
It("should reject virtiofs filesystems when feature gate is disabled", func() {
It("should reject privileged virtiofs filesystems when feature gate is disabled", func() {
vmi := api.NewMinimalVMI("testvm")
guestMemory := resource.MustParse("64Mi")

Expand All @@ -2086,16 +2086,22 @@ var _ = Describe("Validating VMICreate Admitter", func() {
vmi.Spec.Domain.Memory.Hugepages.PageSize = "2Mi"
vmi.Spec.Domain.Devices.Filesystems = []v1.Filesystem{
{
Name: "sharednfstest",
Name: "sharedtestdisk",
Virtiofs: &v1.FilesystemVirtiofs{},
},
}
vmi.Spec.Volumes = append(vmi.Spec.Volumes, v1.Volume{
Name: "sharedtestdisk",
VolumeSource: v1.VolumeSource{
PersistentVolumeClaim: testutils.NewFakePersistentVolumeSource(),
},
})

causes := ValidateVirtualMachineInstanceSpec(k8sfield.NewPath("fake"), &vmi.Spec, config)
Expect(causes).To(HaveLen(1))
Expect(causes[0].Field).To(Equal("fake.Filesystems"))
})
It("should allow virtiofs filesystems when feature gate is enabled", func() {
It("should allow privileged virtiofs filesystems when feature gate is enabled", func() {
enableFeatureGate(virtconfig.VirtIOFSGate)
vmi := api.NewMinimalVMI("testvm")
guestMemory := resource.MustParse("64Mi")
Expand All @@ -2111,10 +2117,16 @@ var _ = Describe("Validating VMICreate Admitter", func() {
vmi.Spec.Domain.Memory.Hugepages.PageSize = "2Mi"
vmi.Spec.Domain.Devices.Filesystems = []v1.Filesystem{
{
Name: "sharednfstest",
Name: "sharedtestdisk",
Virtiofs: &v1.FilesystemVirtiofs{},
},
}
vmi.Spec.Volumes = append(vmi.Spec.Volumes, v1.Volume{
Name: "sharedtestdisk",
VolumeSource: v1.VolumeSource{
PersistentVolumeClaim: testutils.NewFakePersistentVolumeSource(),
},
})

causes := ValidateVirtualMachineInstanceSpec(k8sfield.NewPath("fake"), &vmi.Spec, config)
Expect(causes).To(BeEmpty())
Expand Down
7 changes: 1 addition & 6 deletions pkg/virt-controller/services/virtiofs.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,6 @@ func securityContextVirtioFS(profile securityProfile) *k8sv1.SecurityContext {
}
}

func isConfig(volume *v1.Volume) bool {
return volume.ConfigMap != nil || volume.Secret != nil ||
volume.ServiceAccount != nil || volume.DownwardAPI != nil
}

func isAutoMount(volume *v1.Volume) bool {
// The template service sets pod.Spec.AutomountServiceAccountToken as true
return volume.ServiceAccount != nil
Expand Down Expand Up @@ -167,7 +162,7 @@ func generateContainerFromVolume(volume *v1.Volume, image string, config *virtco

securityProfile := restricted
sandbox := "none"
if !isConfig(volume) {
if virtiofs.RequiresRootPrivileges(volume) {
securityProfile = privileged
sandbox = "chroot"
args = append(args, "--xattr")
Expand Down
5 changes: 4 additions & 1 deletion pkg/virtiofs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,8 @@ go_library(
srcs = ["virtiofs.go"],
importpath = "kubevirt.io/kubevirt/pkg/virtiofs",
visibility = ["//visibility:public"],
deps = ["//pkg/util:go_default_library"],
deps = [
"//pkg/util:go_default_library",
"//staging/src/kubevirt.io/api/core/v1:go_default_library",
],
)
10 changes: 10 additions & 0 deletions pkg/virtiofs/virtiofs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"path/filepath"

v1 "kubevirt.io/api/core/v1"

"kubevirt.io/kubevirt/pkg/util"
)

Expand All @@ -15,3 +17,11 @@ func VirtioFSSocketPath(volumeName string) string {
socketName := fmt.Sprintf("%s.sock", volumeName)
return filepath.Join(VirtioFSContainersMountBaseDir, socketName)
}

// RequiresRootPrivileges Returns true if the volume requires the virtiofs
// container to run as user root
func RequiresRootPrivileges(volume *v1.Volume) bool {
// only config volumes can be shared with an unprivileged container
return volume.ConfigMap == nil && volume.Secret == nil &&
volume.ServiceAccount == nil && volume.DownwardAPI == nil
}
1 change: 1 addition & 0 deletions tests/virtiofs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go_library(
srcs = [
"config.go",
"datavolume.go",
"feature_gate.go",
],
importpath = "kubevirt.io/kubevirt/tests/virtiofs",
visibility = ["//visibility:public"],
Expand Down
11 changes: 0 additions & 11 deletions tests/virtiofs/datavolume.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,6 @@ var _ = Describe("[sig-storage] virtiofs", decorators.SigStorage, func() {
vmi = nil
})

Context("[Serial]With feature gates disabled for", Serial, func() {
It("VirtioFS, it should fail to start a VMI", func() {
tests.DisableFeatureGate(virtconfig.VirtIOFSGate)
vmi := libvmi.NewFedora(libvmi.WithFilesystemDV("something"))
virtClient := kubevirt.Client()
_, err = virtClient.VirtualMachineInstance(testsuite.GetTestNamespace(vmi)).Create(context.Background(), vmi)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("virtiofs feature gate is not enabled"))
})
})

Context("VirtIO-FS with multiple PVCs", func() {
pvc1 := "pvc-1"
pvc2 := "pvc-2"
Expand Down
110 changes: 110 additions & 0 deletions tests/virtiofs/feature_gate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* This file is part of the KubeVirt project
*
* 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.
*
* Copyright 2023 Red Hat, Inc.
*
*/

package virtiofs

import (
"context"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/pborman/uuid"

"kubevirt.io/client-go/kubecli"

virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
"kubevirt.io/kubevirt/tests"
"kubevirt.io/kubevirt/tests/decorators"
"kubevirt.io/kubevirt/tests/framework/checks"
"kubevirt.io/kubevirt/tests/framework/kubevirt"
"kubevirt.io/kubevirt/tests/libvmi"
"kubevirt.io/kubevirt/tests/testsuite"
)

var _ = Describe("[sig-storage] VirtIO-FS feature gate", Serial, decorators.SigStorage, func() {
var virtClient kubecli.KubevirtClient
var featureGateWasEnabled bool

BeforeEach(func() {
virtClient = kubevirt.Client()
featureGateWasEnabled = checks.HasFeature(virtconfig.VirtIOFSGate)
tests.DisableFeatureGate(virtconfig.VirtIOFSGate)
})

AfterEach(func() {
if featureGateWasEnabled {
tests.EnableFeatureGate(virtconfig.VirtIOFSGate)
}
})

Context("[Serial]With feature gates disabled for", func() {
It("DataVolume, it should fail to start a VMI", func() {
vmi := libvmi.NewFedora(libvmi.WithFilesystemDV("something"))
_, err := virtClient.VirtualMachineInstance(testsuite.GetTestNamespace(vmi)).Create(context.Background(), vmi)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("virtiofs feature gate is not enabled"))
})

It("config volumes, it should success to start a VMI", func() {
configMapName := "configmap-" + uuid.NewRandom().String()[:6]
configMapData := map[string]string{
"option1": "value1",
"option2": "value2",
"option3": "value3",
}
tests.CreateConfigMap(configMapName, testsuite.GetTestNamespace(nil), configMapData)

secretName := "secret-" + uuid.NewRandom().String()[:6]
secretData := map[string]string{
"user": "admin",
"password": "redhat",
}
tests.CreateSecret(secretName, testsuite.GetTestNamespace(nil), secretData)

serviceAccountVolumeName := "default-disk"

downwardAPIName := "downwardapi-" + uuid.NewRandom().String()[:6]
testLabelKey := "kubevirt.io.testdownwardapi"
testLabelVal := "downwardAPIValue"

By("Creating VMI")
vmi := libvmi.NewFedora(
libvmi.WithConfigMapFs(configMapName, configMapName),
libvmi.WithSecretFs(secretName, secretName),
libvmi.WithServiceAccountFs("default", serviceAccountVolumeName),
libvmi.WithLabel(testLabelKey, testLabelVal),
libvmi.WithDownwardAPIFs(downwardAPIName),
)

_, err := virtClient.VirtualMachineInstance(testsuite.GetTestNamespace(vmi)).Create(context.Background(), vmi)
Expect(err).NotTo(HaveOccurred())
})

It("DataVolume and ServiceAccount, it should fail to start a VMI", func() {
serviceAccountVolumeName := "default-disk"
vmi := libvmi.NewFedora(
libvmi.WithFilesystemDV("something"),
libvmi.WithServiceAccountFs("default", serviceAccountVolumeName),
)
_, err := virtClient.VirtualMachineInstance(testsuite.GetTestNamespace(vmi)).Create(context.Background(), vmi)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("virtiofs feature gate is not enabled"))
})
})
})

0 comments on commit 2a22efe

Please sign in to comment.