Skip to content

Commit

Permalink
run tests in restricted namespace and required changes
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Henriksen <[email protected]>
  • Loading branch information
mhenriks committed Sep 6, 2022
1 parent ee2f0f5 commit 1f1c0fc
Show file tree
Hide file tree
Showing 28 changed files with 183 additions and 190 deletions.
10 changes: 5 additions & 5 deletions cluster-sync/clean.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ OPERATOR_MANIFEST=./_out/manifests/release/cdi-operator.yaml
LABELS=("operator.cdi.kubevirt.io" "cdi.kubevirt.io" "prometheus.cdi.kubevirt.io")
NAMESPACES=(default kube-system cdi)

set +e
_kubectl patch cdi ${CR_NAME} --type=json -p '[{ "op": "remove", "path": "/metadata/finalizers" }]'
set -e

_kubectl get ot --all-namespaces -o=custom-columns=NAME:.metadata.name,NAMESPACE:.metadata.namespace,FINALIZERS:.metadata.finalizers --no-headers | grep objectTransfer | while read p; do
arr=($p)
name="${arr[0]}"
Expand Down Expand Up @@ -52,10 +48,14 @@ if [ -f "${OPERATOR_CR_MANIFEST}" ]; then
echo "Cleaning CR object ..."
if _kubectl get crd cdis.cdi.kubevirt.io ; then
_kubectl delete --ignore-not-found -f "${OPERATOR_CR_MANIFEST}"
_kubectl wait cdis.cdi.kubevirt.io/${CR_NAME} --for=delete | echo "this is fine"
_kubectl wait cdis.cdi.kubevirt.io/${CR_NAME} --for=delete --timeout=30s | echo "this is fine"
fi
fi

set +e
_kubectl patch cdi ${CR_NAME} --type=json -p '[{ "op": "remove", "path": "/metadata/finalizers" }]' | echo "this is fine"
set -e

if [ "${CDI_CLEAN}" == "all" ] && [ -f "${OPERATOR_MANIFEST}" ]; then
echo "Deleting operator ..."
_kubectl delete --ignore-not-found -f "${OPERATOR_MANIFEST}"
Expand Down
1 change: 1 addition & 0 deletions cmd/cdi-cloner/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,6 @@ container_image(
":cdi-cloner",
":cloner_startup.sh",
],
user = "1001",
visibility = ["//visibility:public"],
)
23 changes: 20 additions & 3 deletions cmd/cdi-cloner/clone-source.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,29 @@ func validateMount() {
}

func newTarReader(preallocation bool) (io.ReadCloser, error) {
args := "cv"
excludeMap := map[string]struct{}{
"lost+found": struct{}{},
}

args := []string{"cv"}
if !preallocation {
// -S is used to handle sparse files. It can only be used when preallocation is not requested
args = "S" + args
args = append(args, "-S")
}

files, err := os.ReadDir(mountPoint)
if err != nil {
return nil, err
}
cmd := exec.Command("/usr/bin/tar", args, ".")

for _, f := range files {
if _, ok := excludeMap[f.Name()]; ok {
continue
}
args = append(args, f.Name())
}

cmd := exec.Command("/usr/bin/tar", args...)
cmd.Dir = mountPoint

stdout, err := cmd.StdoutPipe()
Expand Down
1 change: 1 addition & 0 deletions cmd/cdi-importer/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ container_image(
"//tools/cdi-image-size-detection",
"//tools/cdi-source-update-poller",
],
user = "1001",
visibility = ["//visibility:public"],
)

Expand Down
1 change: 1 addition & 0 deletions cmd/cdi-uploadserver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ container_image(
"-alsologtostderr",
],
files = [":cdi-uploadserver"],
user = "1001",
visibility = ["//visibility:public"],
)

Expand Down
12 changes: 2 additions & 10 deletions pkg/controller/clone-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
Expand Down Expand Up @@ -573,7 +572,7 @@ func MakeCloneSourcePodSpec(sourceVolumeMode corev1.PersistentVolumeMode, image,
}
}

preallocationRequested, _ := targetPvc.Annotations[AnnPreallocationRequested]
preallocationRequested := targetPvc.Annotations[AnnPreallocationRequested]

