Merge pull request #212 from edytuk/sylabs1157

feat: oci: support namespace flags, from sylabs 1157

e2e/actions/actions.go

@@ -2559,8 +2559,9 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests {
 		//
 		// OCI Runtime Mode
 		//
-		"ociRun":   c.actionOciRun,   // apptainer run --oci
-		"ociExec":  c.actionOciExec,  // apptainer exec --oci
-		"ociShell": c.actionOciShell, // apptainer shell --oci
+		"ociRun":     c.actionOciRun,     // apptainer run --oci
+		"ociExec":    c.actionOciExec,    // apptainer exec --oci
+		"ociShell":   c.actionOciShell,   // apptainer shell --oci
+		"ociNetwork": c.actionOciNetwork, // apptainer exec --oci --net
 	}
 }

e2e/actions/oci.go

@@ -147,6 +147,11 @@ func (c actionTests) actionOciExec(t *testing.T) {
 			argv: []string{imageRef, "/bin/sh", "-c", "touch $HOME"},
 			exit: 0,
 		},
+		{
+			name: "UTSNamespace",
+			argv: []string{"--uts", imageRef, "true"},
+			exit: 0,
+		},
 	}
 	for _, profile := range e2e.OCIProfiles {
 		t.Run(profile.String(), func(t *testing.T) {
@@ -218,3 +223,63 @@ func (c actionTests) actionOciShell(t *testing.T) {
 		})
 	}
 }
+
+func (c actionTests) actionOciNetwork(t *testing.T) {
+	e2e.EnsureOCIImage(t, c.env)
+	imageRef := "oci-archive:" + c.env.OCIImagePath
+
+	tests := []struct {
+		name       string
+		profile    e2e.Profile
+		netType    string
+		expectExit int
+	}{
+		{
+			name:       "InvalidNetworkRoot",
+			profile:    e2e.OCIRootProfile,
+			netType:    "bridge",
+			expectExit: 255,
+		},
+		{
+			name:       "InvalidNetworkUser",
+			profile:    e2e.OCIUserProfile,
+			netType:    "bridge",
+			expectExit: 255,
+		},
+		{
+			name:       "InvalidNetworkFakeroot",
+			profile:    e2e.OCIFakerootProfile,
+			netType:    "bridge",
+			expectExit: 255,
+		},
+		{
+			name:       "NoneNetworkRoot",
+			profile:    e2e.OCIRootProfile,
+			netType:    "none",
+			expectExit: 0,
+		},
+		{
+			name:       "NoneNetworkUser",
+			profile:    e2e.OCIUserProfile,
+			netType:    "none",
+			expectExit: 0,
+		},
+		{
+			name:       "NoneNetworkFakeRoot",
+			profile:    e2e.OCIFakerootProfile,
+			netType:    "none",
+			expectExit: 0,
+		},
+	}
+
+	for _, tt := range tests {
+		c.env.RunApptainer(
+			t,
+			e2e.AsSubtest(tt.name),
+			e2e.WithProfile(tt.profile),
+			e2e.WithCommand("exec"),
+			e2e.WithArgs("--net", "--network", tt.netType, imageRef, "id"),
+			e2e.ExpectExit(tt.expectExit),
+		)
+	}
+}

internal/pkg/runtime/launcher/oci/launcher_linux.go

@@ -19,6 +19,7 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"syscall"
 
 	"github.com/apptainer/apptainer/internal/pkg/buildcfg"
 	"github.com/apptainer/apptainer/internal/pkg/cache"
