Skip to content

Commit

Permalink
Merge pull request kubevirt#9137 from xpivarc/remove-psa
Browse files Browse the repository at this point in the history
Remove PSASeccompAllowsUserfaultfd
  • Loading branch information
kubevirt-bot authored Feb 2, 2023
2 parents f0cf719 + 73918f5 commit 0135cca
Show file tree
Hide file tree
Showing 11 changed files with 4 additions and 229 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ go_library(
"//pkg/hooks:go_default_library",
"//pkg/instancetype:go_default_library",
"//pkg/network/link:go_default_library",
"//pkg/psa:go_default_library",
"//pkg/storage/snapshot:go_default_library",
"//pkg/storage/types:go_default_library",
"//pkg/util/hardware:go_default_library",
Expand Down Expand Up @@ -95,7 +94,6 @@ go_test(
deps = [
"//pkg/hooks:go_default_library",
"//pkg/instancetype:go_default_library",
"//pkg/psa:go_default_library",
"//pkg/testutils:go_default_library",
"//pkg/util/webhooks:go_default_library",
"//pkg/virt-api/webhooks:go_default_library",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package admitters

import (
"context"
"encoding/json"
"fmt"

Expand All @@ -37,7 +36,6 @@ import (

"kubevirt.io/client-go/kubecli"

"kubevirt.io/kubevirt/pkg/psa"
webhookutils "kubevirt.io/kubevirt/pkg/util/webhooks"
)

Expand Down Expand Up @@ -97,21 +95,6 @@ func (admitter *MigrationPolicyAdmitter) Admit(ar *admissionv1.AdmissionReview)
}
}

if spec.AllowPostCopy != nil && *spec.AllowPostCopy {
namespace, err := admitter.Client.CoreV1().Namespaces().Get(context.Background(), policy.Namespace, metav1.GetOptions{})
if err != nil {
return webhookutils.ToAdmissionResponseError(err)
}

if !admitter.ClusterConfig.PSASeccompAllowsUserfaultfd() && !psa.IsNamespacePrivileged(namespace) {
causes = append(causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Message: "PostCopy is not allowed if the namespace is unprivileged",
Field: sourceField.Child("allowPostCopy").String(),
})
}
}

if len(causes) > 0 {
return webhookutils.ToAdmissionResponse(causes)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,71 +25,34 @@ import (
v1 "kubevirt.io/api/core/v1"

"kubevirt.io/kubevirt/pkg/testutils"
virtconfig "kubevirt.io/kubevirt/pkg/virt-config"

"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/testing"
"k8s.io/utils/pointer"

"kubevirt.io/api/migrations"

"kubevirt.io/kubevirt/pkg/psa"

migrationsv1 "kubevirt.io/api/migrations/v1alpha1"

"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
admissionv1 "k8s.io/api/admission/v1"
k8sv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"

"kubevirt.io/client-go/kubecli"
)

var _ = Describe("Validating MigrationPolicy Admitter", func() {
config, _, kvInformer := testutils.NewFakeClusterConfigUsingKVConfig(&v1.KubeVirtConfiguration{})
config, _, _ := testutils.NewFakeClusterConfigUsingKVConfig(&v1.KubeVirtConfiguration{})

var ctrl *gomock.Controller
var virtClient *kubecli.MockKubevirtClient
var admitter *MigrationPolicyAdmitter
var policyName string
var kubeClient *fake.Clientset

enableFeatureGate := func(featureGate string) {
testutils.UpdateFakeKubeVirtClusterConfig(kvInformer, &v1.KubeVirt{
Spec: v1.KubeVirtSpec{
Configuration: v1.KubeVirtConfiguration{
DeveloperConfiguration: &v1.DeveloperConfiguration{
FeatureGates: []string{featureGate},
},
},
},
})
}
disableFeatureGates := func() {
testutils.UpdateFakeKubeVirtClusterConfig(kvInformer, &v1.KubeVirt{
Spec: v1.KubeVirtSpec{
Configuration: v1.KubeVirtConfiguration{
DeveloperConfiguration: &v1.DeveloperConfiguration{
FeatureGates: make([]string, 0),
},
},
},
})
}

const (
privilegedNamespace = true
nonPrivilegedNamespace = false
enableTheFeatureGate = true
disableTheFeatureGate = false
shouldSucceed = true
shouldFail = false
)

BeforeEach(func() {
ctrl = gomock.NewController(GinkgoT())
kubeClient = fake.NewSimpleClientset()
Expand Down Expand Up @@ -145,44 +108,6 @@ var _ = Describe("Validating MigrationPolicy Admitter", func() {
migrationsv1.MigrationPolicySpec{},
),
)

DescribeTable("migration policy with postcopy enabled should be", func(usePrivilegedNamespace, useFeatureGate, expectedOutcome bool) {
if useFeatureGate {
enableFeatureGate(virtconfig.PSASeccompAllowsUserfaultfd)
} else {
disableFeatureGates()
}
kubeClient.Fake.PrependReactor("get", "namespaces", func(action testing.Action) (handled bool, obj runtime.Object, err error) {
_, ok := action.(testing.GetAction)
Expect(ok).To(BeTrue())

labels := map[string]string{}
if usePrivilegedNamespace {
labels[psa.PSALabel] = "privileged"
}

return true, &k8sv1.Namespace{
TypeMeta: metav1.TypeMeta{Kind: "Namespace"},
ObjectMeta: metav1.ObjectMeta{
Name: "default",
Labels: labels,
},
}, nil
})

By("Setting up a new policy")
policy := kubecli.NewMinimalMigrationPolicy(policyName)
policy.Spec = migrationsv1.MigrationPolicySpec{AllowPostCopy: pointer.BoolPtr(true)}

By("Expecting admitter would allow it")
admitter.admitAndExpect(policy, expectedOutcome)
},
Entry("allowed in a privileged namespace", privilegedNamespace, disableTheFeatureGate, shouldSucceed),
Entry("denied in a non-privileged namespace", nonPrivilegedNamespace, disableTheFeatureGate, shouldFail),
Entry("allowed in a privileged namespace when the feature gate is enabled", privilegedNamespace, enableTheFeatureGate, shouldSucceed),
Entry("allowed in a non-privileged namespace when the feature gate is enabled", nonPrivilegedNamespace, enableTheFeatureGate, shouldSucceed),
)

})

func createPolicyAdmissionReview(policy *migrationsv1.MigrationPolicy, namespace string) *admissionv1.AdmissionReview {
Expand Down
6 changes: 0 additions & 6 deletions pkg/virt-config/feature-gates.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ const (
DockerSELinuxMCSWorkaround = "DockerSELinuxMCSWorkaround"
PSA = "PSA"
VSOCKGate = "VSOCK"
// PSASeccompAllowsUserfaultfd tells us that the seccomp policy on the nodes allow the userfaultfd syscall, which is needed for post-copy migrations
PSASeccompAllowsUserfaultfd = "PSASeccompAllowsUserfaultfd"
// DisableCustomSELinuxPolicy disables the installation of the custom SELinux policy for virt-launcher
DisableCustomSELinuxPolicy = "DisableCustomSELinuxPolicy"
// KubevirtSeccompProfile indicate that Kubevirt will install its custom profile and
Expand Down Expand Up @@ -191,10 +189,6 @@ func (config *ClusterConfig) VSOCKEnabled() bool {
return config.isFeatureGateEnabled(VSOCKGate)
}

func (config *ClusterConfig) PSASeccompAllowsUserfaultfd() bool {
return config.isFeatureGateEnabled(PSASeccompAllowsUserfaultfd)
}

func (config *ClusterConfig) CustomSELinuxPolicyDisabled() bool {
return config.isFeatureGateEnabled(DisableCustomSELinuxPolicy)
}
Expand Down
16 changes: 2 additions & 14 deletions pkg/virt-controller/services/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ const (
const customSELinuxType = "virt_launcher.process"

type TemplateService interface {
RenderMigrationManifest(vmi *v1.VirtualMachineInstance, sourcePod *k8sv1.Pod, migrationConfiguration *v1.MigrationConfiguration) (*k8sv1.Pod, error)
RenderMigrationManifest(vmi *v1.VirtualMachineInstance, sourcePod *k8sv1.Pod) (*k8sv1.Pod, error)
RenderLaunchManifest(vmi *v1.VirtualMachineInstance) (*k8sv1.Pod, error)
RenderHotplugAttachmentPodTemplate(volume []*v1.Volume, ownerPod *k8sv1.Pod, vmi *v1.VirtualMachineInstance, claimMap map[string]*k8sv1.PersistentVolumeClaim, tempPod bool) (*k8sv1.Pod, error)
RenderHotplugAttachmentTriggerPodTemplate(volume *v1.Volume, ownerPod *k8sv1.Pod, vmi *v1.VirtualMachineInstance, pvcName string, isBlock bool, tempPod bool) (*k8sv1.Pod, error)
Expand Down Expand Up @@ -252,7 +252,7 @@ func (t *templateService) RenderLaunchManifestNoVm(vmi *v1.VirtualMachineInstanc
return t.renderLaunchManifest(vmi, nil, true)
}

func (t *templateService) RenderMigrationManifest(vmi *v1.VirtualMachineInstance, pod *k8sv1.Pod, migrationConfiguration *v1.MigrationConfiguration) (*k8sv1.Pod, error) {
func (t *templateService) RenderMigrationManifest(vmi *v1.VirtualMachineInstance, pod *k8sv1.Pod) (*k8sv1.Pod, error) {
reproducibleImageIDs, err := containerdisk.ExtractImageIDsFromSourcePod(vmi, pod)
if err != nil {
return nil, fmt.Errorf("can not proceed with the migration when no reproducible image digest can be detected: %v", err)
Expand All @@ -261,18 +261,6 @@ func (t *templateService) RenderMigrationManifest(vmi *v1.VirtualMachineInstance
if err != nil {
return nil, err
}

// PostCopy needs the userfaultfd syscall which can be restricted in the RuntimeDefault seccomp profile
if migrationConfiguration.AllowPostCopy != nil && *migrationConfiguration.AllowPostCopy &&
!t.clusterConfig.PSASeccompAllowsUserfaultfd() {
if podManifest.Spec.SecurityContext == nil {
podManifest.Spec.SecurityContext = &k8sv1.PodSecurityContext{}
}
if podManifest.Spec.SecurityContext.SeccompProfile == nil {
podManifest.Spec.SecurityContext.SeccompProfile = &k8sv1.SeccompProfile{}
}
podManifest.Spec.SecurityContext.SeccompProfile.Type = k8sv1.SeccompProfileTypeUnconfined
}
return podManifest, err
}

Expand Down
14 changes: 0 additions & 14 deletions pkg/virt-controller/services/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3540,20 +3540,6 @@ var _ = Describe("Template", func() {
}),
)

It("should compute the correct security context when migrating and postcopy is enabled and PSASeccompAllowsUserfaultfd is disabled", func() {
vmi := api.NewMinimalVMI("fake-vmi")
vmi.Status.RuntimeUser = uint64(nonRootUser)
pod, err := svc.RenderLaunchManifest(vmi)
Expect(err).ToNot(HaveOccurred())

migrationConfig := &v1.MigrationConfiguration{
AllowPostCopy: pointer.Bool(true),
}
pod, err = svc.RenderMigrationManifest(vmi, pod, migrationConfig)
Expect(err).ToNot(HaveOccurred())
Expect(pod.Spec.SecurityContext.SeccompProfile.Type).To(Equal(kubev1.SeccompProfileTypeUnconfined))
})

It("should compute the correct security context when rendering hotplug attachment pods", func() {
vmi := api.NewMinimalVMI("fake-vmi")
ownerPod, err := svc.RenderLaunchManifest(vmi)
Expand Down
2 changes: 0 additions & 2 deletions pkg/virt-controller/watch/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ go_library(
"//pkg/monitoring/vmstats:go_default_library",
"//pkg/network/sriov:go_default_library",
"//pkg/network/vmispec:go_default_library",
"//pkg/psa:go_default_library",
"//pkg/service:go_default_library",
"//pkg/storage/export/export:go_default_library",
"//pkg/storage/snapshot:go_default_library",
Expand Down Expand Up @@ -121,7 +120,6 @@ go_test(
"//pkg/controller:go_default_library",
"//pkg/instancetype:go_default_library",
"//pkg/network/sriov:go_default_library",
"//pkg/psa:go_default_library",
"//pkg/rest:go_default_library",
"//pkg/storage/export/export:go_default_library",
"//pkg/storage/snapshot:go_default_library",
Expand Down
51 changes: 1 addition & 50 deletions pkg/virt-controller/watch/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/workqueue"
"k8s.io/utils/pointer"

"kubevirt.io/kubevirt/pkg/apimachinery/patch"
"kubevirt.io/kubevirt/pkg/psa"
"kubevirt.io/kubevirt/pkg/util"
"kubevirt.io/kubevirt/pkg/util/pdbs"
"kubevirt.io/kubevirt/pkg/util/status"
Expand Down Expand Up @@ -563,41 +561,8 @@ func (c *MigrationController) updateStatus(migration *virtv1.VirtualMachineInsta
return nil
}

func (c *MigrationController) getMigrationConfiguration(vmi *virtv1.VirtualMachineInstance) (*virtv1.MigrationConfiguration, error) {
migrationConfig := c.clusterConfig.GetMigrationConfiguration().DeepCopy()
vmiNamespace, err := c.clientset.CoreV1().Namespaces().Get(context.Background(), vmi.Namespace, v1.GetOptions{})
if err != nil {
return nil, err
}

var policies []v1alpha1.MigrationPolicy
migrationInterfaceList := c.migrationPolicyInformer.GetStore().List()
for _, obj := range migrationInterfaceList {
policy := obj.(*v1alpha1.MigrationPolicy)
policies = append(policies, *policy)
}
policiesListObj := v1alpha1.MigrationPolicyList{Items: policies}

matchedPolicy := MatchPolicy(&policiesListObj, vmi, vmiNamespace)
if matchedPolicy == nil {
return migrationConfig, nil
}

_, err = matchedPolicy.GetMigrationConfByPolicy(migrationConfig)
if err != nil {
return nil, err
}
return migrationConfig, nil
}

func (c *MigrationController) createTargetPod(migration *virtv1.VirtualMachineInstanceMigration, vmi *virtv1.VirtualMachineInstance, sourcePod *k8sv1.Pod) error {

migrationConf, err := c.getMigrationConfiguration(vmi)
if err != nil {
return fmt.Errorf("failed to get migration configuration: %v", err)
}

templatePod, err := c.templateService.RenderMigrationManifest(vmi, sourcePod, migrationConf)
templatePod, err := c.templateService.RenderMigrationManifest(vmi, sourcePod)
if err != nil {
return fmt.Errorf("failed to render launch manifest: %v", err)
}
Expand Down Expand Up @@ -844,20 +809,6 @@ func (c *MigrationController) handleTargetPodHandoff(migration *virtv1.VirtualMa
vmiCopy.Status.MigrationState.MigrationConfiguration = clusterMigrationConfigs
}

if vmiCopy.Status.MigrationState.MigrationConfiguration.AllowPostCopy != nil &&
*vmiCopy.Status.MigrationState.MigrationConfiguration.AllowPostCopy {
isPrivileged, err := psa.IsNamespacePrivilegedWithStore(c.namespaceStore, vmiCopy.Namespace)
if err != nil {
return err
}

if !isPrivileged && !c.clusterConfig.PSASeccompAllowsUserfaultfd() {
vmiCopy.Status.MigrationState.MigrationConfiguration.AllowPostCopy = pointer.Bool(false)
log.Log.Object(vmi).Warningf("PostCopy disabled for migration %s/%s as the namespace is not privileged", migration.Namespace, migration.Name)
c.recorder.Eventf(migration, k8sv1.EventTypeWarning, WarningPostCopyNotAllowed, "Disabled PostCopy as the namespace is not privileged.")
}
}

err = c.patchVMI(vmi, vmiCopy)
if err != nil {
c.recorder.Eventf(migration, k8sv1.EventTypeWarning, FailedHandOverPodReason, fmt.Sprintf("Failed to set MigrationStat in VMI status. :%v", err))
Expand Down
44 changes: 0 additions & 44 deletions pkg/virt-controller/watch/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (

apimachpatch "kubevirt.io/kubevirt/pkg/apimachinery/patch"
virtcontroller "kubevirt.io/kubevirt/pkg/controller"
"kubevirt.io/kubevirt/pkg/psa"

"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/utils/pointer"
Expand Down Expand Up @@ -1653,49 +1652,6 @@ var _ = Describe("Migration watcher", func() {
),
)

DescribeTable("postcopy should be", func(privilegedNamespace bool) {
By("Defining a namespace")
namespace := &k8sv1.Namespace{
TypeMeta: metav1.TypeMeta{Kind: "Namespace"},
ObjectMeta: metav1.ObjectMeta{
Name: "namespace",
},
}

if privilegedNamespace {
namespace.Labels = map[string]string{
psa.PSALabel: "privileged",
}
}

Expect(namespaceStore.Add(namespace)).To(Succeed())
vmi.Namespace = namespace.Name

By("Defining migration policy, matching it to vmi and posting it into the cluster")
migrationPolicy := tests.PreparePolicyAndVMI(vmi)
migrationPolicy.Spec.AllowPostCopy = pointer.BoolPtr(true)
addMigrationPolicies(*migrationPolicy)

By("Expecting right patch to occur")
expectedConfigs := getDefaultMigrationConfiguration()
expectedConfigs.AllowPostCopy = pointer.BoolPtr(privilegedNamespace)
patch := getExpectedVmiPatch(true, expectedConfigs, migrationPolicy)
shouldExpectVirtualMachineInstancePatch(vmi, patch)

virtClient.EXPECT().VirtualMachineInstance(namespace.Name).Return(vmiInterface).AnyTimes()

By("Running the controller")
controller.Execute()

if !privilegedNamespace {
testutils.ExpectEvent(recorder, WarningPostCopyNotAllowed)
}
testutils.ExpectEvent(recorder, SuccessfulHandOverPodReason)
},
Entry("allowed in a privileged namespace", true),
Entry("denied in a non-privileged namespace", false),
)

})

Context("Migration of host-model VMI", func() {
Expand Down
Loading

0 comments on commit 0135cca

Please sign in to comment.