From 3b99bea9c2113c368cf72f728091b51a3e0f13b9 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 11 Apr 2022 14:04:34 +0200 Subject: [PATCH 1/5] add new etchosts package Add a new libnetwork/etchosts package to manage reading/writing hosts files. This package exports four functions New(), Add(), AddIfExists() and Remove(). See the godoc comments on the functions. Both podman and buildah should use this functions to make sure files are generated identical. Signed-off-by: Paul Holzinger --- libnetwork/etchosts/hosts.go | 340 +++++++++++++++++++ libnetwork/etchosts/hosts_test.go | 535 ++++++++++++++++++++++++++++++ 2 files changed, 875 insertions(+) create mode 100644 libnetwork/etchosts/hosts.go create mode 100644 libnetwork/etchosts/hosts_test.go diff --git a/libnetwork/etchosts/hosts.go b/libnetwork/etchosts/hosts.go new file mode 100644 index 000000000..1788de2f3 --- /dev/null +++ b/libnetwork/etchosts/hosts.go @@ -0,0 +1,340 @@ +package etchosts + +import ( + "bufio" + "errors" + "fmt" + "io" + "os" + "strings" + + "github.com/containers/common/pkg/util" +) + +const ( + // DefaultHostsFile is the default path to the hosts file + DefaultHostsFile = "/etc/hosts" + hostContainersInternal = "host.containers.internal" + localhost = "localhost" +) + +type HostEntries []HostEntry + +type HostEntry struct { + IP string + Names []string +} + +// Params for the New() function call +type Params struct { + // BaseFile is the file where we read entries from and add entries to + // the target hosts file. If the name is empty it will not read any entries. + BaseFile string + // ExtraHosts is a slice of entries in the "hostname:ip" format. + // Optional. + ExtraHosts []string + // ContainerIPs should contain the main container ipv4 and ipv6 if available + // with the container name and host name as names set. + // Optional. + ContainerIPs HostEntries + // HostContainersInternalIP is the IP for the host.containers.internal entry. + // Optional. + HostContainersInternalIP string + // TargetFile where the hosts are written to. + TargetFile string +} + +// New will create a new hosts file and write this to the target file. +// This function does not prevent any kind of concurrency problems, it is +// the callers responsibility to avoid concurrent writes to this file. +// The extraHosts are written first, then the hosts from the file baseFile and the +// containerIps. The container ip entry is only added when the name was not already +// added before. +func New(params *Params) error { + if err := new(params); err != nil { + return fmt.Errorf("failed to create new hosts file: %w", err) + } + return nil +} + +// Add adds the given entries to the hosts file, entries are only added if +// they are not already present. +// Add is not atomic because it will keep the current file inode. This is +// required to keep bind mounts for containers working. +func Add(file string, entries HostEntries) error { + if err := add(file, entries); err != nil { + return fmt.Errorf("failed to add entries to hosts file: %w", err) + } + return nil +} + +// AddIfExists will add the given entries only if one of the existsEntries +// is in the hosts file. This API is required for podman network connect. +// Since we want to add the same host name for each network ip we want to +// add duplicates and the normal Add() call prevents us from doing so. +// However since we also do not want to overwrite potential entries that +// were added by users manually we first have to check if there are the +// current expected entries in the file. Note that this will only check +// for one match not all. It will also only check that the ip and one of +// the hostnames match like Remove(). +func AddIfExists(file string, existsEntries, newEntries HostEntries) error { + if err := addIfExists(file, existsEntries, newEntries); err != nil { + return fmt.Errorf("failed to add entries to hosts file: %w", err) + } + return nil +} + +// Remove will remove the given entries from the file. An entry will be +// removed when the ip and at least one name matches. Not all names have +// to match. If the given entries are not present in the file no error is +// returned. +// Remove is not atomic because it will keep the current file inode. This is +// required to keep bind mounts for containers working. +func Remove(file string, entries HostEntries) error { + if err := remove(file, entries); err != nil { + return fmt.Errorf("failed to remove entries from hosts file: %w", err) + } + return nil +} + +// new see comment on New() +func new(params *Params) error { + entries, err := parseExtraHosts(params.ExtraHosts) + if err != nil { + return err + } + entries2, err := parseHostsFile(params.BaseFile) + if err != nil { + return err + } + entries = append(entries, entries2...) + + // preallocate the slice with enough space for the 3 special entries below + containerIPs := make(HostEntries, 0, len(params.ContainerIPs)+3) + + // if localhost was not added we add it + // https://github.com/containers/podman/issues/11411 + lh := []string{localhost} + l1 := HostEntry{IP: "127.0.0.1", Names: lh} + l2 := HostEntry{IP: "::1", Names: lh} + containerIPs = append(containerIPs, l1, l2) + if params.HostContainersInternalIP != "" { + e := HostEntry{IP: params.HostContainersInternalIP, Names: []string{hostContainersInternal}} + containerIPs = append(containerIPs, e) + } + containerIPs = append(containerIPs, params.ContainerIPs...) + + if err := writeHostFile(params.TargetFile, entries, containerIPs); err != nil { + return err + } + return nil +} + +// add see comment on Add() +func add(file string, entries HostEntries) error { + currentEntries, err := parseHostsFile(file) + if err != nil { + return err + } + + names := make(map[string]struct{}) + for _, entry := range currentEntries { + for _, name := range entry.Names { + names[name] = struct{}{} + } + } + + // open file in append mode since we only add, we do not have to write existing entries again + f, err := os.OpenFile(file, os.O_WRONLY|os.O_APPEND, 0o644) + if err != nil { + return err + } + defer f.Close() + + return addEntriesIfNotExists(f, entries, names) +} + +// addIfExists see comment on AddIfExists() +func addIfExists(file string, existsEntries, newEntries HostEntries) error { + // special case when there are no existing entries do a normal add + // this can happen when we connect a network which was not connected + // to any other networks before + if len(existsEntries) == 0 { + return add(file, newEntries) + } + + currentEntries, err := parseHostsFile(file) + if err != nil { + return err + } + + for _, entry := range currentEntries { + if !checkIfEntryExists(entry, existsEntries) { + // keep looking for existing entries + continue + } + // if we have a matching existing entry add the new entries + // open file in append mode since we only add, we do not have to write existing entries again + f, err := os.OpenFile(file, os.O_WRONLY|os.O_APPEND, 0o644) + if err != nil { + return err + } + defer f.Close() + + for _, e := range newEntries { + if _, err = f.WriteString(formatLine(e.IP, e.Names)); err != nil { + return err + } + } + return nil + } + // no match found is no error + return nil +} + +// remove see comment on Remove() +func remove(file string, entries HostEntries) error { + currentEntries, err := parseHostsFile(file) + if err != nil { + return err + } + + f, err := os.Create(file) + if err != nil { + return err + } + defer f.Close() + + for _, entry := range currentEntries { + if checkIfEntryExists(entry, entries) { + continue + } + if _, err = f.WriteString(formatLine(entry.IP, entry.Names)); err != nil { + return err + } + } + return nil +} + +func checkIfEntryExists(current HostEntry, entries HostEntries) bool { + // check if the current entry equals one of the given entries + for _, rm := range entries { + if current.IP == rm.IP { + // it is enough if one of the names match, in this case we remove the full entry + for _, name := range current.Names { + if util.StringInSlice(name, rm.Names) { + return true + } + } + } + } + return false +} + +// parseExtraHosts converts a slice of "name:ip" string to entries. +// Because podman and buildah both store the extra hosts in this format +// we convert it here instead of having to this on the caller side. +func parseExtraHosts(extraHosts []string) (HostEntries, error) { + entries := make(HostEntries, 0, len(extraHosts)) + for _, entry := range extraHosts { + values := strings.SplitN(entry, ":", 2) + if len(values) != 2 { + return nil, fmt.Errorf("unable to parse host entry %q: incorrect format", entry) + } + if values[0] == "" { + return nil, fmt.Errorf("hostname in host entry %q is empty", entry) + } + if values[1] == "" { + return nil, fmt.Errorf("IP address in host entry %q is empty", entry) + } + e := HostEntry{IP: values[1], Names: []string{values[0]}} + entries = append(entries, e) + } + return entries, nil +} + +// parseHostsFile parses a given host file and returns all entries in it. +// Note that this will remove all comments and spaces. +func parseHostsFile(file string) (HostEntries, error) { + // empty file is valid, in this case we skip adding entries from the file + if file == "" { + return nil, nil + } + + f, err := os.Open(file) + if err != nil { + // do not error when the default hosts file does not exists + // https://github.com/containers/podman/issues/12667 + if errors.Is(err, os.ErrNotExist) && file == DefaultHostsFile { + return nil, nil + } + return nil, err + } + defer f.Close() + + entries := HostEntries{} + scanner := bufio.NewScanner(f) + for scanner.Scan() { + // split of the comments + line := scanner.Text() + if c := strings.IndexByte(line, '#'); c != -1 { + line = line[:c] + } + fields := strings.Fields(line) + // if we only have a ip without names we skip it + if len(fields) < 2 { + continue + } + + e := HostEntry{IP: fields[0], Names: fields[1:]} + entries = append(entries, e) + } + + return entries, scanner.Err() +} + +// writeHostFile write the entries to the given file +func writeHostFile(file string, userEntries, containerIPs HostEntries) error { + f, err := os.Create(file) + if err != nil { + return err + } + defer f.Close() + + names := make(map[string]struct{}) + for _, entry := range userEntries { + for _, name := range entry.Names { + names[name] = struct{}{} + } + if _, err = f.WriteString(formatLine(entry.IP, entry.Names)); err != nil { + return err + } + } + + return addEntriesIfNotExists(f, containerIPs, names) +} + +// addEntriesIfNotExists only adds the entries for names that are not already +// in the hosts file, otherwise we start overwriting user entries +func addEntriesIfNotExists(f io.StringWriter, containerIPs HostEntries, names map[string]struct{}) error { + for _, entry := range containerIPs { + freeNames := make([]string, 0, len(entry.Names)) + for _, name := range entry.Names { + if _, ok := names[name]; !ok { + freeNames = append(freeNames, name) + } + } + if len(freeNames) > 0 { + if _, err := f.WriteString(formatLine(entry.IP, freeNames)); err != nil { + return err + } + } + } + return nil +} + +// formatLine converts the given ip and names to a valid hosts line. +// The returned string includes the newline. +func formatLine(ip string, names []string) string { + return ip + "\t" + strings.Join(names, " ") + "\n" +} diff --git a/libnetwork/etchosts/hosts_test.go b/libnetwork/etchosts/hosts_test.go new file mode 100644 index 000000000..23a4682af --- /dev/null +++ b/libnetwork/etchosts/hosts_test.go @@ -0,0 +1,535 @@ +package etchosts + +import ( + "io/ioutil" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "golang.org/x/sys/unix" +) + +const baseFileContent1Spaces = `127.0.0.1 localhost localhost.localdomain localhost4 localhost4.localdomain4 +::1 localhost localhost.localdomain localhost6 localhost6.localdomain6 +` + +const baseFileContent1Tabs = `127.0.0.1 localhost localhost.localdomain localhost4 localhost4.localdomain4 +::1 localhost localhost.localdomain localhost6 localhost6.localdomain6 +` + +const baseFileContent1Mixed = `127.0.0.1 localhost localhost.localdomain localhost4 localhost4.localdomain4 +::1 localhost localhost.localdomain localhost6 localhost6.localdomain6 +` + +const targetFileContent1 = `127.0.0.1 localhost localhost.localdomain localhost4 localhost4.localdomain4 +::1 localhost localhost.localdomain localhost6 localhost6.localdomain6 +` + +const baseFileContent2 = `127.0.0.1 localhost +::1 localhost +1.1.1.1 name1 +2.2.2.2 name2 +` + +const targetFileContent2 = `127.0.0.1 localhost +::1 localhost +1.1.1.1 name1 +2.2.2.2 name2 +` + +const baseFileContent3Comments1 = `127.0.0.1 localhost #localhost +::1 localhost +# with comments +` + +const baseFileContent3Comments2 = `#localhost +` + +const targetFileContent3 = `127.0.0.1 localhost +::1 localhost +` + +const baseFileContent4 = `127.0.0.1 localhost +` + +const targetFileContent4 = `1.1.1.1 name1 +2.2.2.2 name2 +127.0.0.1 localhost +` + +const targetFileContent5 = `1.1.1.1 name1 +2.2.2.2 name2 +127.0.1.1 localhost +` + +const baseFileContent6 = `127.0.0.1 localhost +::1 localhost +1.1.1.1 host.containers.internal +` + +const targetFileContent6 = `127.0.0.1 localhost +::1 localhost +1.1.1.1 host.containers.internal +` + +const baseFileContent7 = ` +1.1.1.1 +` + +const targetFileContent7 = `127.0.0.1 localhost +::1 localhost +` + +func TestNew(t *testing.T) { + tests := []struct { + name string + // only used to trigger fails for not existing files + baseFileName string + baseFileContent string + noWriteBaseFile bool + extraHosts []string + containerIPs HostEntries + hostContainersInternal string + expectedTargetFileContent string + wantErrString string + }{ + { + name: "with spaces", + baseFileContent: baseFileContent1Spaces, + expectedTargetFileContent: targetFileContent1, + }, + { + name: "with tabs", + baseFileContent: baseFileContent1Tabs, + expectedTargetFileContent: targetFileContent1, + }, + { + name: "with spaces and tabs", + baseFileContent: baseFileContent1Mixed, + expectedTargetFileContent: targetFileContent1, + }, + { + name: "with more entries", + baseFileContent: baseFileContent2, + expectedTargetFileContent: targetFileContent2, + }, + { + name: "with no entries", + baseFileContent: "", + expectedTargetFileContent: targetFileContent3, + }, + { + name: "base file is empty", + baseFileContent: "", + noWriteBaseFile: true, + expectedTargetFileContent: targetFileContent3, + }, + { + name: "with comments 1", + baseFileContent: baseFileContent3Comments1, + expectedTargetFileContent: targetFileContent3, + }, + { + name: "with comments 2", + baseFileContent: baseFileContent3Comments2, + expectedTargetFileContent: targetFileContent3, + }, + { + name: "extra hosts", + baseFileContent: baseFileContent4, + extraHosts: []string{"name1:1.1.1.1", "name2:2.2.2.2"}, + expectedTargetFileContent: targetFileContent4, + }, + { + name: "extra hosts with localhost", + baseFileContent: "", + extraHosts: []string{"name1:1.1.1.1", "name2:2.2.2.2", "localhost:127.0.1.1"}, + expectedTargetFileContent: targetFileContent5, + }, + { + name: "with more entries and extra host", + baseFileContent: baseFileContent2, + extraHosts: []string{"name1:1.1.1.1"}, + expectedTargetFileContent: "1.1.1.1\tname1\n" + targetFileContent2, + }, + { + name: "with more entries and extra host", + baseFileContent: baseFileContent2, + extraHosts: []string{"name1:1.1.1.1"}, + expectedTargetFileContent: "1.1.1.1\tname1\n" + targetFileContent2, + }, + { + name: "with more entries and extra host", + baseFileContent: baseFileContent2, + extraHosts: []string{"name1:1.1.1.1"}, + expectedTargetFileContent: "1.1.1.1\tname1\n" + targetFileContent2, + }, + { + name: "container ips", + baseFileContent: baseFileContent1Spaces, + containerIPs: []HostEntry{{IP: "1.2.3.4", Names: []string{"conname", "hostname"}}}, + expectedTargetFileContent: targetFileContent1 + "1.2.3.4\tconname hostname\n", + }, + { + name: "container ips 2", + baseFileContent: baseFileContent1Spaces, + containerIPs: []HostEntry{ + {IP: "1.2.3.4", Names: []string{"conname", "hostname"}}, + {IP: "fd::1", Names: []string{"conname", "hostname"}}, + }, + expectedTargetFileContent: targetFileContent1 + "1.2.3.4\tconname hostname\nfd::1\tconname hostname\n", + }, + { + name: "container ips and extra hosts", + baseFileContent: baseFileContent1Spaces, + extraHosts: []string{"name1:1.1.1.1"}, + containerIPs: []HostEntry{{IP: "1.2.3.4", Names: []string{"conname", "hostname"}}}, + expectedTargetFileContent: "1.1.1.1\tname1\n" + targetFileContent1 + "1.2.3.4\tconname hostname\n", + }, + { + name: "container ips and extra hosts 2", + baseFileContent: baseFileContent2, + extraHosts: []string{"name1:1.1.1.1"}, + containerIPs: []HostEntry{{IP: "1.2.3.4", Names: []string{"conname", "hostname"}}}, + expectedTargetFileContent: "1.1.1.1\tname1\n" + targetFileContent2 + "1.2.3.4\tconname hostname\n", + }, + { + name: "container ip name is not added when name is already present", + baseFileContent: baseFileContent2, + containerIPs: []HostEntry{{IP: "1.2.3.4", Names: []string{"name1", "hostname"}}}, + expectedTargetFileContent: targetFileContent2 + "1.2.3.4\thostname\n", + }, + { + name: "container ip name is not added when name is already present 2", + baseFileContent: baseFileContent2, + containerIPs: []HostEntry{{IP: "1.2.3.4", Names: []string{"name1"}}}, + expectedTargetFileContent: targetFileContent2, + }, + { + name: "container ip name is not added when name is already present in extra hosts", + baseFileContent: baseFileContent1Spaces, + extraHosts: []string{"somename:1.1.1.1"}, + containerIPs: []HostEntry{{IP: "1.2.3.4", Names: []string{"somename", "hostname"}}}, + expectedTargetFileContent: "1.1.1.1\tsomename\n" + targetFileContent1 + "1.2.3.4\thostname\n", + }, + { + name: "with host.containers.internal ip", + baseFileContent: baseFileContent1Spaces, + hostContainersInternal: "10.0.0.1", + expectedTargetFileContent: targetFileContent1 + "10.0.0.1\thost.containers.internal\n", + }, + { + name: "host.containers.internal not added when already present in extra hosts", + baseFileContent: baseFileContent1Spaces, + extraHosts: []string{"host.containers.internal:1.1.1.1"}, + hostContainersInternal: "10.0.0.1", + expectedTargetFileContent: "1.1.1.1\thost.containers.internal\n" + targetFileContent1, + }, + { + name: "host.containers.internal not added when already present in base hosts", + baseFileContent: baseFileContent6, + hostContainersInternal: "10.0.0.1", + expectedTargetFileContent: targetFileContent6, + }, + { + name: "invalid hosts content", + baseFileContent: baseFileContent7, + expectedTargetFileContent: targetFileContent7, + }, + // errors + { + name: "base file does not exists", + baseFileName: "does/not/exists123456789", + noWriteBaseFile: true, + wantErrString: "no such file or directory", + }, + { + name: "invalid extra hosts hostname empty", + baseFileContent: baseFileContent1Spaces, + extraHosts: []string{":1.1.1.1"}, + wantErrString: "hostname in host entry \":1.1.1.1\" is empty", + }, + { + name: "invalid extra hosts empty ip", + baseFileContent: baseFileContent1Spaces, + extraHosts: []string{"name:"}, + wantErrString: "IP address in host entry \"name:\" is empty", + }, + { + name: "invalid extra hosts empty ip", + baseFileContent: baseFileContent1Spaces, + extraHosts: []string{"name:"}, + wantErrString: "IP address in host entry \"name:\" is empty", + }, + { + name: "invalid extra hosts format", + baseFileContent: baseFileContent1Spaces, + extraHosts: []string{"name"}, + wantErrString: "unable to parse host entry \"name\": incorrect format", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + baseHostFile := tt.baseFileName + if !tt.noWriteBaseFile { + f, err := ioutil.TempFile(t.TempDir(), "basehosts") + assert.NoErrorf(t, err, "failed to create base host file: %v", err) + defer f.Close() + baseHostFile = f.Name() + _, err = f.WriteString(tt.baseFileContent) + assert.NoError(t, err, "failed to write base host file: %v", err) + } + + targetFile := filepath.Join(t.TempDir(), "target") + + params := &Params{ + BaseFile: baseHostFile, + ExtraHosts: tt.extraHosts, + ContainerIPs: tt.containerIPs, + HostContainersInternalIP: tt.hostContainersInternal, + TargetFile: targetFile, + } + + err := New(params) + if tt.wantErrString != "" { + assert.ErrorContains(t, err, tt.wantErrString) + return + } else { + assert.NoError(t, err, "New() failed") + } + + content, err := ioutil.ReadFile(targetFile) + assert.NoErrorf(t, err, "failed to read target host file: %v", err) + assert.Equal(t, tt.expectedTargetFileContent, string(content), "check hosts content") + }) + } +} + +func TestAdd(t *testing.T) { + tests := []struct { + name string + baseFileContent string + entries HostEntries + expectedTargetFileContent string + wantErrString string + }{ + { + name: "add entry", + baseFileContent: baseFileContent1Mixed, + entries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: targetFileContent1 + "1.1.1.1\tname1 name2\n", + }, + { + name: "add two entries", + baseFileContent: baseFileContent1Mixed, + entries: HostEntries{ + {IP: "1.1.1.1", Names: []string{"name1", "name2"}}, + {IP: "1.1.1.2", Names: []string{"name3", "name4"}}, + }, + expectedTargetFileContent: targetFileContent1 + "1.1.1.1\tname1 name2\n1.1.1.2\tname3 name4\n", + }, + { + name: "add entry to empty file", + baseFileContent: "", + entries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: "1.1.1.1\tname1 name2\n", + }, + { + name: "add entry which already exists", + baseFileContent: baseFileContent2, + entries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: targetFileContent2, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + f, err := ioutil.TempFile(t.TempDir(), "hosts") + assert.NoErrorf(t, err, "failed to create base host file: %v", err) + defer f.Close() + hostFile := f.Name() + _, err = f.WriteString(tt.baseFileContent) + assert.NoError(t, err, "failed to write base host file: %v", err) + + var st unix.Stat_t + err = unix.Stat(hostFile, &st) + assert.NoError(t, err, "stat host file: %v", err) + + err = Add(hostFile, tt.entries) + if tt.wantErrString != "" { + assert.ErrorContains(t, err, tt.wantErrString) + return + } else { + assert.NoError(t, err, "Add() failed") + } + + content, err := ioutil.ReadFile(hostFile) + assert.NoErrorf(t, err, "failed to read host file: %v", err) + assert.Equal(t, tt.expectedTargetFileContent, string(content), "check hosts content") + + var st2 unix.Stat_t + err = unix.Stat(hostFile, &st2) + assert.NoError(t, err, "stat host file: %v", err) + assert.Equal(t, st.Ino, st2.Ino, "inode before and after Add() must match") + }) + } +} + +func TestAddIfExists(t *testing.T) { + tests := []struct { + name string + baseFileContent string + existsEntries HostEntries + newEntries HostEntries + expectedTargetFileContent string + wantErrString string + }{ + { + name: "add entry", + baseFileContent: baseFileContent1Mixed, + newEntries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: targetFileContent1 + "1.1.1.1\tname1 name2\n", + }, + { + name: "add entry with existing entries match", + baseFileContent: baseFileContent1Mixed, + existsEntries: HostEntries{{IP: "::1", Names: []string{"localhost"}}}, + newEntries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: targetFileContent1 + "1.1.1.1\tname1 name2\n", + }, + { + name: "existing entries with no match should not add", + baseFileContent: baseFileContent1Mixed, + existsEntries: HostEntries{{IP: "::1", Names: []string{"name"}}}, + newEntries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: targetFileContent1, + }, + { + name: "add two entries", + baseFileContent: baseFileContent1Mixed, + newEntries: HostEntries{ + {IP: "1.1.1.1", Names: []string{"name1", "name2"}}, + {IP: "1.1.1.2", Names: []string{"name3", "name4"}}, + }, + expectedTargetFileContent: targetFileContent1 + "1.1.1.1\tname1 name2\n1.1.1.2\tname3 name4\n", + }, + { + name: "add two entries with existing entries match", + baseFileContent: baseFileContent1Mixed, + existsEntries: HostEntries{{IP: "127.0.0.1", Names: []string{"localhost"}}}, + newEntries: HostEntries{ + {IP: "1.1.1.1", Names: []string{"name1", "name2"}}, + {IP: "1.1.1.2", Names: []string{"name3", "name4"}}, + }, + expectedTargetFileContent: targetFileContent1 + "1.1.1.1\tname1 name2\n1.1.1.2\tname3 name4\n", + }, + { + name: "add entry to empty file", + baseFileContent: "", + newEntries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: "1.1.1.1\tname1 name2\n", + }, + { + name: "add entry to empty file with no existing match", + baseFileContent: "", + existsEntries: HostEntries{{IP: "127.0.0.1", Names: []string{"localhost"}}}, + newEntries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: "", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + f, err := ioutil.TempFile(t.TempDir(), "hosts") + assert.NoErrorf(t, err, "failed to create base host file: %v", err) + defer f.Close() + hostFile := f.Name() + _, err = f.WriteString(tt.baseFileContent) + assert.NoError(t, err, "failed to write base host file: %v", err) + + var st unix.Stat_t + err = unix.Stat(hostFile, &st) + assert.NoError(t, err, "stat host file: %v", err) + + err = AddIfExists(hostFile, tt.existsEntries, tt.newEntries) + if tt.wantErrString != "" { + assert.ErrorContains(t, err, tt.wantErrString) + return + } else { + assert.NoError(t, err, "AddIfExists() failed") + } + + content, err := ioutil.ReadFile(hostFile) + assert.NoErrorf(t, err, "failed to read host file: %v", err) + assert.Equal(t, tt.expectedTargetFileContent, string(content), "check hosts content") + + var st2 unix.Stat_t + err = unix.Stat(hostFile, &st2) + assert.NoError(t, err, "stat host file: %v", err) + assert.Equal(t, st.Ino, st2.Ino, "inode before and after AddIfExists() must match") + }) + } +} + +func TestRemove(t *testing.T) { + tests := []struct { + name string + baseFileContent string + entries HostEntries + expectedTargetFileContent string + }{ + { + name: "remove entry which does not exists", + baseFileContent: baseFileContent1Spaces, + entries: HostEntries{{IP: "1.1.1.1", Names: []string{"name1", "name2"}}}, + expectedTargetFileContent: targetFileContent1, + }, + { + name: "do not remove entry when only ip matches", + baseFileContent: baseFileContent2, + entries: HostEntries{{IP: "1.1.1.1", Names: []string{"new1", "new2"}}}, + expectedTargetFileContent: targetFileContent2, + }, + { + name: "remove two entries", + baseFileContent: baseFileContent2, + entries: HostEntries{ + {IP: "1.1.1.1", Names: []string{"name1"}}, + {IP: "2.2.2.2", Names: []string{"name2", "name4"}}, + }, + expectedTargetFileContent: targetFileContent3, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + f, err := ioutil.TempFile(t.TempDir(), "hosts") + assert.NoErrorf(t, err, "failed to create base host file: %v", err) + defer f.Close() + hostFile := f.Name() + _, err = f.WriteString(tt.baseFileContent) + assert.NoError(t, err, "failed to write base host file: %v", err) + + var st unix.Stat_t + err = unix.Stat(hostFile, &st) + assert.NoError(t, err, "stat host file: %v", err) + + err = Remove(hostFile, tt.entries) + assert.NoError(t, err, "Remove() failed") + + content, err := ioutil.ReadFile(hostFile) + assert.NoErrorf(t, err, "failed to read host file: %v", err) + assert.Equal(t, tt.expectedTargetFileContent, string(content), "check hosts content") + + var st2 unix.Stat_t + err = unix.Stat(hostFile, &st2) + assert.NoError(t, err, "stat host file: %v", err) + assert.Equal(t, st.Ino, st2.Ino, "inode before and after Remove() must match") + }) + } +} From b5c27e2817f39b7c62441dd58cda5d3b3d3f8946 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 11 Apr 2022 15:21:23 +0200 Subject: [PATCH 2/5] add base_hosts_file field to containers.conf base_hosts_file can be used to overwrite the default base host file /etc/hosts which is used to copy hosts entries from this file into the containers /etc/hosts file. As special value "image" can be used to copy the entries from the image hosts file or "none" to not use a base file at all. IF the value is empty we should use /etc/hosts as default. Ref https://github.com/containers/podman/issues/13277 Ref https://github.com/containers/podman/issues/13748 Signed-off-by: Paul Holzinger --- docs/containers.conf.5.md | 7 +++++++ libnetwork/etchosts/hosts.go | 5 ++--- pkg/config/config.go | 7 +++++++ pkg/config/config_test.go | 2 ++ pkg/config/containers.conf | 7 +++++++ pkg/config/default.go | 3 +++ pkg/config/testdata/containers_default.conf | 2 ++ 7 files changed, 30 insertions(+), 3 deletions(-) diff --git a/docs/containers.conf.5.md b/docs/containers.conf.5.md index a4f872a94..af77070ec 100644 --- a/docs/containers.conf.5.md +++ b/docs/containers.conf.5.md @@ -59,6 +59,13 @@ Example: "run.oci.keep_original_groups=1" Used to change the name of the default AppArmor profile of container engines. The default profile name is "container-default". +**base_hosts_file**="" + +The hosts entries from the base hosts file are added to the containers hosts +file. This must be either an absolute path or as special values "image" which +uses the hosts file from the container image or "none" which means +no base hosts file is used. The default is "" which will use /etc/hosts. + **cgroups**="enabled" Determines whether the container will create CGroups. diff --git a/libnetwork/etchosts/hosts.go b/libnetwork/etchosts/hosts.go index 1788de2f3..ce248a181 100644 --- a/libnetwork/etchosts/hosts.go +++ b/libnetwork/etchosts/hosts.go @@ -8,12 +8,11 @@ import ( "os" "strings" + "github.com/containers/common/pkg/config" "github.com/containers/common/pkg/util" ) const ( - // DefaultHostsFile is the default path to the hosts file - DefaultHostsFile = "/etc/hosts" hostContainersInternal = "host.containers.internal" localhost = "localhost" ) @@ -265,7 +264,7 @@ func parseHostsFile(file string) (HostEntries, error) { if err != nil { // do not error when the default hosts file does not exists // https://github.com/containers/podman/issues/12667 - if errors.Is(err, os.ErrNotExist) && file == DefaultHostsFile { + if errors.Is(err, os.ErrNotExist) && file == config.DefaultHostsFile { return nil, nil } return nil, err diff --git a/pkg/config/config.go b/pkg/config/config.go index b28c527bc..9ce5316fb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -95,6 +95,13 @@ type ContainersConfig struct { // Annotation to add to all containers Annotations []string `toml:"annotations,omitempty"` + // BaseHostsFile is the path to a hosts file, the entries from this file + // are added to the containers hosts file. As special value "image" is + // allowed which uses the /etc/hosts file from within the image and "none" + // which uses no base file at all. If it is empty we should default + // to /etc/hosts. + BaseHostsFile string `toml:"base_hosts_file,omitempty"` + // Default way to create a cgroup namespace for the container CgroupNS string `toml:"cgroupns,omitempty"` diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index fe0274c31..f64081635 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -27,6 +27,7 @@ var _ = Describe("Config", func() { // Then gomega.Expect(err).To(gomega.BeNil()) gomega.Expect(defaultConfig.Containers.ApparmorProfile).To(gomega.Equal(apparmor.Profile)) + gomega.Expect(defaultConfig.Containers.BaseHostsFile).To(gomega.Equal("")) gomega.Expect(defaultConfig.Containers.PidsLimit).To(gomega.BeEquivalentTo(2048)) gomega.Expect(defaultConfig.Engine.ServiceTimeout).To(gomega.BeEquivalentTo(5)) gomega.Expect(defaultConfig.NetNS()).To(gomega.BeEquivalentTo("private")) @@ -375,6 +376,7 @@ image_copy_tmp_dir="storage"` gomega.Expect(err).To(gomega.BeNil()) gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal("container-default")) gomega.Expect(config.Containers.PidsLimit).To(gomega.BeEquivalentTo(2048)) + gomega.Expect(config.Containers.BaseHostsFile).To(gomega.BeEquivalentTo("/etc/hosts2")) }) It("contents of passed-in file should override others", func() { diff --git a/pkg/config/containers.conf b/pkg/config/containers.conf index f069c531d..7f5d9a34f 100644 --- a/pkg/config/containers.conf +++ b/pkg/config/containers.conf @@ -26,6 +26,13 @@ # #apparmor_profile = "container-default" +# The hosts entries from the base hosts file are added to the containers hosts +# file. This must be either an absolute path or as special values "image" which +# uses the hosts file from the container image or "none" which means +# no base hosts file is used. The default is "" which will use /etc/hosts. +# +#base_hosts_file = "" + # Default way to to create a cgroup namespace for the container # Options are: # `private` Create private Cgroup Namespace for the container. diff --git a/pkg/config/default.go b/pkg/config/default.go index 275f67cbf..0acaedd44 100644 --- a/pkg/config/default.go +++ b/pkg/config/default.go @@ -122,6 +122,8 @@ const ( CgroupfsCgroupsManager = "cgroupfs" // DefaultApparmorProfile specifies the default apparmor profile for the container. DefaultApparmorProfile = apparmor.Profile + // DefaultHostsFile is the default path to the hosts file + DefaultHostsFile = "/etc/hosts" // SystemdCgroupsManager represents systemd native cgroup manager SystemdCgroupsManager = "systemd" // DefaultLogSizeMax is the default value for the maximum log size @@ -189,6 +191,7 @@ func DefaultConfig() (*Config, error) { Volumes: []string{}, Annotations: []string{}, ApparmorProfile: DefaultApparmorProfile, + BaseHostsFile: "", CgroupNS: cgroupNS, Cgroups: "enabled", DefaultCapabilities: DefaultCapabilities, diff --git a/pkg/config/testdata/containers_default.conf b/pkg/config/testdata/containers_default.conf index 25f8a1c54..e7eb9b6be 100644 --- a/pkg/config/testdata/containers_default.conf +++ b/pkg/config/testdata/containers_default.conf @@ -17,6 +17,8 @@ devices = [ # profile name is "container-default". apparmor_profile = "container-default" +base_hosts_file = "/etc/hosts2" + # List of default capabilities for containers. If it is empty or commented out, # only the capabilities defined in the containers json file by the user/kube # will be added. From 56484929a53c4b030953e64f30ac2b00cfa0701d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 11 Apr 2022 16:10:40 +0200 Subject: [PATCH 3/5] add host_containers_internal_ip to containers.conf Set the ip for the host.containers.internal entry in the containers /etc/hosts file. This can be set to "none" to disable adding this entry. By default it will automatically choose the host ip. Also add a function to get the correct host.containers.internal ip. This should be used by podman and buildah and then passed to the New() function. Ref https://github.com/containers/podman/issues/13224 Signed-off-by: Paul Holzinger --- docs/containers.conf.5.md | 10 +++ libnetwork/etchosts/ip.go | 76 +++++++++++++++++++++ pkg/config/config.go | 3 + pkg/config/config_test.go | 1 + pkg/config/containers.conf | 11 ++- pkg/config/testdata/containers_default.conf | 2 + 6 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 libnetwork/etchosts/ip.go diff --git a/docs/containers.conf.5.md b/docs/containers.conf.5.md index af77070ec..9148451a3 100644 --- a/docs/containers.conf.5.md +++ b/docs/containers.conf.5.md @@ -150,6 +150,16 @@ environment variables to the container. Pass all host environment variables into the container. +**host_containers_internal_ip**="" + +Set the ip for the host.containers.internal entry in the containers /etc/hosts +file. This can be set to "none" to disable adding this entry. By default it +will automatically choose the host ip. + +NOTE: When using podman machine this entry will never be added to the containers +hosts file instead the gvproxy dns resolver will resolve this hostname. Therefore +it is not possible to disable the entry in this case. + **http_proxy**=true Default proxy environment variables will be passed into the container. diff --git a/libnetwork/etchosts/ip.go b/libnetwork/etchosts/ip.go new file mode 100644 index 000000000..6962e203d --- /dev/null +++ b/libnetwork/etchosts/ip.go @@ -0,0 +1,76 @@ +package etchosts + +import ( + "net" + + "github.com/containers/common/libnetwork/types" + "github.com/containers/common/libnetwork/util" + "github.com/containers/common/pkg/config" + "github.com/containers/storage/pkg/unshare" +) + +// GetHostContainersInternalIP return 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 { + switch conf.Containers.HostContainersInternalIP { + case "": + // if empty (default) we will automatically choose one below + // if machine we let the gvproxy dns server handle the dns name so do not add it + if conf.Engine.MachineEnabled { + return "" + } + case "none": + return "" + default: + return 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 getLocalIP() + } + for net, status := range netStatus { + network, err := 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 { + continue + } + for _, netInt := range status.Interfaces { + for _, netAddress := range netInt.Subnets { + if netAddress.Gateway != nil { + if util.IsIPv4(netAddress.Gateway) { + return netAddress.Gateway.String() + } + // ipv6 address but keep looking since we prefer to use ipv4 + ip = netAddress.Gateway.String() + } + } + } + } + if ip != "" { + return ip + } + return getLocalIP() +} + +// getLocalIP returns the non loopback local IP of the host +func getLocalIP() string { + addrs, err := net.InterfaceAddrs() + if err != nil { + return "" + } + ip := "" + for _, address := range addrs { + // check the address type and if it is not a loopback the display it + if ipnet, ok := address.(*net.IPNet); ok && ipnet.IP.IsGlobalUnicast() { + if util.IsIPv4(ipnet.IP) { + return ipnet.IP.String() + } + // if ipv6 we keep looking for an ipv4 address + ip = ipnet.IP.String() + } + } + return ip +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 9ce5316fb..319b8d153 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -143,6 +143,9 @@ type ContainersConfig struct { // EnvHost Pass all host environment variables into the container. EnvHost bool `toml:"env_host,omitempty"` + // HostContainersInternalIP is used to set a specific host.containers.internal ip. + HostContainersInternalIP string `toml:"host_containers_internal_ip,omitempty"` + // HTTPProxy is the proxy environment variable list to apply to container process HTTPProxy bool `toml:"http_proxy,omitempty"` diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index f64081635..bf705c52d 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -377,6 +377,7 @@ image_copy_tmp_dir="storage"` gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal("container-default")) gomega.Expect(config.Containers.PidsLimit).To(gomega.BeEquivalentTo(2048)) gomega.Expect(config.Containers.BaseHostsFile).To(gomega.BeEquivalentTo("/etc/hosts2")) + gomega.Expect(config.Containers.HostContainersInternalIP).To(gomega.BeEquivalentTo("1.2.3.4")) }) It("contents of passed-in file should override others", func() { diff --git a/pkg/config/containers.conf b/pkg/config/containers.conf index 7f5d9a34f..afb913d61 100644 --- a/pkg/config/containers.conf +++ b/pkg/config/containers.conf @@ -121,6 +121,16 @@ default_sysctls = [ # #env_host = false +# Set the ip for the host.containers.internal entry in the containers /etc/hosts +# file. This can be set to "none" to disable adding this entry. By default it +# will automatically choose the host ip. +# +# NOTE: When using podman machine this entry will never be added to the containers +# hosts file instead the gvproxy dns resolver will resolve this hostname. Therefore +# it is not possible to disable the entry in this case. +# +#host_containers_internal_ip = "" + # Default proxy environment variables passed into the container. # The environment variables passed in include: # http_proxy, https_proxy, ftp_proxy, no_proxy, and the upper case versions of @@ -651,4 +661,3 @@ default_sysctls = [ # TOML does not provide a way to end a table other than a further table being # defined, so every key hereafter will be part of [machine] and not the # main config. - diff --git a/pkg/config/testdata/containers_default.conf b/pkg/config/testdata/containers_default.conf index e7eb9b6be..ca4948c2f 100644 --- a/pkg/config/testdata/containers_default.conf +++ b/pkg/config/testdata/containers_default.conf @@ -63,6 +63,8 @@ env = [ # Run an init inside the container that forwards signals and reaps processes. init = false +host_containers_internal_ip = "1.2.3.4" + # proxy environment variables are passed into the container http_proxy = false From 88163a6db70c6d16376ae6b3c1c2a4163a48b5fd Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 13 Apr 2022 15:21:03 +0200 Subject: [PATCH 4/5] libnetwork/etchosts: add GetNetworkHostEntries() Add function to get all host entries from a given network status. Signed-off-by: Paul Holzinger --- libnetwork/etchosts/ip.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/libnetwork/etchosts/ip.go b/libnetwork/etchosts/ip.go index 6962e203d..3d14b7147 100644 --- a/libnetwork/etchosts/ip.go +++ b/libnetwork/etchosts/ip.go @@ -74,3 +74,18 @@ func getLocalIP() string { } return ip } + +// GetNetworkHostEntries returns HostEntries for all ips in the network status +// with the given hostnames. +func GetNetworkHostEntries(netStatus map[string]types.StatusBlock, names ...string) HostEntries { + hostEntries := make(HostEntries, 0, len(netStatus)) + for _, status := range netStatus { + for _, netInt := range status.Interfaces { + for _, netAddress := range netInt.Subnets { + e := HostEntry{IP: netAddress.IPNet.IP.String(), Names: names} + hostEntries = append(hostEntries, e) + } + } + } + return hostEntries +} From 7cb3f0d8ce0d672d57c71ed87bd5f1fe4723b8ef Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 14 Apr 2022 16:27:44 +0200 Subject: [PATCH 5/5] libnetwork/etchosts: add GetBaseHostFile() Add helper function to convert the base_hosts_file config value to a actual path. It is important to use securejoin to make sure that containers cannot point to a file on the hosts via a symlink. Signed-off-by: Paul Holzinger --- libnetwork/etchosts/util.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 libnetwork/etchosts/util.go diff --git a/libnetwork/etchosts/util.go b/libnetwork/etchosts/util.go new file mode 100644 index 000000000..d78284594 --- /dev/null +++ b/libnetwork/etchosts/util.go @@ -0,0 +1,30 @@ +package etchosts + +import ( + "fmt" + + "github.com/containers/common/pkg/config" + securejoin "github.com/cyphar/filepath-securejoin" +) + +// GetBaseHostFile return the hosts file which should be used as base. +// The first param should be the config value config.Containers.BaseHostsFile +// The second param should be the root path to the mounted image. This is +// required when the user conf value is set to "image". +func GetBaseHostFile(confValue, imageRoot string) (string, error) { + switch confValue { + case "": + return config.DefaultHostsFile, nil + case "none": + return "", nil + case "image": + // use secure join to prevent problems with symlinks + path, err := securejoin.SecureJoin(imageRoot, config.DefaultHostsFile) + if err != nil { + return "", fmt.Errorf("failed to get /etc/hosts path in image: %w", err) + } + return path, nil + default: + return confValue, nil + } +}