pod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -644,14 +643,6 @@ func MakeCloneSourcePodSpec(sourceVolumeMode corev1.PersistentVolumeMode, image,
Protocol: corev1.ProtocolTCP,
},
},
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
AllowPrivilegeEscalation: pointer.BoolPtr(false),
},
},
},
RestartPolicy: corev1.RestartPolicyOnFailure,
Expand Down Expand Up @@ -740,6 +731,7 @@ func MakeCloneSourcePodSpec(sourceVolumeMode corev1.PersistentVolumeMode, image,

pod.Spec.Containers[0].Env = append(pod.Spec.Containers[0].Env, addVars...)
SetPodPvcAnnotations(pod, targetPvc)
SetRestrictedSecurityContext(&pod.Spec)
return pod
}

Expand Down
10 changes: 1 addition & 9 deletions pkg/controller/dataimportcron-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/event"
Expand Down Expand Up @@ -752,14 +751,6 @@ func (r *DataImportCronReconciler) newCronJob(cron *cdiv1.DataImportCron) (*batc
return nil, err
}
container := corev1.Container{
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
AllowPrivilegeEscalation: pointer.BoolPtr(false),
},
Name: "cdi-source-update-poller",
Image: r.image,
Command: []string{
Expand Down Expand Up @@ -873,6 +864,7 @@ func (r *DataImportCronReconciler) newCronJob(cron *cdiv1.DataImportCron) (*batc
if err := sdk.SetLastAppliedConfiguration(cronJob, AnnLastAppliedConfig); err != nil {
return nil, err
}
SetRestrictedSecurityContext(&cronJob.Spec.JobTemplate.Spec.Template.Spec)
return cronJob, nil
}

Expand Down
21 changes: 4 additions & 17 deletions pkg/controller/datavolume-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
Expand Down Expand Up @@ -1630,14 +1629,6 @@ func (r *DatavolumeReconciler) createExpansionPod(pvc *corev1.PersistentVolumeCl
ImagePullPolicy: corev1.PullPolicy(r.pullPolicy),
Command: []string{"/bin/bash"},
Args: []string{"-c", "echo", "'hello cdi'"},
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
AllowPrivilegeEscalation: pointer.BoolPtr(false),
},
},
},
RestartPolicy: corev1.RestartPolicyOnFailure,
Expand Down Expand Up @@ -1677,6 +1668,8 @@ func (r *DatavolumeReconciler) createExpansionPod(pvc *corev1.PersistentVolumeCl
return nil, err
}

SetRestrictedSecurityContext(&pod.Spec)

if err := r.client.Create(context.TODO(), pod); err != nil {
if !k8serrors.IsAlreadyExists(err) {
return nil, err
Expand Down Expand Up @@ -3149,6 +3142,8 @@ func (r *DatavolumeReconciler) makeSizeDetectionPodSpec(
},
}

SetRestrictedSecurityContext(&pod.Spec)

return pod
}

Expand Down Expand Up @@ -3185,14 +3180,6 @@ func (r *DatavolumeReconciler) makeSizeDetectionContainerSpec(volName string) *c
Name: volName,
},
},
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
AllowPrivilegeEscalation: pointer.BoolPtr(false),
},
}

// Get and assign container's default resource requirements
Expand Down
41 changes: 6 additions & 35 deletions pkg/controller/import-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
Expand Down Expand Up @@ -943,7 +942,7 @@ func createImporterPod(log logr.Logger, client client.Client, args *importerPodA
// makeNodeImporterPodSpec creates and returns the node docker cache based importer pod spec based on the passed-in importImage and pvc.
func makeNodeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
// importer pod name contains the pvc name
podName, _ := args.pvc.Annotations[AnnImportPod]
podName := args.pvc.Annotations[AnnImportPod]

volumes := []corev1.Volume{
{
Expand Down Expand Up @@ -995,14 +994,6 @@ func makeNodeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
Name: "shared-volume",
},
},
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
AllowPrivilegeEscalation: pointer.BoolPtr(false),
},
},
},
Containers: []corev1.Container{
Expand All @@ -1018,14 +1009,6 @@ func makeNodeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
Name: "shared-volume",
},
},
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
AllowPrivilegeEscalation: pointer.BoolPtr(false),
},
},
},
RestartPolicy: corev1.RestartPolicyOnFailure,
Expand Down Expand Up @@ -1072,13 +1055,15 @@ func makeNodeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
Name: "shared-volume",
})

