-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Add namespace isolation support for proxy processes #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
b56d626
c8f0026
670e0a4
d57170c
3736da6
aad47da
9649e44
f46b9d3
9c568d3
bb81f30
e6e1e51
b1cc94f
6fb1977
8f01ce8
f534a7f
827a7ac
654d472
34c5ff1
a4bf97d
bee0dcf
6663003
959ee9e
4b743a2
e0838bb
b39e1ea
3800ba8
379ef50
2974a96
913f822
f7bb9d7
3429464
fe5a97d
f96f1ad
a31f40c
5d5b162
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,6 +128,48 @@ 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") | ||
| } | ||
|
Luke-Parkin marked this conversation as resolved.
Outdated
|
||
| 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? Is this a pre-flight check?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I put this in as just a safeguard; in the case the runtime returned some clearly wrong value we wouldn't continue testing against that PID. Possibly not needed though, doesn't really hurt much?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair. Those e2e tests are a getting chunky, so I'm trying to keep context to minimum and describe as much as possible why/what/where. I wonder if this would improve the readability and maintain small context: pid, err := getRunningContainerPid(bin, containerName)And then func getRunningContainerPid() (uint, err) {
state, err := getContainerState()
if state.Pid == 0 {
return 0, fmt.Println("container doesn't seem to be running - pid is 0")
}
return state.Pid, nil
} |
||
|
|
||
| hostMountNS, err := os.Readlink("/proc/self/ns/mnt") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The go test process is launching the runtime. The proxy should have inherited the namespace from the go test proxy. This is the same flow as a user initiating the proxy (give or take) |
||
| 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) | ||
|
|
@@ -156,7 +198,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 +221,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") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| package proxy | ||
|
muchzill4 marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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) { | ||
|
muchzill4 marked this conversation as resolved.
Outdated
|
||
| 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) | ||
| }) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.