@@ -30,6 +31,8 @@ import (
 	useragent "github.com/apptainer/apptainer/pkg/util/user-agent"
 	"github.com/containers/image/v5/types"
 	"github.com/google/uuid"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"golang.org/x/term"
 )
 
 var (
@@ -143,25 +146,10 @@ func checkOpts(lo launcher.Options) error {
 		badOpt = append(badOpt, "NoEval")
 	}
 
-	if lo.Namespaces.IPC {
-		badOpt = append(badOpt, "Namespaces.IPC")
-	}
-	if lo.Namespaces.Net {
-		badOpt = append(badOpt, "Namespaces.Net")
-	}
-	if lo.Namespaces.PID {
-		badOpt = append(badOpt, "Namespaces.PID")
-	}
-	if lo.Namespaces.UTS {
-		badOpt = append(badOpt, "Namespaces.UTS")
-	}
-	if lo.Namespaces.User {
-		badOpt = append(badOpt, "Namespaces.User")
-	}
-
 	// Network always set in CLI layer even if network namespace not requested.
-	if lo.Namespaces.Net && lo.Network != "" {
-		badOpt = append(badOpt, "Network")
+	// We only support isolation at present
+	if lo.Namespaces.Net && lo.Network != "none" {
+		badOpt = append(badOpt, "Network (except none)")
 	}
 
 	if len(lo.NetworkArgs) > 0 {
@@ -247,6 +235,47 @@ func checkOpts(lo launcher.Options) error {
 	return nil
 }
 
+// createSpec produces an OCI runtime specification, suitable to launch a
+// container. This spec excludes ProcessArgs, as these have to be computed where
+// the image config is available, to account for the image's CMD / ENTRYPOINT.
+func (l *Launcher) createSpec() (*specs.Spec, error) {
+	spec := minimalSpec()
+
+	// Override the default Process.Terminal to false if our stdin is not a terminal.
+	if !term.IsTerminal(syscall.Stdin) {
+		spec.Process.Terminal = false
+	}
+
+	spec.Process.User = l.getProcessUser()
+
+	// If we are *not* requesting fakeroot, then we need to map the container
+	// uid back to host uid, through the initial fakeroot userns.
+	if !l.cfg.Fakeroot && os.Getuid() != 0 {
+		uidMap, gidMap, err := l.getReverseUserMaps()
+		if err != nil {
+			return nil, err
+		}
+		spec.Linux.UIDMappings = uidMap
+		spec.Linux.GIDMappings = gidMap
+	}
+
+	spec = addNamespaces(spec, l.cfg.Namespaces)
+
+	cwd, err := l.getProcessCwd()
+	if err != nil {
+		return nil, err
+	}
+	spec.Process.Cwd = cwd
+
+	mounts, err := l.getMounts()
+	if err != nil {
+		return nil, err
+	}
+	spec.Mounts = mounts
+
+	return &spec, nil
+}
+
 // Exec will interactively execute a container via the runc low-level runtime.
 // image is a reference to an OCI image, e.g. docker://ubuntu or oci:/tmp/mycontainer
 func (l *Launcher) Exec(ctx context.Context, image string, process string, args []string, instanceName string) error {
@@ -290,36 +319,11 @@ func (l *Launcher) Exec(ctx context.Context, image string, process string, args
 		}
 	}
 
-	spec, err := MinimalSpec()
+	spec, err := l.createSpec()
 	if err != nil {
-		return err
+		return fmt.Errorf("while creating OCI spec: %w", err)
 	}
 
-	spec.Process.User = l.getProcessUser()
-
-	// If we are *not* requesting fakeroot, then we need to map the container
-	// uid back to host uid, through the initial fakeroot userns.
-	if !l.cfg.Fakeroot && os.Getuid() != 0 {
-		uidMap, gidMap, err := l.getReverseUserMaps()
-		if err != nil {
-			return err
-		}
-		spec.Linux.UIDMappings = uidMap
-		spec.Linux.GIDMappings = gidMap
-	}
-
-	cwd, err := l.getProcessCwd()
-	if err != nil {
-		return err
-	}
-	spec.Process.Cwd = cwd
-
-	mounts, err := l.getMounts()
-	if err != nil {
-		return err
-	}
-	spec.Mounts = mounts
-
 	b, err := native.New(
 		native.OptBundlePath(bundleDir),
 		native.OptImageRef(image),

internal/pkg/runtime/launcher/oci/launcher_linux_test.go

@@ -14,10 +14,14 @@ import (
 	"testing"
 
 	"github.com/apptainer/apptainer/internal/pkg/runtime/launcher"
+	"github.com/apptainer/apptainer/internal/pkg/test"
 	"github.com/apptainer/apptainer/pkg/util/apptainerconf"
 )
 
 func TestNewLauncher(t *testing.T) {
+	test.DropPrivilege(t)
+	defer test.ResetPrivilege(t)
+
 	sc, err := apptainerconf.GetConfig(nil)
 	if err != nil {
 		t.Fatalf("while initializing apptainerconf: %s", err)

internal/pkg/runtime/launcher/oci/spec_linux.go

@@ -10,13 +10,30 @@
 package oci
 
 import (
+	"os"
+
+	"github.com/apptainer/apptainer/internal/pkg/runtime/launcher"
+	"github.com/apptainer/apptainer/pkg/sylog"
 	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
-// MinimalSpec returns an OCI runtime spec with a minimal OCI configuration that
+// defaultNamespaces matching native runtime with --compat / --containall.
+var defaultNamespaces = []specs.LinuxNamespace{
+	{
+		Type: specs.IPCNamespace,
+	},
+	{
+		Type: specs.PIDNamespace,
+	},
+	{
+		Type: specs.MountNamespace,
+	},
+}
+
+// minimalSpec returns an OCI runtime spec with a minimal OCI configuration that
 // is a starting point for compatibility with Apptainer's native launcher in
 // `--compat` mode.
-func MinimalSpec() (*specs.Spec, error) {
+func minimalSpec() specs.Spec {
 	config := specs.Spec{
 		Version: specs.Version,
 	}
@@ -58,20 +75,47 @@ func MinimalSpec() (*specs.Spec, error) {
 	// All mounts are added by the launcher, as it must handle flags.
 	config.Mounts = []specs.Mount{}
 
-	config.Linux = &specs.Linux{
-		// Minimum namespaces matching native runtime with --compat / --containall.
-		// TODO: Additional namespaces to be added by launcher.
-		Namespaces: []specs.LinuxNamespace{
-			{
-				Type: specs.IPCNamespace,
-			},
-			{
-				Type: specs.PIDNamespace,
-			},
-			{
-				Type: specs.MountNamespace,
-			},
-		},
+	config.Linux = &specs.Linux{Namespaces: defaultNamespaces}
+	return config
+}
+
+// addNamespaces adds requested namespace, if appropriate, to an existing spec.
+// It is assumed that spec contains at least the defaultNamespaces.
+func addNamespaces(spec specs.Spec, ns launcher.Namespaces) specs.Spec {
+	if ns.IPC {
+		sylog.Infof("--oci runtime always uses an IPC namespace, ipc flag is redundant.")
+	}
+
+	// Currently supports only `--network none`, i.e. isolated loopback only.
+	// Launcher.checkopts enforces this.
+	if ns.Net {
+		spec.Linux.Namespaces = append(
+			spec.Linux.Namespaces,
+			specs.LinuxNamespace{Type: specs.NetworkNamespace},
+		)
 	}
-	return &config, nil
+
+	if ns.PID {
+		sylog.Infof("--oci runtime always uses a PID namespace, pid flag is redundant.")
+	}
+
+	if ns.User {
+		if os.Getuid() == 0 {
+			spec.Linux.Namespaces = append(
+				spec.Linux.Namespaces,
+				specs.LinuxNamespace{Type: specs.UserNamespace},
+			)
+		} else {
+			sylog.Infof("--oci runtime always uses a user namespace when run as a non-root userns, user flag is redundant.")
+		}
+	}
+
+	if ns.UTS {
+		spec.Linux.Namespaces = append(
+			spec.Linux.Namespaces,
+			specs.LinuxNamespace{Type: specs.UTSNamespace},
+		)
+	}
+
+	return spec
 }