SetRestrictedSecurityContext(&pod.Spec)

return pod
}

// makeImporterPodSpec creates and return the importer pod spec based on the passed-in endpoint, secret and pvc.
func makeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
// importer pod name contains the pvc name
podName, _ := args.pvc.Annotations[AnnImportPod]
podName := args.pvc.Annotations[AnnImportPod]

blockOwnerDeletion := true
isController := true
Expand Down Expand Up @@ -1230,6 +1215,8 @@ func makeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
pod.Spec.Volumes = append(pod.Spec.Volumes, vol)
}

SetRestrictedSecurityContext(&pod.Spec)

return pod
}

Expand All @@ -1253,14 +1240,6 @@ func setImporterPodCommons(pod *corev1.Pod, podEnvVar *importPodEnvVar, pvc *cor

pod.Spec.Containers[0].Env = makeImportEnv(podEnvVar, ownerUID)

if podEnvVar.contentType == string(cdiv1.DataVolumeKubeVirt) {
// Set the fsGroup on the security context to the QemuSubGid
if pod.Spec.SecurityContext == nil {
pod.Spec.SecurityContext = &corev1.PodSecurityContext{}
}
fsGroup := common.QemuSubGid
pod.Spec.SecurityContext.FSGroup = &fsGroup
}
SetPodPvcAnnotations(pod, pvc)
}

Expand All @@ -1277,14 +1256,6 @@ func makeImporterContainerSpec(image, verbose, pullPolicy string) *corev1.Contai
Protocol: corev1.ProtocolTCP,
},
},
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
AllowPrivilegeEscalation: pointer.BoolPtr(false),
},
}
}

Expand Down
25 changes: 0 additions & 25 deletions pkg/controller/import-controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,6 @@ var _ = Describe("ImportConfig Controller reconcile loop", func() {
}
}
Expect(foundEndPoint).To(BeTrue())
By("Verifying the fsGroup of the pod is the qemu user")
Expect(*pod.Spec.SecurityContext.FSGroup).To(Equal(int64(107)))
})

It("Should create a POD with node placement", func() {
Expand Down Expand Up @@ -332,8 +330,6 @@ var _ = Describe("ImportConfig Controller reconcile loop", func() {
}
}
Expect(foundEndPoint).To(BeTrue())
By("Verifying the fsGroup of the pod is the qemu user")
Expect(*pod.Spec.SecurityContext.FSGroup).To(Equal(int64(107)))
By("Verifying the pod is annotated correctly")
Expect(pod.GetAnnotations()[AnnPodNetwork]).To(Equal("net1"))
Expect(pod.GetAnnotations()[AnnPodSidecarInjection]).To(Equal(AnnPodSidecarInjectionDefault))
Expand All @@ -360,27 +356,6 @@ var _ = Describe("ImportConfig Controller reconcile loop", func() {
Expect(pod.GetAnnotations()["annot1"]).ToNot(Equal("value1"))
})

It("Should create a POD if a bound PVC with all needed annotations is passed, but not set fsgroup if not kubevirt contenttype", func() {
pvc := createPvc("testPvc1", "default", map[string]string{AnnEndpoint: testEndPoint, AnnImportPod: "importer-testPvc1", AnnContentType: string(cdiv1.DataVolumeArchive)}, nil)
pvc.Status.Phase = v1.ClaimBound
reconciler = createImportReconciler(pvc)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "testPvc1", Namespace: "default"}})
Expect(err).ToNot(HaveOccurred())
pod := &corev1.Pod{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "importer-testPvc1", Namespace: "default"}, pod)
Expect(err).ToNot(HaveOccurred())
foundEndPoint := false
for _, envVar := range pod.Spec.Containers[0].Env {
if envVar.Name == common.ImporterEndpoint {
foundEndPoint = true
Expect(envVar.Value).To(Equal(testEndPoint))
}
}
Expect(foundEndPoint).To(BeTrue())
By("Verifying the fsGroupis not set")
Expect(pod.Spec.SecurityContext).To(BeNil())
})

It("Should error if a POD with the same name exists, but is not owned by the PVC, if a PVC with all needed annotations is passed", func() {
pod := &corev1.Pod{
TypeMeta: metav1.TypeMeta{
Expand Down
Loading

0 comments on commit 1f1c0fc

Please sign in to comment.