buildah: actually use containers.conf settings

Buildah ignores the container.conf settings. Commit 05d5d22 tried to fix this but I cannot see how this would have worked, there were also no test added related to containers.conf. The code below were we read the default just overwrites everything we already set so the config settings are ignored. Also there are many weird places were settings are just overwritten for no particular reason. The namespaces code path looks like mess to me. Namespaces are added/changed/removed in many different places and there is no explanation why. Maybe I am just not familar enough with this code base but I cannot really understand this. Hopefully a maintainer can take a closer look to see if my changes are indeed correct. Fixes containers/podman#13294 Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Luap99 · Apr 5, 2022 · 3ae9b6d · 3ae9b6d
1 parent 46d18e0
commit 3ae9b6d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 45 deletions.
diff --git a/run_linux.go b/run_linux.go
@@ -1181,9 +1181,7 @@ func setupRootlessNetwork(pid int) (teardown func(), err error) {
 
 func (b *Builder) runConfigureNetwork(pid int, isolation define.Isolation, options RunOptions, configureNetworks []string, containerName string) (teardown func(), err error) {
 	if isolation == IsolationOCIRootless {
-		if ns := options.NamespaceOptions.Find(string(specs.NetworkNamespace)); ns != nil && !ns.Host && ns.Path == "" {
-			return setupRootlessNetwork(pid)
-		}
+		return setupRootlessNetwork(pid)
 	}
 
 	if len(configureNetworks) == 0 {
@@ -2400,33 +2398,14 @@ func waitForSync(pipeR *os.File) error {
 func checkAndOverrideIsolationOptions(isolation define.Isolation, options *RunOptions) error {
 	switch isolation {
 	case IsolationOCIRootless:
-		if ns := options.NamespaceOptions.Find(string(specs.IPCNamespace)); ns == nil || ns.Host {
-			logrus.Debugf("Forcing use of an IPC namespace.")
-		}
-		options.NamespaceOptions.AddOrReplace(define.NamespaceOption{Name: string(specs.IPCNamespace)})
-		_, err := exec.LookPath("slirp4netns")
-		hostNetworking := err != nil
-		networkNamespacePath := ""
-		if ns := options.NamespaceOptions.Find(string(specs.NetworkNamespace)); ns != nil {
-			hostNetworking = ns.Host
-			networkNamespacePath = ns.Path
-			if hostNetworking {
-				networkNamespacePath = ""
-			}
-		}
-		options.NamespaceOptions.AddOrReplace(define.NamespaceOption{
-			Name: string(specs.NetworkNamespace),
-			Host: hostNetworking,
-			Path: networkNamespacePath,
-		})
-		if ns := options.NamespaceOptions.Find(string(specs.PIDNamespace)); ns == nil || ns.Host {
-			logrus.Debugf("Forcing use of a PID namespace.")
-		}
-		options.NamespaceOptions.AddOrReplace(define.NamespaceOption{Name: string(specs.PIDNamespace), Host: false})
-		if ns := options.NamespaceOptions.Find(string(specs.UserNamespace)); ns == nil || ns.Host {
-			logrus.Debugf("Forcing use of a user namespace.")
+		// only change the netns if the caller did not set it
+		if ns := options.NamespaceOptions.Find(string(specs.NetworkNamespace)); ns == nil {
+			if _, err := exec.LookPath("slirp4netns"); err != nil {
+				// if slirp4netns is not installed we have to use the hosts net namespace
+				options.NamespaceOptions.AddOrReplace(define.NamespaceOption{Name: string(specs.NetworkNamespace), Host: true})
+			}
 		}
-		options.NamespaceOptions.AddOrReplace(define.NamespaceOption{Name: string(specs.UserNamespace)})
+		fallthrough
 	case IsolationOCI:
 		pidns := options.NamespaceOptions.Find(string(specs.PIDNamespace))
 		userns := options.NamespaceOptions.Find(string(specs.UserNamespace))
@@ -2447,25 +2426,12 @@ func DefaultNamespaceOptions() (define.NamespaceOptions, error) {
 	options := define.NamespaceOptions{
 		{Name: string(specs.CgroupNamespace), Host: cfg.CgroupNS() == "host"},
 		{Name: string(specs.IPCNamespace), Host: cfg.IPCNS() == "host"},
-		{Name: string(specs.MountNamespace), Host: true},
-		{Name: string(specs.NetworkNamespace), Host: cfg.NetNS() == "host" || cfg.NetNS() == "container"},
+		{Name: string(specs.MountNamespace), Host: false},
+		{Name: string(specs.NetworkNamespace), Host: cfg.NetNS() == "host"},
 		{Name: string(specs.PIDNamespace), Host: cfg.PidNS() == "host"},
-		{Name: string(specs.UserNamespace), Host: true},
+		{Name: string(specs.UserNamespace), Host: cfg.Containers.UserNS == "host"},
 		{Name: string(specs.UTSNamespace), Host: cfg.UTSNS() == "host"},
 	}
-	g, err := generate.New("linux")
-	if err != nil {
-		return options, errors.Wrapf(err, "error generating new 'linux' runtime spec")
-	}
-	spec := g.Config
-	if spec.Linux != nil {
-		for _, ns := range spec.Linux.Namespaces {
-			options.AddOrReplace(define.NamespaceOption{
-				Name: string(ns.Type),
-				Path: ns.Path,
-			})
-		}
-	}
 	return options, nil
 }
 

diff --git a/tests/namespaces.bats b/tests/namespaces.bats
@@ -486,3 +486,39 @@ _EOF
   run_buildah 125 from --signature-policy ${TESTSDIR}/policy.json --quiet --userns-gid-map=0:10000:65536 alpine
   expect_output --substring "userns-gid-map can not be used without --userns-uid-map"
 }
+
+@test "use containers.conf namespace settings" {
+  _prefetch alpine
+
+  containers_conf_file="$TESTDIR/containers-namespaces.conf"
+
+  for mode in host private; do
+    cat > "$containers_conf_file" << EOF
+[containers]
+
+cgroupns = "$mode"
+netns = "$mode"
+pidns = "$mode"
+ipcns = "$mode"
+utsns = "$mode"
+EOF
+
+    CONTAINERS_CONF="$containers_conf_file" run_buildah from --signature-policy ${TESTSDIR}/policy.json --quiet alpine
+    [ "$output" != "" ]
+    ctr="$output"
+
+    local op="=="
+    if [[ "$mode" == "private" ]]; then
+      op="!="
+    fi
+
+    for nstype in cgroup ipc net pid uts; do
+      run readlink /proc/self/ns/"$nstype"
+      ns="$output"
+      run_buildah run $ctr readlink /proc/self/ns/"$nstype"
+      assert "$output" $op "$ns" "namespace matches expected ($mode)"
+    done
+  done
+
+  rm "$containers_conf_file"
+}