From 7b44aef1a860eda0cae3c65c0c3fb817d8580138 Mon Sep 17 00:00:00 2001 From: aram price Date: Mon, 8 Jul 2024 13:46:56 -0700 Subject: [PATCH 1/2] Rename local variables for clarity Signed-off-by: Nader Ziada --- src/bpm/runc/adapter/adapter.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bpm/runc/adapter/adapter.go b/src/bpm/runc/adapter/adapter.go index e99d36c0..5eebcc14 100644 --- a/src/bpm/runc/adapter/adapter.go +++ b/src/bpm/runc/adapter/adapter.go @@ -232,11 +232,11 @@ func (a *RuncAdapter) BuildSpec( ms.addMounts(boshMounts) ms.addMounts(userProvidedIdentityMounts(bpmCfg, procCfg.AdditionalVolumes)) if procCfg.Unsafe != nil && len(procCfg.Unsafe.UnrestrictedVolumes) > 0 { - expanded, err := a.globExpandVolumes(procCfg.Unsafe.UnrestrictedVolumes) + expandUnrestrictedVolumes, err := a.globExpandVolumes(procCfg.Unsafe.UnrestrictedVolumes) if err != nil { return specs.Spec{}, err } - filteredVolumes := filterVolumesUnderBoshMounts(boshMounts, expanded) + filteredVolumes := filterVolumesUnderBoshMounts(boshMounts, expandUnrestrictedVolumes) ms.addMounts(userProvidedIdentityMounts(bpmCfg, filteredVolumes)) } @@ -288,11 +288,11 @@ func (a *RuncAdapter) BuildSpec( return *spec, nil } -func filterVolumesUnderBoshMounts(mounts []specs.Mount, volumes []config.Volume) []config.Volume { +func filterVolumesUnderBoshMounts(boshMounts []specs.Mount, unrestrictedVolumes []config.Volume) []config.Volume { var filteredVolumes []config.Volume - for _, v := range volumes { + for _, v := range unrestrictedVolumes { keep := true - for _, m := range mounts { + for _, m := range boshMounts { if strings.HasPrefix(v.Path, m.Destination) { keep = false } From 6a2f85d58efe5ea063c8d0b37bb1f45b4ade66df Mon Sep 17 00:00:00 2001 From: Nader Ziada Date: Tue, 9 Jul 2024 13:58:48 -0700 Subject: [PATCH 2/2] Compare path-parts instead of path-strings when filtering mounts Check to see whether the "directory parts" of the volume are a sub-set of and existing BPM-default directory (that will already be mounted) so that we do not accidentally filter out mounts which have a name that is a sub-string of the existing job. For example the job `service-metrics` should be able to have an unrestricted volume mount of the `service-metrics-adapter` job directory. Specifically a job located at `/var/vcap/jobs/service-metrics` was unable to access an "unrestricted volume" located at `/var/vcap/jobs/service-metrics-adapter` because the job being instantiated alreay had default mount which was a (string) prefix of the job directory it was attempting to mount. Signed-off-by: Rajath Agasthya Signed-off-by: aram price --- src/bpm/runc/adapter/adapter.go | 15 +++++++++++++-- src/bpm/runc/adapter/adapter_test.go | 11 +++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/bpm/runc/adapter/adapter.go b/src/bpm/runc/adapter/adapter.go index 5eebcc14..0add5cd6 100644 --- a/src/bpm/runc/adapter/adapter.go +++ b/src/bpm/runc/adapter/adapter.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "code.cloudfoundry.org/bytefmt" @@ -293,8 +294,18 @@ func filterVolumesUnderBoshMounts(boshMounts []specs.Mount, unrestrictedVolumes for _, v := range unrestrictedVolumes { keep := true for _, m := range boshMounts { - if strings.HasPrefix(v.Path, m.Destination) { - keep = false + // Check to see whether the "directory parts" of the volume are a sub-set of and existing BPM-default + // directory (that will already be mounted) so that we do not accidentally filter out mounts which + // have a name that is a sub-string of the existing job. For example the job `service-metrics` should be + // able to have an unrestricted volume mount of the `service-metrics-adapter` job directory. + boshMountDirParts := strings.Split(m.Destination, fmt.Sprintf("%c", filepath.Separator)) + volumeDirParts := strings.Split(v.Path, fmt.Sprintf("%c", filepath.Separator)) + + if len(boshMountDirParts) <= len(volumeDirParts) { + volumeDirPartsPrefix := volumeDirParts[:len(boshMountDirParts)] + if slices.Compare(boshMountDirParts, volumeDirPartsPrefix) == 0 { + keep = false + } } } diff --git a/src/bpm/runc/adapter/adapter_test.go b/src/bpm/runc/adapter/adapter_test.go index f33e7688..347f7b77 100644 --- a/src/bpm/runc/adapter/adapter_test.go +++ b/src/bpm/runc/adapter/adapter_test.go @@ -869,7 +869,8 @@ var _ = Describe("RuncAdapter", func() { {Path: "/this/is/an/unrestricted/path"}, {Path: "/writable/executable/path", Writable: true, AllowExecutions: true}, {Path: "/var/vcap/jobs/example/config/config.yml", MountOnly: true}, - {Path: "/var/vcap/jobs/other/config/config.yml", MountOnly: true}, + {Path: "/var/vcap/jobs/other/config/config.yml", MountOnly: true, AllowExecutions: true}, + {Path: "/var/vcap/jobs/example-two/config/config.yml", MountOnly: true, AllowExecutions: true}, }, } }) @@ -894,7 +895,13 @@ var _ = Describe("RuncAdapter", func() { Destination: "/var/vcap/jobs/other/config/config.yml", Type: "bind", Source: "/var/vcap/jobs/other/config/config.yml", - Options: []string{"nodev", "nosuid", "noexec", "rbind", "ro"}, + Options: []string{"nodev", "nosuid", "rbind", "exec", "ro"}, + })) + Expect(spec.Mounts).To(HaveMount(specs.Mount{ + Destination: "/var/vcap/jobs/example-two/config/config.yml", + Type: "bind", + Source: "/var/vcap/jobs/example-two/config/config.yml", + Options: []string{"nodev", "nosuid", "rbind", "exec", "ro"}, })) })