From 2805c7353b15679d66eec988949bb79e1e320805 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 31 May 2022 18:37:40 +0200 Subject: pkg/specgen: parse default network mode on server When podman-remote is used we should not resolve the default network mode on the client. Defaults should be set on the server. In this case this is important because we have different defaults for root/rootless. So when the client is rootless and the server is root we must pick the root default. Note that this already worked when --network was set since we did not parsed the flag in this case. To reproduce you need --network=default. Also removed a unused function. [NO NEW TESTS NEEDED] I tested it manually but I am not sure how I can hook a test like this up in CI. The client would need to run as rootless and the server as root or the other way around. Fixes #14368 Signed-off-by: Paul Holzinger --- pkg/specgen/container_validate.go | 6 ++-- pkg/specgen/generate/namespaces.go | 12 +++---- pkg/specgen/generate/pod_create.go | 9 ++++-- pkg/specgen/namespaces.go | 65 +------------------------------------- pkg/specgen/namespaces_test.go | 25 +++++---------- 5 files changed, 25 insertions(+), 92 deletions(-) (limited to 'pkg/specgen') diff --git a/pkg/specgen/container_validate.go b/pkg/specgen/container_validate.go index 532a2094f..5616a4511 100644 --- a/pkg/specgen/container_validate.go +++ b/pkg/specgen/container_validate.go @@ -183,10 +183,12 @@ func (s *SpecGenerator) Validate() error { } // Set defaults if network info is not provided - if s.NetNS.NSMode == "" { - s.NetNS.NSMode = Bridge + // when we are rootless we default to slirp4netns + if s.NetNS.IsPrivate() || s.NetNS.IsDefault() { if rootless.IsRootless() { s.NetNS.NSMode = Slirp + } else { + s.NetNS.NSMode = Bridge } } if err := validateNetNS(&s.NetNS); err != nil { diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index 37d561ec2..4dd6b3eaf 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -236,10 +236,12 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod. toReturn = append(toReturn, libpod.WithCgroupsMode(s.CgroupsMode)) } - // Net - // TODO validate CNINetworks, StaticIP, StaticIPv6 are only set if we - // are in bridge mode. postConfigureNetNS := !s.UserNS.IsHost() + // when we are rootless we default to slirp4netns + if rootless.IsRootless() && (s.NetNS.IsPrivate() || s.NetNS.IsDefault()) { + s.NetNS.NSMode = specgen.Slirp + } + switch s.NetNS.NSMode { case specgen.FromPod: if pod == nil || infraCtr == nil { @@ -262,9 +264,7 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod. val = fmt.Sprintf("slirp4netns:%s", s.NetNS.Value) } toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, val, nil)) - case specgen.Private: - fallthrough - case specgen.Bridge: + case specgen.Bridge, specgen.Private, specgen.Default: portMappings, expose, err := createPortMappings(s, imageData) if err != nil { return nil, err diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 5b7bb2b57..d4f281a11 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -141,6 +141,9 @@ func MapSpec(p *specgen.PodSpecGenerator) (*specgen.SpecGenerator, error) { case specgen.Bridge: p.InfraContainerSpec.NetNS.NSMode = specgen.Bridge logrus.Debugf("Pod using bridge network mode") + case specgen.Private: + p.InfraContainerSpec.NetNS.NSMode = specgen.Private + logrus.Debugf("Pod will use default network mode") case specgen.Host: logrus.Debugf("Pod will use host networking") if len(p.InfraContainerSpec.PortMappings) > 0 || @@ -151,15 +154,15 @@ func MapSpec(p *specgen.PodSpecGenerator) (*specgen.SpecGenerator, error) { p.InfraContainerSpec.NetNS.NSMode = specgen.Host case specgen.Slirp: logrus.Debugf("Pod will use slirp4netns") - if p.InfraContainerSpec.NetNS.NSMode != "host" { + if p.InfraContainerSpec.NetNS.NSMode != specgen.Host { p.InfraContainerSpec.NetworkOptions = p.NetworkOptions - p.InfraContainerSpec.NetNS.NSMode = specgen.NamespaceMode("slirp4netns") + p.InfraContainerSpec.NetNS.NSMode = specgen.Slirp } case specgen.NoNetwork: logrus.Debugf("Pod will not use networking") if len(p.InfraContainerSpec.PortMappings) > 0 || len(p.InfraContainerSpec.Networks) > 0 || - p.InfraContainerSpec.NetNS.NSMode == "host" { + p.InfraContainerSpec.NetNS.NSMode == specgen.Host { return nil, errors.Wrapf(define.ErrInvalidArg, "cannot disable pod network if network-related configuration is specified") } p.InfraContainerSpec.NetNS.NSMode = specgen.NoNetwork diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go index 5a3b94ca4..f1343f6e2 100644 --- a/pkg/specgen/namespaces.go +++ b/pkg/specgen/namespaces.go @@ -10,7 +10,6 @@ import ( "github.com/containers/common/pkg/cgroups" cutil "github.com/containers/common/pkg/util" "github.com/containers/podman/v4/libpod/define" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/util" "github.com/containers/storage" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -319,62 +318,6 @@ func ParseUserNamespace(ns string) (Namespace, error) { return ParseNamespace(ns) } -// ParseNetworkNamespace parses a network namespace specification in string -// form. -// Returns a namespace and (optionally) a list of CNI networks to join. -func ParseNetworkNamespace(ns string, rootlessDefaultCNI bool) (Namespace, map[string]types.PerNetworkOptions, error) { - toReturn := Namespace{} - networks := make(map[string]types.PerNetworkOptions) - // Net defaults to Slirp on rootless - switch { - case ns == string(Slirp), strings.HasPrefix(ns, string(Slirp)+":"): - toReturn.NSMode = Slirp - case ns == string(FromPod): - toReturn.NSMode = FromPod - case ns == "" || ns == string(Default) || ns == string(Private): - if rootless.IsRootless() { - if rootlessDefaultCNI { - toReturn.NSMode = Bridge - } else { - toReturn.NSMode = Slirp - } - } else { - toReturn.NSMode = Bridge - } - case ns == string(Bridge): - toReturn.NSMode = Bridge - case ns == string(NoNetwork): - toReturn.NSMode = NoNetwork - case ns == string(Host): - toReturn.NSMode = Host - case strings.HasPrefix(ns, "ns:"): - split := strings.SplitN(ns, ":", 2) - if len(split) != 2 { - return toReturn, nil, errors.Errorf("must provide a path to a namespace when specifying \"ns:\"") - } - toReturn.NSMode = Path - toReturn.Value = split[1] - case strings.HasPrefix(ns, string(FromContainer)+":"): - split := strings.SplitN(ns, ":", 2) - if len(split) != 2 { - return toReturn, nil, errors.Errorf("must provide name or ID or a container when specifying \"container:\"") - } - toReturn.NSMode = FromContainer - toReturn.Value = split[1] - default: - // Assume we have been given a list of CNI networks. - // Which only works in bridge mode, so set that. - networkList := strings.Split(ns, ",") - for _, net := range networkList { - networks[net] = types.PerNetworkOptions{} - } - - toReturn.NSMode = Bridge - } - - return toReturn, networks, nil -} - // ParseNetworkFlag parses a network string slice into the network options // If the input is nil or empty it will use the default setting from containers.conf func ParseNetworkFlag(networks []string) (Namespace, map[string]types.PerNetworkOptions, map[string][]string, error) { @@ -400,13 +343,7 @@ func ParseNetworkFlag(networks []string) (Namespace, map[string]types.PerNetwork case ns == string(FromPod): toReturn.NSMode = FromPod case ns == "" || ns == string(Default) || ns == string(Private): - // Net defaults to Slirp on rootless - if rootless.IsRootless() { - toReturn.NSMode = Slirp - break - } - // if root we use bridge - fallthrough + toReturn.NSMode = Private case ns == string(Bridge), strings.HasPrefix(ns, string(Bridge)+":"): toReturn.NSMode = Bridge parts := strings.SplitN(ns, ":", 2) diff --git a/pkg/specgen/namespaces_test.go b/pkg/specgen/namespaces_test.go index 368c92bd5..d03a6d032 100644 --- a/pkg/specgen/namespaces_test.go +++ b/pkg/specgen/namespaces_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/containers/common/libnetwork/types" - "github.com/containers/podman/v4/pkg/rootless" "github.com/stretchr/testify/assert" ) @@ -17,14 +16,6 @@ func parsMacNoErr(mac string) types.HardwareAddr { func TestParseNetworkFlag(t *testing.T) { // root and rootless have different defaults defaultNetName := "default" - defaultNetworks := map[string]types.PerNetworkOptions{ - defaultNetName: {}, - } - defaultNsMode := Namespace{NSMode: Bridge} - if rootless.IsRootless() { - defaultNsMode = Namespace{NSMode: Slirp} - defaultNetworks = map[string]types.PerNetworkOptions{} - } tests := []struct { name string @@ -37,26 +28,26 @@ func TestParseNetworkFlag(t *testing.T) { { name: "empty input", args: nil, - nsmode: defaultNsMode, - networks: defaultNetworks, + nsmode: Namespace{NSMode: Private}, + networks: map[string]types.PerNetworkOptions{}, }, { name: "empty string as input", args: []string{}, - nsmode: defaultNsMode, - networks: defaultNetworks, + nsmode: Namespace{NSMode: Private}, + networks: map[string]types.PerNetworkOptions{}, }, { name: "default mode", args: []string{"default"}, - nsmode: defaultNsMode, - networks: defaultNetworks, + nsmode: Namespace{NSMode: Private}, + networks: map[string]types.PerNetworkOptions{}, }, { name: "private mode", args: []string{"private"}, - nsmode: defaultNsMode, - networks: defaultNetworks, + nsmode: Namespace{NSMode: Private}, + networks: map[string]types.PerNetworkOptions{}, }, { name: "bridge mode", -- cgit v1.2.3-54-g00ecf