From 3ae9b6d30652067c53a0d7c0250a13ef9b65f1cc Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 22 Mar 2022 19:14:06 +0100 Subject: [PATCH] buildah: actually use containers.conf settings Buildah ignores the container.conf settings. Commit 05d5d22dc6b7 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 --- run_linux.go | 56 +++++++++---------------------------------- tests/namespaces.bats | 36 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 45 deletions(-) diff --git a/run_linux.go b/run_linux.go index 411a0f8cfde..b7bf995f45b 100644 --- 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 index d0fb75d5eaf..bfaace20772 100644 --- 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" +}