Support of read only mount of filesystem volumes added (#2365)

* Support of read only mount added Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com> * review comments fixed Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com> * renamings regarding review comments Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com> --------- Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
kanisterio · Oct 17, 2023 · c5432c7 · c5432c7
1 parent bf89e76
commit c5432c7
Show file tree

Hide file tree

Showing 11 changed files with 191 additions and 38 deletions.
diff --git a/pkg/controllers/repositoryserver/handler.go b/pkg/controllers/repositoryserver/handler.go
@@ -267,7 +267,7 @@ func (h *RepoServerHandler) setCredDataFromSecretInPod(ctx context.Context, podO
 		h.Logger.Info("Setting credentials data from secret as env variables")
 		podOptions.EnvironmentVariables = envVars
 	}
-	pod, err = kube.GetPodObjectFromPodOptions(h.KubeCli, podOptions)
+	pod, err = kube.GetPodObjectFromPodOptions(ctx, h.KubeCli, podOptions)
 	if err != nil {
 		return nil, nil, err
 	}

diff --git a/pkg/controllers/repositoryserver/utils.go b/pkg/controllers/repositoryserver/utils.go
@@ -199,7 +199,7 @@ func addTLSCertConfigurationInPodOverride(podOverride *map[string]interface{}, t
 	return nil
 }
 
-func getPodOptions(namespace string, svc *corev1.Service, vols map[string]string) *kube.PodOptions {
+func getPodOptions(namespace string, svc *corev1.Service, vols map[string]kube.VolumeMountOptions) *kube.PodOptions {
 	uidguid := int64(0)
 	nonRootBool := false
 	return &kube.PodOptions{
@@ -253,8 +253,13 @@ func getCondition(status metav1.ConditionStatus, reason string, message string,
 	}
 }
 
-func getVolumes(ctx context.Context, cli kubernetes.Interface, secret *corev1.Secret, namespace string) (map[string]string, error) {
-	vols := make(map[string]string, 0)
+func getVolumes(
+	ctx context.Context,
+	cli kubernetes.Interface,
+	secret *corev1.Secret,
+	namespace string,
+) (map[string]kube.VolumeMountOptions, error) {
+	vols := make(map[string]kube.VolumeMountOptions, 0)
 	var claimName []byte
 	if len(secret.Data) == 0 {
 		return nil, errors.Errorf(secerrors.EmptySecretErrorMessage, secret.Namespace, secret.Name)
@@ -263,11 +268,17 @@ func getVolumes(ctx context.Context, cli kubernetes.Interface, secret *corev1.Se
 		if claimName, ok = secret.Data[reposerver.ClaimNameKey]; !ok {
 			return nil, errors.New("Claim name not set for file store location secret, failed to retrieve PVC")
 		}
+
 		claimNameString := string(claimName)
-		if _, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, claimNameString, metav1.GetOptions{}); err != nil {
+		pvc, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, claimNameString, metav1.GetOptions{})
+		if err != nil {
 			return nil, errors.Wrapf(err, "Failed to validate if PVC %s:%s exists", namespace, claimName)
 		}
-		vols[claimNameString] = storage.DefaultFSMountPath
+
+		vols[claimNameString] = kube.VolumeMountOptions{
+			MountPath: storage.DefaultFSMountPath,
+			ReadOnly:  kube.PVCContainsReadOnlyAccessMode(pvc),
+		}
 	}
 	return vols, nil
 }
diff --git a/pkg/function/copy_volume_data.go b/pkg/function/copy_volume_data.go
@@ -69,20 +69,34 @@ func (*copyVolumeDataFunc) Name() string {
 	return CopyVolumeDataFuncName
 }
 
-func copyVolumeData(ctx context.Context, cli kubernetes.Interface, tp param.TemplateParams, namespace, pvc, targetPath, encryptionKey string, podOverride map[string]interface{}) (map[string]interface{}, error) {
+func copyVolumeData(
+	ctx context.Context,
+	cli kubernetes.Interface,
+	tp param.TemplateParams,
+	namespace,
+	pvcName,
+	targetPath,
+	encryptionKey string,
+	podOverride map[string]interface{},
+) (map[string]interface{}, error) {
 	// Validate PVC exists
-	if _, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvc, metav1.GetOptions{}); err != nil {
-		return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvc)
+	pvc, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvcName, metav1.GetOptions{})
+	if err != nil {
+		return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvcName)
 	}
+
 	// Create a pod with PVCs attached
-	mountPoint := fmt.Sprintf(CopyVolumeDataMountPoint, pvc)
+	mountPoint := fmt.Sprintf(CopyVolumeDataMountPoint, pvcName)
 	options := &kube.PodOptions{
 		Namespace:    namespace,
 		GenerateName: CopyVolumeDataJobPrefix,
 		Image:        consts.GetKanisterToolsImage(),
 		Command:      []string{"sh", "-c", "tail -f /dev/null"},
-		Volumes:      map[string]string{pvc: mountPoint},
-		PodOverride:  podOverride,
+		Volumes: map[string]kube.VolumeMountOptions{pvcName: {
+			MountPath: mountPoint,
+			ReadOnly:  kube.PVCContainsReadOnlyAccessMode(pvc),
+		}},
+		PodOverride: podOverride,
 	}
 	pr := kube.NewPodRunner(cli, options)
 	podFunc := copyVolumeDataPodFunc(cli, tp, mountPoint, targetPath, encryptionKey)

diff --git a/pkg/function/prepare_data.go b/pkg/function/prepare_data.go
@@ -85,17 +85,25 @@ func getVolumes(tp param.TemplateParams) (map[string]string, error) {
 
 func prepareData(ctx context.Context, cli kubernetes.Interface, namespace, serviceAccount, image string, vols map[string]string, podOverride crv1alpha1.JSONMap, command ...string) (map[string]interface{}, error) {
 	// Validate volumes
-	for pvc := range vols {
-		if _, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvc, metav1.GetOptions{}); err != nil {
-			return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvc)
+	validatedVols := make(map[string]kube.VolumeMountOptions)
+	for pvcName, mountPoint := range vols {
+		pvc, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvcName, metav1.GetOptions{})
+		if err != nil {
+			return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvcName)
+		}
+
+		validatedVols[pvcName] = kube.VolumeMountOptions{
+			MountPath: mountPoint,
+			ReadOnly:  kube.PVCContainsReadOnlyAccessMode(pvc),
 		}
 	}
+
 	options := &kube.PodOptions{
 		Namespace:          namespace,
 		GenerateName:       prepareDataJobPrefix,
 		Image:              image,
 		Command:            command,
-		Volumes:            vols,
+		Volumes:            validatedVols,
 		ServiceAccountName: serviceAccount,
 		PodOverride:        podOverride,
 	}

diff --git a/pkg/function/restore_data.go b/pkg/function/restore_data.go
@@ -115,17 +115,25 @@ func validateAndGetOptArgs(args map[string]interface{}, tp param.TemplateParams)
 func restoreData(ctx context.Context, cli kubernetes.Interface, tp param.TemplateParams, namespace, encryptionKey, backupArtifactPrefix, restorePath, backupTag, backupID, jobPrefix, image string,
 	vols map[string]string, podOverride crv1alpha1.JSONMap) (map[string]interface{}, error) {
 	// Validate volumes
-	for pvc := range vols {
-		if _, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvc, metav1.GetOptions{}); err != nil {
-			return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvc)
+	validatedVols := make(map[string]kube.VolumeMountOptions)
+	for pvcName, mountPoint := range vols {
+		pvc, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvcName, metav1.GetOptions{})
+		if err != nil {
+			return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvcName)
+		}
+
+		validatedVols[pvcName] = kube.VolumeMountOptions{
+			MountPath: mountPoint,
+			ReadOnly:  kube.PVCContainsReadOnlyAccessMode(pvc),
 		}
 	}
+
 	options := &kube.PodOptions{
 		Namespace:    namespace,
 		GenerateName: jobPrefix,
 		Image:        image,
 		Command:      []string{"sh", "-c", "tail -f /dev/null"},
-		Volumes:      vols,
+		Volumes:      validatedVols,
 		PodOverride:  podOverride,
 	}
 	pr := kube.NewPodRunner(cli, options)

diff --git a/pkg/function/restore_data_using_kopia_server.go b/pkg/function/restore_data_using_kopia_server.go
@@ -184,9 +184,17 @@ func restoreDataFromServer(
 	vols map[string]string,
 	podOverride crv1alpha1.JSONMap,
 ) (map[string]any, error) {
-	for pvc := range vols {
-		if _, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvc, metav1.GetOptions{}); err != nil {
-			return nil, errors.Wrap(err, "Failed to retrieve PVC from namespace: "+namespace+" name: "+pvc)
+	validatedVols := make(map[string]kube.VolumeMountOptions)
+	// Validate volumes
+	for pvcName, mountPoint := range vols {
+		pvc, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvcName, metav1.GetOptions{})
+		if err != nil {
+			return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvcName)
+		}
+
+		validatedVols[pvcName] = kube.VolumeMountOptions{
+			MountPath: mountPoint,
+			ReadOnly:  kube.PVCContainsReadOnlyAccessMode(pvc),
 		}
 	}
 
@@ -195,7 +203,7 @@ func restoreDataFromServer(
 		GenerateName: jobPrefix,
 		Image:        image,
 		Command:      []string{"bash", "-c", "tail -f /dev/null"},
-		Volumes:      vols,
+		Volumes:      validatedVols,
 		PodOverride:  podOverride,
 	}
 

diff --git a/pkg/kube/job.go b/pkg/kube/job.go
@@ -40,12 +40,19 @@ type Job struct {
 	name      string
 	sa        string
 	// vols is a map of PVC->Mount points to add to the job pod spec
-	vols      map[string]string
+	vols      map[string]VolumeMountOptions
 	clientset kubernetes.Interface
 }
 
 // NewJob creates a new Job object.
-func NewJob(clientset kubernetes.Interface, jobName string, namespace string, serviceAccount string, image string, vols map[string]string, command ...string) (*Job, error) {
+func NewJob(clientset kubernetes.Interface,
+	jobName string,
+	namespace string,
+	serviceAccount string,
+	image string,
+	vols map[string]VolumeMountOptions,
+	command ...string,
+) (*Job, error) {
 	if jobName == "" {
 		return nil, errors.New("Job name is required")
 	}
@@ -73,7 +80,8 @@ func NewJob(clientset kubernetes.Interface, jobName string, namespace string, se
 // Create creates the Job in Kubernetes.
 func (job *Job) Create() error {
 	falseVal := false
-	volumeMounts, podVolumes, err := createFilesystemModeVolumeSpecs(job.vols)
+	ctx := context.TODO()
+	volumeMounts, podVolumes, err := createFilesystemModeVolumeSpecs(ctx, job.vols)
 	if err != nil {
 		return errors.Wrapf(err, "Failed to create volume spec for job %s", job.name)
 	}
@@ -115,7 +123,7 @@ func (job *Job) Create() error {
 	batchClient := job.clientset.BatchV1()
 	jobsClient := batchClient.Jobs(job.namespace)
 
-	newJob, err := jobsClient.Create(context.TODO(), k8sJob, metav1.CreateOptions{})
+	newJob, err := jobsClient.Create(ctx, k8sJob, metav1.CreateOptions{})
 	if err != nil {
 		return errors.Wrapf(err, "Failed to create job %s", job.name)
 	}
@@ -125,21 +133,30 @@ func (job *Job) Create() error {
 	return nil
 }
 
-func createFilesystemModeVolumeSpecs(vols map[string]string) (volumeMounts []v1.VolumeMount, podVolumes []v1.Volume, error error) {
+func createFilesystemModeVolumeSpecs(
+	ctx context.Context,
+	vols map[string]VolumeMountOptions,
+) (volumeMounts []v1.VolumeMount, podVolumes []v1.Volume, error error) {
 	// Build filesystem mode volume specs
-	for pvc, mountPath := range vols {
+	for pvcName, mountOpts := range vols {
 		id, err := uuid.NewV1()
 		if err != nil {
 			return nil, nil, err
 		}
+
+		if mountOpts.ReadOnly {
+			log.Debug().WithContext(ctx).Print("PVC will be mounted in read-only mode", field.M{"pvcName": pvcName})
+		}
+
 		podVolName := fmt.Sprintf("vol-%s", id.String())
-		volumeMounts = append(volumeMounts, v1.VolumeMount{Name: podVolName, MountPath: mountPath})
+		volumeMounts = append(volumeMounts, v1.VolumeMount{Name: podVolName, MountPath: mountOpts.MountPath, ReadOnly: mountOpts.ReadOnly})
 		podVolumes = append(podVolumes,
 			v1.Volume{
 				Name: podVolName,
 				VolumeSource: v1.VolumeSource{
 					PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-						ClaimName: pvc,
+						ClaimName: pvcName,
+						ReadOnly:  mountOpts.ReadOnly,
 					},
 				},
 			},

diff --git a/pkg/kube/job_test.go b/pkg/kube/job_test.go
@@ -236,7 +236,27 @@ func (s *JobSuite) TestJobsWaitOnNonExistentJob(c *C) {
 
 func (s *JobSuite) TestJobsVolumes(c *C) {
 	cli := fake.NewSimpleClientset()
-	vols := map[string]string{"pvc-test": "/mnt/data1"}
+	vols := map[string]VolumeMountOptions{"pvc-test": {MountPath: "/mnt/data1", ReadOnly: false}}
+	job, err := NewJob(cli, testJobName, testJobNamespace, testJobServiceAccount, testJobImage, vols, "sleep", "300")
+	c.Assert(err, IsNil)
+	c.Assert(job.Create(), IsNil)
+
+	a := cli.Actions()
+	c.Assert(a, HasLen, 1)
+	createAction := a[0]
+	createdJob, ok := createAction.(k8stesting.CreateAction).GetObject().(*batch.Job)
+	c.Assert(ok, Equals, true)
+
+	c.Assert(createdJob.Name, Equals, testJobName)
+	podSpec := createdJob.Spec.Template.Spec
+	c.Assert(podSpec.Volumes, HasLen, 1)
+	c.Assert(podSpec.Volumes[0].VolumeSource.PersistentVolumeClaim.ClaimName, Equals, "pvc-test")
+	c.Assert(podSpec.Containers[0].VolumeMounts[0].MountPath, Equals, "/mnt/data1")
+}
+
+func (s *JobSuite) TestJobsReadOnlyVolumes(c *C) {
+	cli := fake.NewSimpleClientset()
+	vols := map[string]VolumeMountOptions{"pvc-test": {MountPath: "/mnt/data1", ReadOnly: true}}
 	job, err := NewJob(cli, testJobName, testJobNamespace, testJobServiceAccount, testJobImage, vols, "sleep", "300")
 	c.Assert(err, IsNil)
 	c.Assert(job.Create(), IsNil)

diff --git a/pkg/kube/pod.go b/pkg/kube/pod.go
@@ -48,6 +48,11 @@ const (
 	defaultContainerName   = "container"
 )
 
+type VolumeMountOptions struct {
+	MountPath string
+	ReadOnly  bool
+}
+
 // PodOptions specifies options for `CreatePod`
 type PodOptions struct {
 	Annotations        map[string]string
@@ -59,7 +64,7 @@ type PodOptions struct {
 	Labels             map[string]string
 	Namespace          string
 	ServiceAccountName string
-	Volumes            map[string]string
+	Volumes            map[string]VolumeMountOptions
 	BlockVolumes       map[string]string
 	// PodSecurityContext and ContainerSecurityContext can be used to set the security context
 	// at the pod level and container level respectively.
@@ -76,7 +81,7 @@ type PodOptions struct {
 	Lifecycle                *v1.Lifecycle
 }
 
-func GetPodObjectFromPodOptions(cli kubernetes.Interface, opts *PodOptions) (*v1.Pod, error) {
+func GetPodObjectFromPodOptions(ctx context.Context, cli kubernetes.Interface, opts *PodOptions) (*v1.Pod, error) {
 	// If Namespace is not specified, use the controller Namespace.
 	cns, err := GetControllerNamespace()
 	if err != nil {
@@ -101,7 +106,7 @@ func GetPodObjectFromPodOptions(cli kubernetes.Interface, opts *PodOptions) (*v1
 		opts.RestartPolicy = v1.RestartPolicyNever
 	}
 
-	volumeMounts, podVolumes, err := createFilesystemModeVolumeSpecs(opts.Volumes)
+	volumeMounts, podVolumes, err := createFilesystemModeVolumeSpecs(ctx, opts.Volumes)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed to create volume spec")
 	}
@@ -207,11 +212,13 @@ func ContainerNameFromPodOptsOrDefault(po *PodOptions) string {
 
 // CreatePod creates a pod with a single container based on the specified image
 func CreatePod(ctx context.Context, cli kubernetes.Interface, opts *PodOptions) (*v1.Pod, error) {
-	pod, err := GetPodObjectFromPodOptions(cli, opts)
+	pod, err := GetPodObjectFromPodOptions(ctx, cli, opts)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed to get pod from podOptions. Namespace: %s, NameFmt: %s", opts.Namespace, opts.GenerateName)
 	}
 
+	log.Debug().WithContext(ctx).Print("Creating POD", field.M{"name": pod.Name, "namespace": pod.Namespace})
+
 	pod, err = cli.CoreV1().Pods(pod.Namespace).Create(ctx, pod, metav1.CreateOptions{})
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed to create pod. Namespace: %s, NameFmt: %s", opts.Namespace, opts.GenerateName)