From b56d6269ef2cc0cf8ef78265ca1f550dfed19ef5 Mon Sep 17 00:00:00 2001 From: luke Date: Mon, 6 Oct 2025 17:27:29 +0100 Subject: [PATCH 01/29] Create proxy in the correct namespace defined by specs Signed-off-by: luke --- internal/proxy/proxy.go | 43 ++++++++++++++++++++++++++++++++++++-- internal/runtime/create.go | 2 +- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index ee4b0426..9499fbf0 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -5,17 +5,56 @@ import ( "os" "os/exec" "syscall" + + "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/sys/unix" ) -func NewProcess(devicePath string) (int, error) { +var namespaceFlags = map[specs.LinuxNamespaceType]uintptr{ + specs.CgroupNamespace: unix.CLONE_NEWCGROUP, + specs.IPCNamespace: unix.CLONE_NEWIPC, + specs.MountNamespace: unix.CLONE_NEWNS, + specs.NetworkNamespace: unix.CLONE_NEWNET, + specs.PIDNamespace: unix.CLONE_NEWPID, + specs.TimeNamespace: unix.CLONE_NEWTIME, + specs.UserNamespace: unix.CLONE_NEWUSER, + specs.UTSNamespace: unix.CLONE_NEWUTS, +} + +func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { + if spec == nil { + return 0, nil + } + + var flags uintptr + for _, ns := range spec.Linux.Namespaces { + if ns.Path != "" { + continue + } + flag, err := namespaceFlags[ns.Type] + if err { + return 0, fmt.Errorf("Unknown namespace type %q", ns.Type) + } + flags |= flag + } + return flags, nil +} + +func NewProcess(spec *specs.Spec, devicePath string) (int, error) { execPath, err := os.Executable() if err != nil { return -1, fmt.Errorf("failed to get executable path: %w", err) } + namespaceFlags, err := namespaceCloneFlags(spec) + if err != nil { + return -1, err + } + cmd := exec.Command(execPath, "proxy", "--device-path", devicePath) cmd.SysProcAttr = &syscall.SysProcAttr{ - Setpgid: true, + Setpgid: true, + Cloneflags: namespaceFlags, } if err := cmd.Start(); err != nil { diff --git a/internal/runtime/create.go b/internal/runtime/create.go index 9666b361..1cddfb56 100644 --- a/internal/runtime/create.go +++ b/internal/runtime/create.go @@ -46,7 +46,7 @@ func Create(containerID string, bundlePath string, pidFile string) error { } }() - pid, err := proxy.NewProcess(devicePath) + pid, err := proxy.NewProcess(spec, devicePath) if err != nil { return fmt.Errorf("failed to start proxy process: %w", err) } From c8f002690b02c79384241bfa16bf0f9562a498c3 Mon Sep 17 00:00:00 2001 From: luke Date: Mon, 6 Oct 2025 17:28:08 +0100 Subject: [PATCH 02/29] Use OK rather than err Signed-off-by: luke --- internal/proxy/proxy.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 9499fbf0..4538d988 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -31,8 +31,8 @@ func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { if ns.Path != "" { continue } - flag, err := namespaceFlags[ns.Type] - if err { + flag, ok := namespaceFlags[ns.Type] + if !ok { return 0, fmt.Errorf("Unknown namespace type %q", ns.Type) } flags |= flag From 670e0a463744791f6e742b02e0c179ee16b3a39d Mon Sep 17 00:00:00 2001 From: luke Date: Tue, 7 Oct 2025 11:22:26 +0100 Subject: [PATCH 03/29] Add a test for inheriting the correct namespace Signed-off-by: luke --- e2e/runtime_test.go | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/e2e/runtime_test.go b/e2e/runtime_test.go index 04966fc3..d8ca5c50 100644 --- a/e2e/runtime_test.go +++ b/e2e/runtime_test.go @@ -128,6 +128,45 @@ func TestRuntimeWriteProcessPid(t *testing.T) { assertFileContent(t, pidFilePath, fmt.Sprintf("%d", pid)) } +func TestRuntimeProxyInheritsCorrectNamespace(t *testing.T) { + rootDir := t.TempDir() + remoteprocName := "a-lovely-blue-device" + sim := remoteproc.NewSimulator(rootDir).WithName(remoteprocName) + if err := sim.Start(); err != nil { + t.Fatalf("failed to run simulator: %s", err) + } + defer func() { _ = sim.Stop() }() + + bin, err := repo.BuildRuntimeBin(t.TempDir(), rootDir, nil) + require.NoError(t, err) + + const containerName = "blue-container" + bundlePath := t.TempDir() + + require.NoError(t, generateBundle( + bundlePath, + remoteprocName, + specs.LinuxNamespace{Type: specs.MountNamespace}, + )) + + _, err = invokeRuntime(bin, "create", "--bundle", bundlePath, containerName) + require.NoError(t, err) + + pid, err := getContainerPid(bin, containerName) + require.NoError(t, err) + require.Greater(t, pid, 0) + + hostMountNS, err := os.Readlink("/proc/self/ns/mnt") + require.NoError(t, err) + proxyMountNS, err := os.Readlink(fmt.Sprintf("/proc/%d/ns/mnt", pid)) + require.NoError(t, err) + + assert.NotEqual(t, hostMountNS, proxyMountNS) + + _, err = invokeRuntime(bin, "delete", containerName) + require.NoError(t, err) +} + func assertContainerStatus(t testing.TB, bin repo.RuntimeBin, containerName string, wantStatus specs.ContainerState) { t.Helper() state, err := getContainerState(bin, containerName) @@ -156,7 +195,7 @@ func getContainerState(bin repo.RuntimeBin, containerName string) (specs.State, return state, nil } -func generateBundle(targetDir string, remoteprocName string) error { +func generateBundle(targetDir string, remoteprocName string, namespaces ...specs.LinuxNamespace) error { const bundleRoot = "rootfs" const firmwareName = "hello_world.elf" @@ -179,6 +218,7 @@ func generateBundle(targetDir string, remoteprocName string) error { Annotations: map[string]string{ "remoteproc.name": remoteprocName, }, + Linux: &specs.Linux{Namespaces: namespaces}, } specPath := filepath.Join(targetDir, "config.json") From d57170c54ac1df0e91491a6a9201b4b94ba6c590 Mon Sep 17 00:00:00 2001 From: luke Date: Tue, 7 Oct 2025 16:49:54 +0100 Subject: [PATCH 04/29] If running without root, do not do namespace isolation Signed-off-by: luke --- internal/proxy/proxy.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 4538d988..e6bbcf14 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -21,6 +21,8 @@ var namespaceFlags = map[specs.LinuxNamespaceType]uintptr{ specs.UTSNamespace: unix.CLONE_NEWUTS, } +var getEUID = os.Geteuid + func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { if spec == nil { return 0, nil @@ -40,13 +42,29 @@ func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { return flags, nil } +func effectiveNamespaceFlags(spec *specs.Spec) (uintptr, error) { + flags, err := namespaceCloneFlags(spec) + if err != nil { + return 0, err + } + + if getEUID() != 0 { + if flags != 0 { + fmt.Fprintln(os.Stderr, "[WARN] running without root: namespaces disabled") + } + return 0, nil + } + + return flags, nil +} + func NewProcess(spec *specs.Spec, devicePath string) (int, error) { execPath, err := os.Executable() if err != nil { return -1, fmt.Errorf("failed to get executable path: %w", err) } - namespaceFlags, err := namespaceCloneFlags(spec) + namespaceFlags, err := effectiveNamespaceFlags(spec) if err != nil { return -1, err } From 3736da6d164c010d95ee99494e76a254f77980a0 Mon Sep 17 00:00:00 2001 From: luke Date: Tue, 7 Oct 2025 17:13:18 +0100 Subject: [PATCH 05/29] Add tests rootless run The options here were either to override the EUID in the code, or require the E2E tests to be run one as sudo, and one as not sudo. The latter was chosen as the changes to the code for tests were somewhat heavy. Add sudo test run to the actions. Signed-off-by: luke --- .github/workflows/ci.yml | 3 +++ docs/DEVELOPMENT.md | 3 +++ e2e/runtime_test.go | 50 ++++++++++++++++++++++++++++++++++++++++ internal/proxy/proxy.go | 2 +- 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf22df94..ae2bd7f5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,3 +78,6 @@ jobs: - name: Run e2e tests run: go test -v ./e2e/... + + - name: Run root namespace E2E test + run: sudo -E env "PATH=$PATH" go test -v -run '^TestRuntimeProxyInheritsCorrectNamespace$' ./e2e diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 1f3fa9ce..b91494de 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -63,6 +63,9 @@ brew install lima # macOS # Run tests go test -v ./e2e/... + +# Run root namespace-specific tests (requires sudo) +sudo -E env "PATH=$PATH" go test -v -run '^TestRuntimeProxyInheritsCorrectNamespace$' ./e2e ``` ### Manual testing with Remoteproc Simulator diff --git a/e2e/runtime_test.go b/e2e/runtime_test.go index d8ca5c50..db235247 100644 --- a/e2e/runtime_test.go +++ b/e2e/runtime_test.go @@ -129,6 +129,10 @@ func TestRuntimeWriteProcessPid(t *testing.T) { } func TestRuntimeProxyInheritsCorrectNamespace(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("requires root privileges to create new namespaces") + } + rootDir := t.TempDir() remoteprocName := "a-lovely-blue-device" sim := remoteproc.NewSimulator(rootDir).WithName(remoteprocName) @@ -167,6 +171,48 @@ func TestRuntimeProxyInheritsCorrectNamespace(t *testing.T) { require.NoError(t, err) } +func TestRuntimeProxyKeepsHostNamespaceWhenNotRoot(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("this test must be run as non-root") + } + rootDir := t.TempDir() + remoteprocName := "a-lovely-blue-device" + sim := remoteproc.NewSimulator(rootDir).WithName(remoteprocName) + if err := sim.Start(); err != nil { + t.Fatalf("failed to run simulator: %s", err) + } + defer func() { _ = sim.Stop() }() + + bin, err := repo.BuildRuntimeBin(t.TempDir(), rootDir, nil) + require.NoError(t, err) + + const containerName = "good-looking-container" + bundlePath := t.TempDir() + + require.NoError(t, generateBundle( + bundlePath, + remoteprocName, + specs.LinuxNamespace{Type: specs.MountNamespace}, + )) + + _, err = invokeRuntime(bin, "create", "--bundle", bundlePath, containerName) + require.NoError(t, err) + defer func() { + _, _ = invokeRuntime(bin, "delete", containerName) + }() + + state, err := getContainerState(bin, containerName) + require.NoError(t, err) + require.Greater(t, state.Pid, 0) + + hostMountNS, err := os.Readlink("/proc/self/ns/mnt") + require.NoError(t, err) + proxyMountNS, err := os.Readlink(fmt.Sprintf("/proc/%d/ns/mnt", state.Pid)) + require.NoError(t, err) + + assert.Equal(t, hostMountNS, proxyMountNS) +} + func assertContainerStatus(t testing.TB, bin repo.RuntimeBin, containerName string, wantStatus specs.ContainerState) { t.Helper() state, err := getContainerState(bin, containerName) @@ -183,6 +229,10 @@ func getContainerPid(bin repo.RuntimeBin, containerName string) (int, error) { } func getContainerState(bin repo.RuntimeBin, containerName string) (specs.State, error) { + return getContainerStateWithEnv(bin, containerName, nil) +} + +func getContainerStateWithEnv(bin repo.RuntimeBin, containerName string, extraEnv []string) (specs.State, error) { var state specs.State out, err := invokeRuntime(bin, "state", containerName) if err != nil { diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index e6bbcf14..2395ad3d 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -50,7 +50,7 @@ func effectiveNamespaceFlags(spec *specs.Spec) (uintptr, error) { if getEUID() != 0 { if flags != 0 { - fmt.Fprintln(os.Stderr, "[WARN] running without root: namespaces disabled") + fmt.Fprintln(os.Stderr, "[WARN] running without root; namespace isolation disabled") } return 0, nil } From aad47dae61c0170793ba4cd080a5886eec52744a Mon Sep 17 00:00:00 2001 From: luke Date: Tue, 7 Oct 2025 18:06:18 +0100 Subject: [PATCH 06/29] lowercase error messages Signed-off-by: luke --- internal/proxy/proxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 2395ad3d..d71bf68f 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -35,7 +35,7 @@ func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { } flag, ok := namespaceFlags[ns.Type] if !ok { - return 0, fmt.Errorf("Unknown namespace type %q", ns.Type) + return 0, fmt.Errorf("unknown namespace type %q", ns.Type) } flags |= flag } From 9649e44606c128f1d89ed2377dda75922182295a Mon Sep 17 00:00:00 2001 From: luke Date: Tue, 7 Oct 2025 18:19:01 +0100 Subject: [PATCH 07/29] Remove unneeded arg and use correct terminology Signed-off-by: luke --- e2e/runtime_test.go | 4 ++-- internal/proxy/proxy.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/e2e/runtime_test.go b/e2e/runtime_test.go index db235247..8e4fe67f 100644 --- a/e2e/runtime_test.go +++ b/e2e/runtime_test.go @@ -229,10 +229,10 @@ func getContainerPid(bin repo.RuntimeBin, containerName string) (int, error) { } func getContainerState(bin repo.RuntimeBin, containerName string) (specs.State, error) { - return getContainerStateWithEnv(bin, containerName, nil) + return getContainerStateWithEnv(bin, containerName) } -func getContainerStateWithEnv(bin repo.RuntimeBin, containerName string, extraEnv []string) (specs.State, error) { +func getContainerStateWithEnv(bin repo.RuntimeBin, containerName string) (specs.State, error) { var state specs.State out, err := invokeRuntime(bin, "state", containerName) if err != nil { diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index d71bf68f..15aef106 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -50,7 +50,7 @@ func effectiveNamespaceFlags(spec *specs.Spec) (uintptr, error) { if getEUID() != 0 { if flags != 0 { - fmt.Fprintln(os.Stderr, "[WARN] running without root; namespace isolation disabled") + fmt.Fprintln(os.Stderr, "[WARN] running non-root; namespace isolation disabled") } return 0, nil } From f46b9d319ff5710ec8039aa1d5446967b932508c Mon Sep 17 00:00:00 2001 From: luke Date: Thu, 9 Oct 2025 10:48:36 +0100 Subject: [PATCH 08/29] Don't use root-requiring tests, add a unit test. Signed-off-by: luke --- .github/workflows/ci.yml | 3 --- docs/DEVELOPMENT.md | 3 --- e2e/runtime_test.go | 47 --------------------------------- internal/proxy/proxy.go | 4 +-- internal/proxy/proxy_test.go | 51 ++++++++++++++++++++++++++++++++++++ 5 files changed, 52 insertions(+), 56 deletions(-) create mode 100644 internal/proxy/proxy_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ae2bd7f5..cf22df94 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,6 +78,3 @@ jobs: - name: Run e2e tests run: go test -v ./e2e/... - - - name: Run root namespace E2E test - run: sudo -E env "PATH=$PATH" go test -v -run '^TestRuntimeProxyInheritsCorrectNamespace$' ./e2e diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index b91494de..1f3fa9ce 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -63,9 +63,6 @@ brew install lima # macOS # Run tests go test -v ./e2e/... - -# Run root namespace-specific tests (requires sudo) -sudo -E env "PATH=$PATH" go test -v -run '^TestRuntimeProxyInheritsCorrectNamespace$' ./e2e ``` ### Manual testing with Remoteproc Simulator diff --git a/e2e/runtime_test.go b/e2e/runtime_test.go index 8e4fe67f..e2a9471d 100644 --- a/e2e/runtime_test.go +++ b/e2e/runtime_test.go @@ -128,49 +128,6 @@ func TestRuntimeWriteProcessPid(t *testing.T) { assertFileContent(t, pidFilePath, fmt.Sprintf("%d", pid)) } -func TestRuntimeProxyInheritsCorrectNamespace(t *testing.T) { - if os.Geteuid() != 0 { - t.Skip("requires root privileges to create new namespaces") - } - - rootDir := t.TempDir() - remoteprocName := "a-lovely-blue-device" - sim := remoteproc.NewSimulator(rootDir).WithName(remoteprocName) - if err := sim.Start(); err != nil { - t.Fatalf("failed to run simulator: %s", err) - } - defer func() { _ = sim.Stop() }() - - bin, err := repo.BuildRuntimeBin(t.TempDir(), rootDir, nil) - require.NoError(t, err) - - const containerName = "blue-container" - bundlePath := t.TempDir() - - require.NoError(t, generateBundle( - bundlePath, - remoteprocName, - specs.LinuxNamespace{Type: specs.MountNamespace}, - )) - - _, err = invokeRuntime(bin, "create", "--bundle", bundlePath, containerName) - require.NoError(t, err) - - pid, err := getContainerPid(bin, containerName) - require.NoError(t, err) - require.Greater(t, pid, 0) - - hostMountNS, err := os.Readlink("/proc/self/ns/mnt") - require.NoError(t, err) - proxyMountNS, err := os.Readlink(fmt.Sprintf("/proc/%d/ns/mnt", pid)) - require.NoError(t, err) - - assert.NotEqual(t, hostMountNS, proxyMountNS) - - _, err = invokeRuntime(bin, "delete", containerName) - require.NoError(t, err) -} - func TestRuntimeProxyKeepsHostNamespaceWhenNotRoot(t *testing.T) { if os.Geteuid() == 0 { t.Skip("this test must be run as non-root") @@ -229,10 +186,6 @@ func getContainerPid(bin repo.RuntimeBin, containerName string) (int, error) { } func getContainerState(bin repo.RuntimeBin, containerName string) (specs.State, error) { - return getContainerStateWithEnv(bin, containerName) -} - -func getContainerStateWithEnv(bin repo.RuntimeBin, containerName string) (specs.State, error) { var state specs.State out, err := invokeRuntime(bin, "state", containerName) if err != nil { diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 15aef106..63974c2b 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -21,8 +21,6 @@ var namespaceFlags = map[specs.LinuxNamespaceType]uintptr{ specs.UTSNamespace: unix.CLONE_NEWUTS, } -var getEUID = os.Geteuid - func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { if spec == nil { return 0, nil @@ -48,7 +46,7 @@ func effectiveNamespaceFlags(spec *specs.Spec) (uintptr, error) { return 0, err } - if getEUID() != 0 { + if os.Geteuid() != 0 { if flags != 0 { fmt.Fprintln(os.Stderr, "[WARN] running non-root; namespace isolation disabled") } diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go new file mode 100644 index 00000000..a04c17d5 --- /dev/null +++ b/internal/proxy/proxy_test.go @@ -0,0 +1,51 @@ +package proxy + +import ( + "testing" + + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" + "golang.org/x/sys/unix" +) + +func TestNamespaceCloneFlags(t *testing.T) { + t.Run("returns zero when spec is nil", func(t *testing.T) { + got, err := namespaceCloneFlags(nil) + + assert.NoError(t, err) + assert.Equal(t, uintptr(0), got) + }) + + t.Run("marks for creation namespaces where paths are not provided", func(t *testing.T) { + spec := &specs.Spec{ + Linux: &specs.Linux{ + Namespaces: []specs.LinuxNamespace{ + {Type: specs.NetworkNamespace}, + {Type: specs.PIDNamespace, Path: "/proc/self/ns/pid"}, + {Type: specs.UTSNamespace}, + }, + }, + } + + got, err := namespaceCloneFlags(spec) + + assert.NoError(t, err) + want := uintptr(unix.CLONE_NEWNET | unix.CLONE_NEWUTS) + assert.Equal(t, want, got) + }) + + t.Run("errors with unknown namespace types", func(t *testing.T) { + spec := &specs.Spec{ + Linux: &specs.Linux{ + Namespaces: []specs.LinuxNamespace{ + {Type: specs.LinuxNamespaceType("unknown")}, + }, + }, + } + + _, err := namespaceCloneFlags(spec) + + assert.Error(t, err) + assert.Equal(t, "unknown namespace type \"unknown\"", err.Error()) + }) +} From 9c568d3734d6ce35447a52c2de82c6d580440101 Mon Sep 17 00:00:00 2001 From: luke Date: Thu, 9 Oct 2025 11:10:30 +0100 Subject: [PATCH 09/29] Unit testing Signed-off-by: luke --- internal/proxy/proxy.go | 11 ++++--- internal/proxy/proxy_test.go | 59 ++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 63974c2b..34a3c762 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -21,6 +21,8 @@ var namespaceFlags = map[specs.LinuxNamespaceType]uintptr{ specs.UTSNamespace: unix.CLONE_NEWUTS, } +var namespaceCloneFlagsFn = namespaceCloneFlags + func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { if spec == nil { return 0, nil @@ -40,13 +42,13 @@ func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { return flags, nil } -func effectiveNamespaceFlags(spec *specs.Spec) (uintptr, error) { - flags, err := namespaceCloneFlags(spec) +func effectiveNamespaceFlags(isRoot bool, spec *specs.Spec) (uintptr, error) { + flags, err := namespaceCloneFlagsFn(spec) if err != nil { return 0, err } - if os.Geteuid() != 0 { + if !isRoot { if flags != 0 { fmt.Fprintln(os.Stderr, "[WARN] running non-root; namespace isolation disabled") } @@ -62,7 +64,8 @@ func NewProcess(spec *specs.Spec, devicePath string) (int, error) { return -1, fmt.Errorf("failed to get executable path: %w", err) } - namespaceFlags, err := effectiveNamespaceFlags(spec) + isRoot := os.Geteuid() == 0 + namespaceFlags, err := effectiveNamespaceFlags(isRoot, spec) if err != nil { return -1, err } diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index a04c17d5..04904661 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -1,6 +1,9 @@ package proxy import ( + "errors" + "io" + "os" "testing" "github.com/opencontainers/runtime-spec/specs-go" @@ -8,6 +11,15 @@ import ( "golang.org/x/sys/unix" ) +func stubNamespaceCloneFlags(t *testing.T, fn func(*specs.Spec) (uintptr, error)) { + t.Helper() + original := namespaceCloneFlagsFn + namespaceCloneFlagsFn = fn + t.Cleanup(func() { + namespaceCloneFlagsFn = original + }) +} + func TestNamespaceCloneFlags(t *testing.T) { t.Run("returns zero when spec is nil", func(t *testing.T) { got, err := namespaceCloneFlags(nil) @@ -49,3 +61,50 @@ func TestNamespaceCloneFlags(t *testing.T) { assert.Equal(t, "unknown namespace type \"unknown\"", err.Error()) }) } + +func TestEffectiveNamespaceFlags(t *testing.T) { + t.Run("returns clone flags when running as root", func(t *testing.T) { + stubNamespaceCloneFlags(t, func(spec *specs.Spec) (uintptr, error) { + return 0x1507, nil + }) + + got, err := effectiveNamespaceFlags(true, &specs.Spec{}) + + assert.NoError(t, err) + assert.Equal(t, uintptr(0x1234), got) + }) + + t.Run("warns and does not set namespace if root is not set but flags are given", func(t *testing.T) { + stubNamespaceCloneFlags(t, func(spec *specs.Spec) (uintptr, error) { + return 0x2, nil + }) + + stderr := os.Stderr + reader, writer, err := os.Pipe() + assert.NoError(t, err) + os.Stderr = writer + t.Cleanup(func() { + os.Stderr = stderr + }) + + got, err := effectiveNamespaceFlags(false, &specs.Spec{}) + assert.NoError(t, err) + assert.Equal(t, uintptr(0), got) + + assert.NoError(t, writer.Close()) + out, readErr := io.ReadAll(reader) + assert.NoError(t, readErr) + assert.Contains(t, string(out), "[WARN] running non-root; namespace isolation disabled") + assert.NoError(t, reader.Close()) + }) + + t.Run("propagates namespaceCloneFlags errors", func(t *testing.T) { + expected := errors.New("error!") + stubNamespaceCloneFlags(t, func(spec *specs.Spec) (uintptr, error) { + return 0, expected + }) + + _, err := effectiveNamespaceFlags(true, &specs.Spec{}) + assert.ErrorIs(t, err, expected) + }) +} From bb81f30405e3def8c213c29e93cbbb0866f9ef1e Mon Sep 17 00:00:00 2001 From: luke Date: Thu, 9 Oct 2025 11:16:06 +0100 Subject: [PATCH 10/29] oops Signed-off-by: luke --- internal/proxy/proxy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index 04904661..96f76906 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -71,7 +71,7 @@ func TestEffectiveNamespaceFlags(t *testing.T) { got, err := effectiveNamespaceFlags(true, &specs.Spec{}) assert.NoError(t, err) - assert.Equal(t, uintptr(0x1234), got) + assert.Equal(t, uintptr(0x1507), got) }) t.Run("warns and does not set namespace if root is not set but flags are given", func(t *testing.T) { From b1cc94fdbe496b69e757a127b892ee223ab9158e Mon Sep 17 00:00:00 2001 From: luke Date: Mon, 20 Oct 2025 12:49:53 +0100 Subject: [PATCH 11/29] Rework tests, improve logging Signed-off-by: luke --- cmd/remoteproc-runtime/create.go | 2 +- docs/DEVELOPMENT.md | 3 +- e2e/runtime_test.go | 42 ---------- e2e/runtime_via_lima_test.go | 127 ++++++++++++++++++++++++++++++ internal/proxy/namespaces.go | 61 ++++++++++++++ internal/proxy/namespaces_test.go | 54 +++++++++++++ internal/proxy/proxy.go | 55 +------------ internal/proxy/proxy_test.go | 110 -------------------------- internal/runtime/create.go | 10 ++- 9 files changed, 256 insertions(+), 208 deletions(-) create mode 100644 e2e/runtime_via_lima_test.go create mode 100644 internal/proxy/namespaces.go create mode 100644 internal/proxy/namespaces_test.go delete mode 100644 internal/proxy/proxy_test.go diff --git a/cmd/remoteproc-runtime/create.go b/cmd/remoteproc-runtime/create.go index 2b283bf3..1a4ab409 100644 --- a/cmd/remoteproc-runtime/create.go +++ b/cmd/remoteproc-runtime/create.go @@ -20,7 +20,7 @@ var createCmd = &cobra.Command{ if bundlePath == "" { bundlePath = "." } - return runtime.Create(containerID, bundlePath, pidFile) + return runtime.Create(logger, containerID, bundlePath, pidFile) }, } diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 1f3fa9ce..a02f9786 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -76,7 +76,7 @@ Useful for development without access to hardware with Remoteproc support. #### Testing with Docker -⚠️ Docker network must be set to 'Host' (`--network=host`), as the proxy runs in the host's network namespace. +⚠️ If using docker rootless, the network must be set to 'Host' (`--network=host`), as the proxy runs in the host's network namespace. 1. **Build and install the shim and runtime with custom root** @@ -114,7 +114,6 @@ Useful for development without access to hardware with Remoteproc support. docker run \ --runtime io.containerd.remoteproc.v1 \ --annotation remoteproc.name="test-processor" \ - --network=host \ test-remoteproc-image ``` diff --git a/e2e/runtime_test.go b/e2e/runtime_test.go index e2a9471d..88271c02 100644 --- a/e2e/runtime_test.go +++ b/e2e/runtime_test.go @@ -128,48 +128,6 @@ func TestRuntimeWriteProcessPid(t *testing.T) { assertFileContent(t, pidFilePath, fmt.Sprintf("%d", pid)) } -func TestRuntimeProxyKeepsHostNamespaceWhenNotRoot(t *testing.T) { - if os.Geteuid() == 0 { - t.Skip("this test must be run as non-root") - } - rootDir := t.TempDir() - remoteprocName := "a-lovely-blue-device" - sim := remoteproc.NewSimulator(rootDir).WithName(remoteprocName) - if err := sim.Start(); err != nil { - t.Fatalf("failed to run simulator: %s", err) - } - defer func() { _ = sim.Stop() }() - - bin, err := repo.BuildRuntimeBin(t.TempDir(), rootDir, nil) - require.NoError(t, err) - - const containerName = "good-looking-container" - bundlePath := t.TempDir() - - require.NoError(t, generateBundle( - bundlePath, - remoteprocName, - specs.LinuxNamespace{Type: specs.MountNamespace}, - )) - - _, err = invokeRuntime(bin, "create", "--bundle", bundlePath, containerName) - require.NoError(t, err) - defer func() { - _, _ = invokeRuntime(bin, "delete", containerName) - }() - - state, err := getContainerState(bin, containerName) - require.NoError(t, err) - require.Greater(t, state.Pid, 0) - - hostMountNS, err := os.Readlink("/proc/self/ns/mnt") - require.NoError(t, err) - proxyMountNS, err := os.Readlink(fmt.Sprintf("/proc/%d/ns/mnt", state.Pid)) - require.NoError(t, err) - - assert.Equal(t, hostMountNS, proxyMountNS) -} - func assertContainerStatus(t testing.TB, bin repo.RuntimeBin, containerName string, wantStatus specs.ContainerState) { t.Helper() state, err := getContainerState(bin, containerName) diff --git a/e2e/runtime_via_lima_test.go b/e2e/runtime_via_lima_test.go new file mode 100644 index 00000000..172592da --- /dev/null +++ b/e2e/runtime_via_lima_test.go @@ -0,0 +1,127 @@ +package e2e + +import ( + "encoding/json" + "fmt" + "path/filepath" + "strings" + "testing" + + "github.com/arm/remoteproc-runtime/e2e/limavm" + "github.com/arm/remoteproc-runtime/e2e/remoteproc" + "github.com/arm/remoteproc-runtime/e2e/repo" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRuntimeProxyKeepsHostNamespaceWhenNotRootInLimaVM(t *testing.T) { + rootDir := t.TempDir() + remoteprocName := "a-lovely-blue-device" + + runtimeBin, err := repo.BuildRuntimeBin(t.TempDir(), rootDir, limavm.BinBuildEnv) + require.NoError(t, err) + + vm, err := limavm.NewWithPodman(rootDir, "../testdata", runtimeBin) + require.NoError(t, err) + t.Cleanup(vm.Cleanup) + + sim := remoteproc.NewSimulator(rootDir).WithName(remoteprocName) + if err := sim.Start(); err != nil { + t.Fatalf("failed to run simulator: %s", err) + } + t.Cleanup(func() { _ = sim.Stop() }) + + const containerName = "not-root-namespace-container" + bundlePath := filepath.Join(rootDir, "not-root-namespace-bundle") + require.NoError(t, generateBundle( + bundlePath, + remoteprocName, + specs.LinuxNamespace{Type: specs.MountNamespace}, + )) + + _, stderr, err := vm.RunCommand( + "remoteproc-runtime", + "create", "--bundle", bundlePath, + containerName, + ) + require.NoError(t, err, "stderr: %s", stderr) + t.Cleanup(func() { + _, _, _ = vm.RunCommand("remoteproc-runtime", "delete", containerName) + }) + + stdout, stderr, err := vm.RunCommand("remoteproc-runtime", "state", containerName) + require.NoError(t, err, "stderr: %s", stderr) + + var state specs.State + require.NoError(t, json.Unmarshal([]byte(stdout), &state)) + require.Greater(t, state.Pid, 0) + + hostMountNS, stderr, err := vm.RunCommand("readlink", "/proc/self/ns/mnt") + require.NoError(t, err, "stderr: %s", stderr) + + proxyMountNS, stderr, err := vm.RunCommand("readlink", fmt.Sprintf("/proc/%d/ns/mnt", state.Pid)) + require.NoError(t, err, "stderr: %s", stderr) + + assert.Equal(t, strings.TrimSpace(hostMountNS), strings.TrimSpace(proxyMountNS)) +} + +func TestRuntimeProxyKeepsHostNamespaceWhenRootInLimaVM(t *testing.T) { + rootDir := t.TempDir() + remoteprocName := "a-lovely-blue-device" + + runtimeBin, err := repo.BuildRuntimeBin(t.TempDir(), rootDir, limavm.BinBuildEnv) + require.NoError(t, err) + + vm, err := limavm.NewWithPodman(rootDir, "../testdata", runtimeBin) + require.NoError(t, err) + t.Cleanup(vm.Cleanup) + + sim := remoteproc.NewSimulator(rootDir).WithName(remoteprocName) + if err := sim.Start(); err != nil { + t.Fatalf("failed to run simulator: %s", err) + } + t.Cleanup(func() { _ = sim.Stop() }) + + const containerName = "root-namespace-container" + bundlePath := filepath.Join(rootDir, "root-namespace-bundle") + require.NoError(t, generateBundle( + bundlePath, + remoteprocName, + specs.LinuxNamespace{Type: specs.MountNamespace}, + )) + + _, stderr, err := vm.RunCommand( + "sudo", "remoteproc-runtime", + "create", "--bundle", bundlePath, + containerName, + ) + require.NoError(t, err, "stderr: %s", stderr) + t.Cleanup(func() { + _, _, _ = vm.RunCommand("sudo", "remoteproc-runtime", "delete", containerName) + }) + + stdout, stderr, err := vm.RunCommand("sudo", "remoteproc-runtime", "state", containerName) + require.NoError(t, err, "stderr: %s", stderr) + + var state specs.State + require.NoError(t, json.Unmarshal([]byte(stdout), &state)) + require.Greater(t, state.Pid, 0) + + hostMountNS, stderr, err := vm.RunCommand("readlink", "/proc/self/ns/mnt") + require.NoError(t, err, "stderr: %s", stderr) + + proxyMountNS, stderr, err := vm.RunCommand("sudo", "readlink", fmt.Sprintf("/proc/%d/ns/mnt", state.Pid)) + require.NoError(t, err, "stderr: %s", stderr) + assert.NotEqual(t, strings.TrimSpace(hostMountNS), strings.TrimSpace(proxyMountNS)) + + remoteproc.AssertState(t, sim.DeviceDir(), "offline") + + _, stderr, err = vm.RunCommand("sudo", "remoteproc-runtime", "start", containerName) + require.NoError(t, err, "stderr: %s", stderr) + remoteproc.AssertState(t, sim.DeviceDir(), "running") + + _, stderr, err = vm.RunCommand("sudo", "remoteproc-runtime", "kill", containerName) + require.NoError(t, err, "stderr: %s", stderr) + remoteproc.AssertState(t, sim.DeviceDir(), "offline") +} diff --git a/internal/proxy/namespaces.go b/internal/proxy/namespaces.go new file mode 100644 index 00000000..22c83965 --- /dev/null +++ b/internal/proxy/namespaces.go @@ -0,0 +1,61 @@ +package proxy + +import ( + "fmt" + "log/slog" + + "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/sys/unix" +) + +var specNamespacesToUnixCloneFlags = map[specs.LinuxNamespaceType]uintptr{ + specs.CgroupNamespace: unix.CLONE_NEWCGROUP, + specs.IPCNamespace: unix.CLONE_NEWIPC, + specs.MountNamespace: unix.CLONE_NEWNS, + specs.NetworkNamespace: unix.CLONE_NEWNET, + specs.PIDNamespace: unix.CLONE_NEWPID, + specs.TimeNamespace: unix.CLONE_NEWTIME, + specs.UserNamespace: unix.CLONE_NEWUSER, + specs.UTSNamespace: unix.CLONE_NEWUTS, +} + +func ParseNamespaceFlags(logger *slog.Logger, namespaces []specs.LinuxNamespace) (uintptr, error) { + if namespaces == nil { + return 0, nil + } + + var flags uintptr + for _, ns := range namespaces { + if ns.Path != "" { + continue + } + flag, ok := specNamespacesToUnixCloneFlags[ns.Type] + if !ok { + err := fmt.Errorf("unknown namespace type %q", ns.Type) + if logger != nil { + logger.Error("unknown namespace type", "type", ns.Type) + } + return 0, err + } + flags |= flag + } + return flags, nil +} + +func LinuxCloneFlags(logger *slog.Logger, isRoot bool, namespaces []specs.LinuxNamespace) (uintptr, error) { + flags, err := ParseNamespaceFlags(logger, namespaces) + if err != nil { + return 0, err + } + + if !isRoot { + if flags != 0 { + if logger != nil { + logger.Warn("running non-root; namespace isolation disabled") + } + } + return 0, nil + } + + return flags, nil +} diff --git a/internal/proxy/namespaces_test.go b/internal/proxy/namespaces_test.go new file mode 100644 index 00000000..e60e260f --- /dev/null +++ b/internal/proxy/namespaces_test.go @@ -0,0 +1,54 @@ +package proxy_test + +import ( + "io" + "log/slog" + "testing" + + proxy "github.com/arm/remoteproc-runtime/internal/proxy" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" +) + +func TestLinuxCloneFlags(t *testing.T) { + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + t.Run("converts known linux namespace flags to unix", func(t *testing.T) { + isRoot := true + namespaces := []specs.LinuxNamespace{ + {Type: specs.CgroupNamespace}, + {Type: specs.UserNamespace}, + } + + got, err := proxy.LinuxCloneFlags(logger, isRoot, namespaces) + + require.NoError(t, err) + want := uintptr(unix.CLONE_NEWCGROUP | unix.CLONE_NEWUSER) + require.Equal(t, want, got) + }) + + t.Run("non-root disables cloning", func(t *testing.T) { + isRoot := false + namespaces := []specs.LinuxNamespace{ + {Type: specs.CgroupNamespace}, + {Type: specs.UserNamespace}, + } + + got, err := proxy.LinuxCloneFlags(logger, isRoot, namespaces) + + require.NoError(t, err) + require.Equal(t, uintptr(0), got) + }) + + t.Run("errors given unknown namespace", func(t *testing.T) { + isRoot := true + namespaces := []specs.LinuxNamespace{ + {Type: specs.LinuxNamespaceType("weird-name")}, + } + + _, err := proxy.LinuxCloneFlags(logger, isRoot, namespaces) + + require.Error(t, err) + }) +} diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 34a3c762..c07b72b6 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -2,70 +2,23 @@ package proxy import ( "fmt" + "log/slog" "os" "os/exec" "syscall" "github.com/opencontainers/runtime-spec/specs-go" - "golang.org/x/sys/unix" ) -var namespaceFlags = map[specs.LinuxNamespaceType]uintptr{ - specs.CgroupNamespace: unix.CLONE_NEWCGROUP, - specs.IPCNamespace: unix.CLONE_NEWIPC, - specs.MountNamespace: unix.CLONE_NEWNS, - specs.NetworkNamespace: unix.CLONE_NEWNET, - specs.PIDNamespace: unix.CLONE_NEWPID, - specs.TimeNamespace: unix.CLONE_NEWTIME, - specs.UserNamespace: unix.CLONE_NEWUSER, - specs.UTSNamespace: unix.CLONE_NEWUTS, -} - -var namespaceCloneFlagsFn = namespaceCloneFlags - -func namespaceCloneFlags(spec *specs.Spec) (uintptr, error) { - if spec == nil { - return 0, nil - } - - var flags uintptr - for _, ns := range spec.Linux.Namespaces { - if ns.Path != "" { - continue - } - flag, ok := namespaceFlags[ns.Type] - if !ok { - return 0, fmt.Errorf("unknown namespace type %q", ns.Type) - } - flags |= flag - } - return flags, nil -} - -func effectiveNamespaceFlags(isRoot bool, spec *specs.Spec) (uintptr, error) { - flags, err := namespaceCloneFlagsFn(spec) - if err != nil { - return 0, err - } - - if !isRoot { - if flags != 0 { - fmt.Fprintln(os.Stderr, "[WARN] running non-root; namespace isolation disabled") - } - return 0, nil - } - - return flags, nil -} - -func NewProcess(spec *specs.Spec, devicePath string) (int, error) { +func NewProcess(logger *slog.Logger, namespaces []specs.LinuxNamespace, devicePath string) (int, error) { execPath, err := os.Executable() if err != nil { return -1, fmt.Errorf("failed to get executable path: %w", err) } isRoot := os.Geteuid() == 0 - namespaceFlags, err := effectiveNamespaceFlags(isRoot, spec) + + namespaceFlags, err := LinuxCloneFlags(logger, isRoot, namespaces) if err != nil { return -1, err } diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go deleted file mode 100644 index 96f76906..00000000 --- a/internal/proxy/proxy_test.go +++ /dev/null @@ -1,110 +0,0 @@ -package proxy - -import ( - "errors" - "io" - "os" - "testing" - - "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" - "golang.org/x/sys/unix" -) - -func stubNamespaceCloneFlags(t *testing.T, fn func(*specs.Spec) (uintptr, error)) { - t.Helper() - original := namespaceCloneFlagsFn - namespaceCloneFlagsFn = fn - t.Cleanup(func() { - namespaceCloneFlagsFn = original - }) -} - -func TestNamespaceCloneFlags(t *testing.T) { - t.Run("returns zero when spec is nil", func(t *testing.T) { - got, err := namespaceCloneFlags(nil) - - assert.NoError(t, err) - assert.Equal(t, uintptr(0), got) - }) - - t.Run("marks for creation namespaces where paths are not provided", func(t *testing.T) { - spec := &specs.Spec{ - Linux: &specs.Linux{ - Namespaces: []specs.LinuxNamespace{ - {Type: specs.NetworkNamespace}, - {Type: specs.PIDNamespace, Path: "/proc/self/ns/pid"}, - {Type: specs.UTSNamespace}, - }, - }, - } - - got, err := namespaceCloneFlags(spec) - - assert.NoError(t, err) - want := uintptr(unix.CLONE_NEWNET | unix.CLONE_NEWUTS) - assert.Equal(t, want, got) - }) - - t.Run("errors with unknown namespace types", func(t *testing.T) { - spec := &specs.Spec{ - Linux: &specs.Linux{ - Namespaces: []specs.LinuxNamespace{ - {Type: specs.LinuxNamespaceType("unknown")}, - }, - }, - } - - _, err := namespaceCloneFlags(spec) - - assert.Error(t, err) - assert.Equal(t, "unknown namespace type \"unknown\"", err.Error()) - }) -} - -func TestEffectiveNamespaceFlags(t *testing.T) { - t.Run("returns clone flags when running as root", func(t *testing.T) { - stubNamespaceCloneFlags(t, func(spec *specs.Spec) (uintptr, error) { - return 0x1507, nil - }) - - got, err := effectiveNamespaceFlags(true, &specs.Spec{}) - - assert.NoError(t, err) - assert.Equal(t, uintptr(0x1507), got) - }) - - t.Run("warns and does not set namespace if root is not set but flags are given", func(t *testing.T) { - stubNamespaceCloneFlags(t, func(spec *specs.Spec) (uintptr, error) { - return 0x2, nil - }) - - stderr := os.Stderr - reader, writer, err := os.Pipe() - assert.NoError(t, err) - os.Stderr = writer - t.Cleanup(func() { - os.Stderr = stderr - }) - - got, err := effectiveNamespaceFlags(false, &specs.Spec{}) - assert.NoError(t, err) - assert.Equal(t, uintptr(0), got) - - assert.NoError(t, writer.Close()) - out, readErr := io.ReadAll(reader) - assert.NoError(t, readErr) - assert.Contains(t, string(out), "[WARN] running non-root; namespace isolation disabled") - assert.NoError(t, reader.Close()) - }) - - t.Run("propagates namespaceCloneFlags errors", func(t *testing.T) { - expected := errors.New("error!") - stubNamespaceCloneFlags(t, func(spec *specs.Spec) (uintptr, error) { - return 0, expected - }) - - _, err := effectiveNamespaceFlags(true, &specs.Spec{}) - assert.ErrorIs(t, err, expected) - }) -} diff --git a/internal/runtime/create.go b/internal/runtime/create.go index 1cddfb56..ef9bedd5 100644 --- a/internal/runtime/create.go +++ b/internal/runtime/create.go @@ -2,6 +2,7 @@ package runtime import ( "fmt" + "log/slog" "os" "path/filepath" @@ -11,7 +12,7 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" ) -func Create(containerID string, bundlePath string, pidFile string) error { +func Create(logger *slog.Logger, containerID string, bundlePath string, pidFile string) error { spec, err := oci.ReadSpec(bundlePath) if err != nil { return fmt.Errorf("failed to read container specification: %w", err) @@ -46,7 +47,12 @@ func Create(containerID string, bundlePath string, pidFile string) error { } }() - pid, err := proxy.NewProcess(spec, devicePath) + if spec.Linux == nil { + return fmt.Errorf("linux container information missing") + } + namespaces := spec.Linux.Namespaces + + pid, err := proxy.NewProcess(logger, namespaces, devicePath) if err != nil { return fmt.Errorf("failed to start proxy process: %w", err) } From 6fb19771750a9453ff59f8ef69c54f0dfccce5ef Mon Sep 17 00:00:00 2001 From: luke Date: Mon, 20 Oct 2025 14:25:34 +0100 Subject: [PATCH 12/29] Address review comments, allow nil linux namespace Signed-off-by: luke --- docs/DEVELOPMENT.md | 2 -- e2e/runtime_via_lima_test.go | 2 +- internal/proxy/namespaces.go | 15 +++++---------- internal/runtime/create.go | 6 +++--- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index a02f9786..0edf4d1d 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -76,8 +76,6 @@ Useful for development without access to hardware with Remoteproc support. #### Testing with Docker -⚠️ If using docker rootless, the network must be set to 'Host' (`--network=host`), as the proxy runs in the host's network namespace. - 1. **Build and install the shim and runtime with custom root** ```bash diff --git a/e2e/runtime_via_lima_test.go b/e2e/runtime_via_lima_test.go index 172592da..b63f4f47 100644 --- a/e2e/runtime_via_lima_test.go +++ b/e2e/runtime_via_lima_test.go @@ -15,7 +15,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestRuntimeProxyKeepsHostNamespaceWhenNotRootInLimaVM(t *testing.T) { +func TestRuntimeProxyKeepsHostNamespaceWhenNotRoot(t *testing.T) { rootDir := t.TempDir() remoteprocName := "a-lovely-blue-device" diff --git a/internal/proxy/namespaces.go b/internal/proxy/namespaces.go index 22c83965..f9e253eb 100644 --- a/internal/proxy/namespaces.go +++ b/internal/proxy/namespaces.go @@ -19,7 +19,7 @@ var specNamespacesToUnixCloneFlags = map[specs.LinuxNamespaceType]uintptr{ specs.UTSNamespace: unix.CLONE_NEWUTS, } -func ParseNamespaceFlags(logger *slog.Logger, namespaces []specs.LinuxNamespace) (uintptr, error) { +func ParseNamespaceFlags(namespaces []specs.LinuxNamespace) (uintptr, error) { if namespaces == nil { return 0, nil } @@ -32,9 +32,6 @@ func ParseNamespaceFlags(logger *slog.Logger, namespaces []specs.LinuxNamespace) flag, ok := specNamespacesToUnixCloneFlags[ns.Type] if !ok { err := fmt.Errorf("unknown namespace type %q", ns.Type) - if logger != nil { - logger.Error("unknown namespace type", "type", ns.Type) - } return 0, err } flags |= flag @@ -43,16 +40,14 @@ func ParseNamespaceFlags(logger *slog.Logger, namespaces []specs.LinuxNamespace) } func LinuxCloneFlags(logger *slog.Logger, isRoot bool, namespaces []specs.LinuxNamespace) (uintptr, error) { - flags, err := ParseNamespaceFlags(logger, namespaces) + flags, err := ParseNamespaceFlags(namespaces) if err != nil { return 0, err } - if !isRoot { - if flags != 0 { - if logger != nil { - logger.Warn("running non-root; namespace isolation disabled") - } + if !isRoot && flags != 0 { + if logger != nil { + logger.Warn("running non-root; namespace isolation disabled") } return 0, nil } diff --git a/internal/runtime/create.go b/internal/runtime/create.go index ef9bedd5..590b3647 100644 --- a/internal/runtime/create.go +++ b/internal/runtime/create.go @@ -47,10 +47,10 @@ func Create(logger *slog.Logger, containerID string, bundlePath string, pidFile } }() - if spec.Linux == nil { - return fmt.Errorf("linux container information missing") + var namespaces []specs.LinuxNamespace + if spec.Linux != nil { + namespaces = spec.Linux.Namespaces } - namespaces := spec.Linux.Namespaces pid, err := proxy.NewProcess(logger, namespaces, devicePath) if err != nil { From 8f01ce83b374575f14ffba62d43bd7d0e62f00ec Mon Sep 17 00:00:00 2001 From: luke Date: Mon, 20 Oct 2025 14:42:13 +0100 Subject: [PATCH 13/29] Ensure logger is added, add alpine img for tests. move tests into central file Signed-off-by: luke --- e2e/limavm/limavm.go | 4 + e2e/limavm/prepare-lima-vm.sh | 20 +++-- e2e/runtime_test.go | 109 +++++++++++++++++++++++++ e2e/runtime_via_lima_test.go | 127 ------------------------------ internal/proxy/namespaces_test.go | 8 +- 5 files changed, 134 insertions(+), 134 deletions(-) delete mode 100644 e2e/runtime_via_lima_test.go diff --git a/e2e/limavm/limavm.go b/e2e/limavm/limavm.go index 71180209..c7847ccc 100644 --- a/e2e/limavm/limavm.go +++ b/e2e/limavm/limavm.go @@ -32,6 +32,10 @@ func NewWithPodman(mountDir string, buildContext string, runtimeBin repo.Runtime return New("podman", mountDir, buildContext, string(runtimeBin)) } +func NewAlpine(mountDir string, runtimeBin repo.RuntimeBin) (LimaVM, error) { + return New("alpine", mountDir, "", string(runtimeBin)) +} + func New(template string, mountDir string, buildContext string, binsToInstall ...string) (LimaVM, error) { cmd := exec.Command( prepareLimaVMScript, diff --git a/e2e/limavm/prepare-lima-vm.sh b/e2e/limavm/prepare-lima-vm.sh index dddc2d78..50c584f5 100755 --- a/e2e/limavm/prepare-lima-vm.sh +++ b/e2e/limavm/prepare-lima-vm.sh @@ -11,7 +11,7 @@ BINARIES=() usage() { echo "Usage: $0