From c555424757c5950f6e6e9d8a9219476122fcfc9e Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 2 Sep 2024 10:31:37 +0200 Subject: [PATCH 1/8] pasta: switch dns ip to 169.254.1.1 Per feedback[1] the 169.254.0.0/24 range is reserved for future use in RFC 3927. As such we should not use it here as it might break in the future if the range gets assigned a new meaning. Switch to 169.254.1.1. [1] https://github.com/containers/podman/pull/23791#discussion_r1737913730 Signed-off-by: Paul Holzinger --- libnetwork/pasta/pasta_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnetwork/pasta/pasta_linux.go b/libnetwork/pasta/pasta_linux.go index fd68f87f2..bce603f52 100644 --- a/libnetwork/pasta/pasta_linux.go +++ b/libnetwork/pasta/pasta_linux.go @@ -30,7 +30,7 @@ const ( // dnsForwardIpv4 static ip used as nameserver address inside the netns, // given this is a "link local" ip it should be very unlikely that it causes conflicts - dnsForwardIpv4 = "169.254.0.1" + dnsForwardIpv4 = "169.254.1.1" ) type SetupOptions struct { From 058ba3486771333e52ca2c1ed349fa622b781c83 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 26 Aug 2024 14:45:32 +0200 Subject: [PATCH 2/8] pasta: add new --map-guest-addr option The --map-guest-addr option allows us to sepcify a ip that is remapped to the actual host ip that was used by pasta. This is done to fix the problem where connecting to the host ip was not possible as the same ip was used in the netns. We now set --map-guest-addr 169.254.1.2 which follows the same idea we already used for the --dns-forward option. With that podman can use this ip to set it for host.containers.internal which should the case where there was no second host ip available, see https://github.com/containers/podman/issues/19213 Signed-off-by: Paul Holzinger --- libnetwork/pasta/pasta_linux.go | 48 +++++++++--- libnetwork/pasta/pasta_linux_test.go | 113 +++++++++++++++++++++------ libnetwork/pasta/types.go | 3 + 3 files changed, 129 insertions(+), 35 deletions(-) diff --git a/libnetwork/pasta/pasta_linux.go b/libnetwork/pasta/pasta_linux.go index bce603f52..6caf49411 100644 --- a/libnetwork/pasta/pasta_linux.go +++ b/libnetwork/pasta/pasta_linux.go @@ -26,11 +26,16 @@ import ( ) const ( - dnsForwardOpt = "--dns-forward" + dnsForwardOpt = "--dns-forward" + mapGuestAddrOpt = "--map-guest-addr" // dnsForwardIpv4 static ip used as nameserver address inside the netns, // given this is a "link local" ip it should be very unlikely that it causes conflicts dnsForwardIpv4 = "169.254.1.1" + + // mapGuestAddrIpv4 static ip used as forwarder address inside the netns to reach the host, + // given this is a "link local" ip it should be very unlikely that it causes conflicts + mapGuestAddrIpv4 = "169.254.1.2" ) type SetupOptions struct { @@ -60,7 +65,7 @@ func Setup2(opts *SetupOptions) (*SetupResult, error) { return nil, fmt.Errorf("could not find pasta, the network namespace can't be configured: %w", err) } - cmdArgs, dnsForwardIPs, err := createPastaArgs(opts) + cmdArgs, dnsForwardIPs, mapGuestAddrIPs, err := createPastaArgs(opts) if err != nil { return nil, err } @@ -112,19 +117,27 @@ func Setup2(opts *SetupOptions) (*SetupResult, error) { } result.IPv6 = ipv6 - for _, ip := range dnsForwardIPs { + result.DNSForwardIPs = filterIpFamily(dnsForwardIPs, ipv4, ipv6) + result.MapGuestAddrIPs = filterIpFamily(mapGuestAddrIPs, ipv4, ipv6) + + return result, nil +} + +func filterIpFamily(ips []string, ipv4, ipv6 bool) []string { + var result []string + for _, ip := range ips { ipp := net.ParseIP(ip) - // add the namesever ip only if the address family matches + // add the ip only if the address family matches if ipv4 && util.IsIPv4(ipp) || ipv6 && util.IsIPv6(ipp) { - result.DNSForwardIPs = append(result.DNSForwardIPs, ip) + result = append(result, ip) } } - - return result, nil + return result } -// createPastaArgs creates the pasta arguments, it returns the args to be passed to pasta(1) and as second arg the dns forward ips used. -func createPastaArgs(opts *SetupOptions) ([]string, []string, error) { +// createPastaArgs creates the pasta arguments, it returns the args to be passed to pasta(1) +// and as second arg the dns forward ips used. As third arg the map guest addr ips used. +func createPastaArgs(opts *SetupOptions) ([]string, []string, []string, error) { noTCPInitPorts := true noUDPInitPorts := true noTCPNamespacePorts := true @@ -149,6 +162,7 @@ func createPastaArgs(opts *SetupOptions) ([]string, []string, error) { }) var dnsForwardIPs []string + var mapGuestAddrIPs []string for i, opt := range cmdArgs { switch opt { case "-t", "--tcp-ports": @@ -166,6 +180,10 @@ func createPastaArgs(opts *SetupOptions) ([]string, []string, error) { if len(cmdArgs) > i+1 { dnsForwardIPs = append(dnsForwardIPs, cmdArgs[i+1]) } + case mapGuestAddrOpt: + if len(cmdArgs) > i+1 { + mapGuestAddrIPs = append(mapGuestAddrIPs, cmdArgs[i+1]) + } } } @@ -186,7 +204,7 @@ func createPastaArgs(opts *SetupOptions) ([]string, []string, error) { noUDPInitPorts = false cmdArgs = append(cmdArgs, "-u") default: - return nil, nil, fmt.Errorf("can't forward protocol: %s", protocol) + return nil, nil, nil, fmt.Errorf("can't forward protocol: %s", protocol) } arg := fmt.Sprintf("%s%d-%d:%d-%d", addr, @@ -226,5 +244,13 @@ func createPastaArgs(opts *SetupOptions) ([]string, []string, error) { cmdArgs = append(cmdArgs, "--netns", opts.Netns) - return cmdArgs, dnsForwardIPs, nil + // do this as last arg + if len(mapGuestAddrIPs) == 0 { + // the user did not request custom --map-guest-addr so add our own so that we can use this + // for our own host.containers.internal host entry. + cmdArgs = append(cmdArgs, mapGuestAddrOpt, mapGuestAddrIpv4) + mapGuestAddrIPs = append(mapGuestAddrIPs, mapGuestAddrIpv4) + } + + return cmdArgs, dnsForwardIPs, mapGuestAddrIPs, nil } diff --git a/libnetwork/pasta/pasta_linux_test.go b/libnetwork/pasta/pasta_linux_test.go index 6ad78b242..16beeebbd 100644 --- a/libnetwork/pasta/pasta_linux_test.go +++ b/libnetwork/pasta/pasta_linux_test.go @@ -20,11 +20,12 @@ func makeSetupOptions(configArgs, extraArgs []string, ports []types.PortMapping) func Test_createPastaArgs(t *testing.T) { tests := []struct { - name string - input *SetupOptions - wantArgs []string - wantDnsForward []string - wantErr string + name string + input *SetupOptions + wantArgs []string + wantDnsForward []string + wantMapGuestAddr []string + wantErr string }{ { name: "default options", @@ -36,8 +37,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "basic port", @@ -49,8 +52,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-t", "80-80:80-80", "--dns-forward", dnsForwardIpv4, "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "port range", @@ -62,8 +67,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-t", "80-82:80-82", "--dns-forward", dnsForwardIpv4, "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "different host and container port", @@ -75,8 +82,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-t", "80-80:60-60", "--dns-forward", dnsForwardIpv4, "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "tcp and udp port", @@ -91,8 +100,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-t", "80-80:60-60", "-u", "100-100:100-100", "--dns-forward", dnsForwardIpv4, "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "two tcp ports", @@ -107,8 +118,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-t", "80-80:60-60", "-t", "100-100:100-100", "--dns-forward", dnsForwardIpv4, "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "invalid port", @@ -131,8 +144,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-i", "eth0", "-n", "24", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "config options before extra options", @@ -144,8 +159,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-i", "eth0", "-n", "24", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "-T option", @@ -157,8 +174,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-T", "80", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "--tcp-ns option", @@ -170,8 +189,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "--tcp-ns", "80", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "--map-gw option", @@ -183,8 +204,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-T", "none", "-U", "none", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { // https://github.com/containers/podman/issues/22477 @@ -196,8 +219,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-T", "80", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-U", "none", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "two --map-gw", @@ -209,8 +234,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-T", "80", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-U", "none", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "--dns-forward option", @@ -222,8 +249,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "--dns-forward", "192.168.255.255", "-t", "none", "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{"192.168.255.255"}, + wantDnsForward: []string{"192.168.255.255"}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "two --dns-forward options", @@ -235,8 +264,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "--dns-forward", "192.168.255.255", "--dns-forward", "::1", "-t", "none", "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{"192.168.255.255", "::1"}, + wantDnsForward: []string{"192.168.255.255", "::1"}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "port and custom opt", @@ -248,8 +279,10 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "-i", "eth0", "-t", "80-80:80-80", "--dns-forward", dnsForwardIpv4, "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, }, { name: "Add verbose logging", @@ -261,14 +294,45 @@ func Test_createPastaArgs(t *testing.T) { wantArgs: []string{ "--config-net", "--log-file=/tmp/log", "--trace", "--debug", "--dns-forward", dnsForwardIpv4, "-t", "none", "-u", "none", "-T", "none", "-U", "none", - "--no-map-gw", "--netns", "netns123", + "--no-map-gw", "--netns", "netns123", mapGuestAddrOpt, mapGuestAddrIpv4, }, - wantDnsForward: []string{dnsForwardIpv4}, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{mapGuestAddrIpv4}, + }, + { + name: "--map-guest-addr option", + input: makeSetupOptions( + nil, + []string{mapGuestAddrOpt, "192.168.255.255"}, + nil, + ), + wantArgs: []string{ + "--config-net", mapGuestAddrOpt, "192.168.255.255", dnsForwardOpt, dnsForwardIpv4, + "-t", "none", "-u", "none", "-T", "none", "-U", "none", "--no-map-gw", "--quiet", + "--netns", "netns123", + }, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{"192.168.255.255"}, + }, + { + name: "two --map-guest-addr options", + input: makeSetupOptions( + nil, + []string{mapGuestAddrOpt, "192.168.255.255", mapGuestAddrOpt, "::1"}, + nil, + ), + wantArgs: []string{ + "--config-net", mapGuestAddrOpt, "192.168.255.255", mapGuestAddrOpt, "::1", + dnsForwardOpt, dnsForwardIpv4, "-t", "none", "-u", "none", "-T", "none", + "-U", "none", "--no-map-gw", "--quiet", "--netns", "netns123", + }, + wantDnsForward: []string{dnsForwardIpv4}, + wantMapGuestAddr: []string{"192.168.255.255", "::1"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - args, dnsForward, err := createPastaArgs(tt.input) + args, dnsForward, mapGuestAddr, err := createPastaArgs(tt.input) if tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr, "createPastaArgs error") return @@ -276,6 +340,7 @@ func Test_createPastaArgs(t *testing.T) { assert.NoError(t, err, "expect no createPastaArgs error") assert.Equal(t, tt.wantArgs, args, "check arguments") assert.Equal(t, tt.wantDnsForward, dnsForward, "check dns forward") + assert.Equal(t, tt.wantMapGuestAddr, mapGuestAddr, "check map guest addr") }) } } diff --git a/libnetwork/pasta/types.go b/libnetwork/pasta/types.go index b601e5169..f570afc3e 100644 --- a/libnetwork/pasta/types.go +++ b/libnetwork/pasta/types.go @@ -10,6 +10,9 @@ type SetupResult struct { // DNSForwardIP is the ip used in --dns-forward, it should be added as first // entry to resolv.conf in the container. DNSForwardIPs []string + // MapGuestIps are the ips used for the --map-guest-addr option which + // we can use for the host.containers.internal entry. + MapGuestAddrIPs []string // IPv6 says whenever pasta run with ipv6 support IPv6 bool } From e8d4f595cd74589136fff29f4a3233ca98659b39 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 26 Aug 2024 15:07:08 +0200 Subject: [PATCH 3/8] pasta: make sure --map-guest-addr is backwards compatible --map-guest-addr was just added in 20240814, we cannot yet hard require this option to be present. This means we must deal with the case where the option is not working. Both a version check or checking --help would add extra overhead in the good case. To avoid this we try first with the new option and if this fails check the error message for the right error. If it didn't know about the new option we remove it and try to exec pasta again. Signed-off-by: Paul Holzinger --- libnetwork/pasta/pasta_linux.go | 47 ++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/libnetwork/pasta/pasta_linux.go b/libnetwork/pasta/pasta_linux.go index 6caf49411..9e426d827 100644 --- a/libnetwork/pasta/pasta_linux.go +++ b/libnetwork/pasta/pasta_linux.go @@ -72,23 +72,38 @@ func Setup2(opts *SetupOptions) (*SetupResult, error) { logrus.Debugf("pasta arguments: %s", strings.Join(cmdArgs, " ")) - // pasta forks once ready, and quits once we delete the target namespace - out, err := exec.Command(path, cmdArgs...).CombinedOutput() - if err != nil { - exitErr := &exec.ExitError{} - if errors.As(err, &exitErr) { - return nil, fmt.Errorf("pasta failed with exit code %d:\n%s", - exitErr.ExitCode(), string(out)) + for { + // pasta forks once ready, and quits once we delete the target namespace + out, err := exec.Command(path, cmdArgs...).CombinedOutput() + if err != nil { + exitErr := &exec.ExitError{} + if errors.As(err, &exitErr) { + // special backwards compat check, --map-guest-addr was added in pasta version 20240814 so we + // cannot hard require it yet. Once we are confident that the update is most distros we can remove it. + if exitErr.ExitCode() == 1 && + strings.Contains(string(out), "unrecognized option '"+mapGuestAddrOpt) && + len(mapGuestAddrIPs) == 1 && mapGuestAddrIPs[0] == mapGuestAddrIpv4 { + // we did add the default --map-guest-addr option, if users set something different we want + // to get to the error below. We have to unset mapGuestAddrIPs here to avoid a infinite loop. + mapGuestAddrIPs = nil + // Trim off last two args which are --map-guest-addr 169.254.1.2. + cmdArgs = cmdArgs[:len(cmdArgs)-2] + continue + } + return nil, fmt.Errorf("pasta failed with exit code %d:\n%s", + exitErr.ExitCode(), string(out)) + } + return nil, fmt.Errorf("failed to start pasta: %w", err) } - return nil, fmt.Errorf("failed to start pasta: %w", err) - } - if len(out) > 0 { - // TODO: This should be warning but right now pasta still prints - // things with --quiet that we do not care about. - // For now info is fine and we can bump it up later, it is only a - // nice to have. - logrus.Infof("pasta logged warnings: %q", string(out)) + if len(out) > 0 { + // TODO: This should be warning but right now pasta still prints + // things with --quiet that we do not care about. + // For now info is fine and we can bump it up later, it is only a + // nice to have. + logrus.Infof("pasta logged warnings: %q", string(out)) + } + break } var ipv4, ipv6 bool @@ -244,7 +259,7 @@ func createPastaArgs(opts *SetupOptions) ([]string, []string, []string, error) { cmdArgs = append(cmdArgs, "--netns", opts.Netns) - // do this as last arg + // do this as last arg so we can easily trim them off in the error case when we have an older version if len(mapGuestAddrIPs) == 0 { // the user did not request custom --map-guest-addr so add our own so that we can use this // for our own host.containers.internal host entry. From a7415c3eab4675a5b37dbee9d33833af5f2b086e Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 26 Aug 2024 14:52:06 +0200 Subject: [PATCH 4/8] pasta: rename Setup2() to Setup() I already switch all user from the old Setup over to Setup2(), so no we can again reuse the Setup() name. As such alias Setup and Setup for the same function and then once I migrated all callers in podman and buildah I will remove Setup2() here. Signed-off-by: Paul Holzinger --- libnetwork/pasta/pasta_linux.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libnetwork/pasta/pasta_linux.go b/libnetwork/pasta/pasta_linux.go index 9e426d827..94617adf6 100644 --- a/libnetwork/pasta/pasta_linux.go +++ b/libnetwork/pasta/pasta_linux.go @@ -50,16 +50,16 @@ type SetupOptions struct { ExtraOptions []string } -func Setup(opts *SetupOptions) error { - _, err := Setup2(opts) - return err +// Setup2 alias for Setup() +func Setup2(opts *SetupOptions) (*SetupResult, error) { + return Setup(opts) } -// Setup2 start the pasta process for the given netns. +// Setup start the pasta process for the given netns. // The pasta binary is looked up in the HelperBinariesDir and $PATH. // Note that there is no need for any special cleanup logic, the pasta // process will automatically exit when the netns path is deleted. -func Setup2(opts *SetupOptions) (*SetupResult, error) { +func Setup(opts *SetupOptions) (*SetupResult, error) { path, err := opts.Config.FindHelperBinary(BinaryName, true) if err != nil { return nil, fmt.Errorf("could not find pasta, the network namespace can't be configured: %w", err) From 47bd8a898f3f89d6dbdcd5ddbc0f50da699bc3b8 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 26 Aug 2024 17:49:19 +0200 Subject: [PATCH 5/8] rootlessnetns: cache dns and guest addr options When using the rootless netns (bridge mode) so far podman ignored the proper pasta or slirp4netns dns sever for networks without aardvark-dns. This is not good. We should try to use them by default, and with the new MapGuestAddr option we need to use that as well for host.containers.internal. The problem is that becuase we only know what options we uses when we started the process later container starts from a new podman process do not really see these options if we just cache the result in memory. So in order to make all following podman process aware we serialize this info struct as json and later processes read it when needed. It also means we do not have to lookup the netns ip evey time so I removed that code. Signed-off-by: Paul Holzinger --- .../internal/rootlessnetns/netns_linux.go | 82 +++++++++++++------ libnetwork/types/network.go | 4 + 2 files changed, 63 insertions(+), 23 deletions(-) diff --git a/libnetwork/internal/rootlessnetns/netns_linux.go b/libnetwork/internal/rootlessnetns/netns_linux.go index f8b9b76df..84bc4f621 100644 --- a/libnetwork/internal/rootlessnetns/netns_linux.go +++ b/libnetwork/internal/rootlessnetns/netns_linux.go @@ -1,6 +1,7 @@ package rootlessnetns import ( + "encoding/json" "errors" "fmt" "io/fs" @@ -34,6 +35,9 @@ const ( // refCountFile file name for the ref count file refCountFile = "ref-count" + // infoCacheFile file name for the cache file used to store the rootless netns info + infoCacheFile = "info.json" + // rootlessNetNsConnPidFile is the name of the rootless netns slirp4netns/pasta pid file rootlessNetNsConnPidFile = "rootless-netns-conn.pid" @@ -54,11 +58,9 @@ type Netns struct { // config contains containers.conf options. config *config.Config - // ipAddresses used in the netns, this is needed to store - // the netns ips that are used by pasta. This is then handed - // back to the caller via IPAddresses() which then can make - // sure to not use them for host.containers.internal. - ipAddresses []net.IP + // info contain information about ip addresses used in the netns. + // A caller can get this info via Info(). + info *types.RootlessNetnsInfo } type rootlessNetnsError struct { @@ -115,6 +117,9 @@ func (n *Netns) getOrCreateNetns() (ns.NetNS, bool, error) { // quick check if pasta/slirp4netns are still running err := unix.Kill(pid, 0) if err == nil { + if err := n.deserializeInfo(); err != nil { + return nil, false, wrapError("deserialize info", err) + } // All good, return the netns. return nsRef, false, nil } @@ -227,6 +232,15 @@ func (n *Netns) setupPasta(nsPath string) error { return wrapError("create resolv.conf", err) } + n.info = &types.RootlessNetnsInfo{ + IPAddresses: res.IPAddresses, + DnsForwardIps: res.DNSForwardIPs, + MapGuestIps: res.MapGuestAddrIPs, + } + if err := n.serializeInfo(); err != nil { + return wrapError("serialize info", err) + } + return nil } @@ -261,6 +275,12 @@ func (n *Netns) setupSlirp4netns(nsPath string) error { if err != nil { return wrapError("determine default slirp4netns DNS address", err) } + nameservers := []string{resolveIP.String()} + + netnsIP, err := slirp4netns.GetIP(res.Subnet) + if err != nil { + return wrapError("determine default slirp4netns ip address", err) + } if err := resolvconf.New(&resolvconf.Params{ Path: n.getPath(resolvConfName), @@ -270,10 +290,19 @@ func (n *Netns) setupSlirp4netns(nsPath string) error { }, IPv6Enabled: res.IPv6, KeepHostServers: true, - Nameservers: []string{resolveIP.String()}, + Nameservers: nameservers, }); err != nil { return wrapError("create resolv.conf", err) } + + n.info = &types.RootlessNetnsInfo{ + IPAddresses: []net.IP{*netnsIP}, + DnsForwardIps: nameservers, + } + if err := n.serializeInfo(); err != nil { + return wrapError("serialize info", err) + } + return nil } @@ -541,20 +570,6 @@ func (n *Netns) runInner(toRun func() error, cleanup bool) (err error) { if err := toRun(); err != nil { return err } - - // get the current active addresses in the netns, and store them - addrs, err := net.InterfaceAddrs() - if err != nil { - return err - } - ips := make([]net.IP, 0, len(addrs)) - for _, addr := range addrs { - // make sure to skip localhost and other special addresses - if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.IsGlobalUnicast() { - ips = append(ips, ipnet.IP) - } - } - n.ipAddresses = ips return nil }) } @@ -630,9 +645,7 @@ func (n *Netns) Run(lock *lockfile.LockFile, toRun func() error) error { // IPAddresses returns the currently used ip addresses in the netns // These should then not be assigned for the host.containers.internal entry. func (n *Netns) Info() *types.RootlessNetnsInfo { - return &types.RootlessNetnsInfo{ - IPAddresses: n.ipAddresses, - } + return n.info } func refCount(dir string, inc int) (int, error) { @@ -671,3 +684,26 @@ func readPidFile(path string) (int, error) { } return strconv.Atoi(strings.TrimSpace(string(b))) } + +func (n *Netns) serializeInfo() error { + f, err := os.Create(filepath.Join(n.dir, infoCacheFile)) + if err != nil { + return err + } + return json.NewEncoder(f).Encode(n.info) +} + +func (n *Netns) deserializeInfo() error { + f, err := os.Open(filepath.Join(n.dir, infoCacheFile)) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil + } + return err + } + defer f.Close() + if n.info == nil { + n.info = new(types.RootlessNetnsInfo) + } + return json.NewDecoder(f).Decode(n.info) +} diff --git a/libnetwork/types/network.go b/libnetwork/types/network.go index 9741103f5..77c76bf78 100644 --- a/libnetwork/types/network.go +++ b/libnetwork/types/network.go @@ -342,6 +342,10 @@ type TeardownOptions struct { type RootlessNetnsInfo struct { // IPAddresses used in the netns, must not be used for host.containers.internal IPAddresses []net.IP + // DnsForwardIps ips used in resolv.conf + DnsForwardIps []string + // MapGuestIps should be used for the host.containers.internal entry when set + MapGuestIps []string } // FilterFunc can be passed to NetworkList to filter the networks. From 6e7b17bf4cf26a3bd86804f8e4b57775587b92b8 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 29 Aug 2024 11:29:31 +0200 Subject: [PATCH 6/8] pasta: update warnings line Use %s as %q just quotes/escapes everything which makes it harder to read and trim of the last newline and spaces as well. Also update the warnings comment, we still see warnings by default on our debian VMs in podman CI so this cannot be on the warning level yet. Signed-off-by: Paul Holzinger --- libnetwork/pasta/pasta_linux.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libnetwork/pasta/pasta_linux.go b/libnetwork/pasta/pasta_linux.go index 94617adf6..10b667eb6 100644 --- a/libnetwork/pasta/pasta_linux.go +++ b/libnetwork/pasta/pasta_linux.go @@ -97,11 +97,12 @@ func Setup(opts *SetupOptions) (*SetupResult, error) { } if len(out) > 0 { - // TODO: This should be warning but right now pasta still prints - // things with --quiet that we do not care about. - // For now info is fine and we can bump it up later, it is only a + // TODO: This should be warning but as of August 2024 pasta still prints + // things with --quiet that we do not care about. In podman CI I still see + // "Couldn't get any nameserver address" so until this is fixed we cannot + // enable it. For now info is fine and we can bump it up later, it is only a // nice to have. - logrus.Infof("pasta logged warnings: %q", string(out)) + logrus.Infof("pasta logged warnings: %q", strings.TrimSpace(string(out))) } break } From 6e3e128d96b95540edd6655d7c840af487514373 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 2 Sep 2024 10:58:15 +0200 Subject: [PATCH 7/8] libnetwork/etchosts: rework GetHostContainersInternalIP() GetHostContainersInternalIP() is no longer called in podman or buildah as they use GetHostContainersInternalIPExcluding(). I need to add a new option so chnage the function to accept the parameters as struct so we do not have to break the API every time we add a new parameter. Signed-off-by: Paul Holzinger --- libnetwork/etchosts/ip.go | 46 +++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/libnetwork/etchosts/ip.go b/libnetwork/etchosts/ip.go index 909fcf67d..de8d1fbc1 100644 --- a/libnetwork/etchosts/ip.go +++ b/libnetwork/etchosts/ip.go @@ -10,17 +10,23 @@ import ( "github.com/containers/storage/pkg/unshare" ) -// GetHostContainersInternalIP returns the host.containers.internal ip -// if netStatus is not nil then networkInterface also must be non nil otherwise this function panics -func GetHostContainersInternalIP(conf *config.Config, netStatus map[string]types.StatusBlock, networkInterface types.ContainerNetwork) string { - return GetHostContainersInternalIPExcluding(conf, netStatus, networkInterface, nil) +// HostContainersInternalOptions contains the options for GetHostContainersInternalIP() +type HostContainersInternalOptions struct { + // Conf is the containers.Conf, must not be nil + Conf *config.Config + // NetStatus is the network status for the container, + // if this is set networkInterface must not be nil + NetStatus map[string]types.StatusBlock + // NetworkInterface of the current runtime + NetworkInterface types.ContainerNetwork + // Exclude are then ips that should not be returned, this is + // useful to prevent returning the same ip as in the container. + Exclude []net.IP } -// GetHostContainersInternalIPExcluding returns the host.containers.internal ip -// Exclude are ips that should not be returned, this is useful to prevent returning the same ip as in the container. -// if netStatus is not nil then networkInterface also must be non nil otherwise this function panics -func GetHostContainersInternalIPExcluding(conf *config.Config, netStatus map[string]types.StatusBlock, networkInterface types.ContainerNetwork, exclude []net.IP) string { - switch conf.Containers.HostContainersInternalIP { +// GetHostContainersInternalIP returns the host.containers.internal ip +func GetHostContainersInternalIP(opts HostContainersInternalOptions) string { + switch opts.Conf.Containers.HostContainersInternalIP { case "": // if empty (default) we will automatically choose one below // if machine using gvproxy we let the gvproxy dns server handle the dns name so do not add it @@ -30,16 +36,16 @@ func GetHostContainersInternalIPExcluding(conf *config.Config, netStatus map[str case "none": return "" default: - return conf.Containers.HostContainersInternalIP + return opts.Conf.Containers.HostContainersInternalIP } ip := "" // Only use the bridge ip when root, as rootless the interfaces are created // inside the special netns and not the host so we cannot use them. if unshare.IsRootless() { - return util.GetLocalIPExcluding(exclude) + return util.GetLocalIPExcluding(opts.Exclude) } - for net, status := range netStatus { - network, err := networkInterface.NetworkInspect(net) + for net, status := range opts.NetStatus { + network, err := opts.NetworkInterface.NetworkInspect(net) // only add the host entry for bridge networks // ip/macvlan gateway is normally not on the host if err != nil || network.Driver != types.BridgeNetworkDriver { @@ -60,7 +66,19 @@ func GetHostContainersInternalIPExcluding(conf *config.Config, netStatus map[str if ip != "" { return ip } - return util.GetLocalIPExcluding(exclude) + return util.GetLocalIPExcluding(opts.Exclude) +} + +// GetHostContainersInternalIPExcluding returns the host.containers.internal ip +// Exclude are ips that should not be returned, this is useful to prevent returning the same ip as in the container. +// if netStatus is not nil then networkInterface also must be non nil otherwise this function panics +func GetHostContainersInternalIPExcluding(conf *config.Config, netStatus map[string]types.StatusBlock, networkInterface types.ContainerNetwork, exclude []net.IP) string { + return GetHostContainersInternalIP(HostContainersInternalOptions{ + Conf: conf, + NetStatus: netStatus, + NetworkInterface: networkInterface, + Exclude: exclude, + }) } // GetNetworkHostEntries returns HostEntries for all ips in the network status From 6125e268519f7e54912b226fc7ab9d70f8b07b02 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 2 Sep 2024 11:04:36 +0200 Subject: [PATCH 8/8] libnetwork/etchosts: add PreferIP option For the pasta network mode we now use --map-guest-addr which means we have a specific ip that we want to use as host.containers.internal address. I first thought we could handle it in podman but that doesn't work as the contianers.conf option must have a higher priority. Signed-off-by: Paul Holzinger --- libnetwork/etchosts/ip.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libnetwork/etchosts/ip.go b/libnetwork/etchosts/ip.go index de8d1fbc1..588fbff35 100644 --- a/libnetwork/etchosts/ip.go +++ b/libnetwork/etchosts/ip.go @@ -22,6 +22,10 @@ type HostContainersInternalOptions struct { // Exclude are then ips that should not be returned, this is // useful to prevent returning the same ip as in the container. Exclude []net.IP + // PreferIP is a ip that should be used if set but it has a + // lower priority than the containers.conf config option. + // This is used for the pasta --map-guest-addr ip. + PreferIP string } // GetHostContainersInternalIP returns the host.containers.internal ip @@ -38,6 +42,12 @@ func GetHostContainersInternalIP(opts HostContainersInternalOptions) string { default: return opts.Conf.Containers.HostContainersInternalIP } + + // caller has a specific ip it prefers + if opts.PreferIP != "" { + return opts.PreferIP + } + ip := "" // Only use the bridge ip when root, as rootless the interfaces are created // inside the special netns and not the host so we cannot use them.