From fa6eba9838817ce3d5a1e045f4de6d7f9ffba503 Mon Sep 17 00:00:00 2001 From: George Joseph Date: Sun, 24 Nov 2024 15:31:58 -0700 Subject: [PATCH] Pass container hostname to netavark Passing the hostname allows netavark to include it in DHCP lease requests which, in an environment where DDNS is used, can cause DNS entries to be created automatically. * The current Hostname() function in container.go was updated to check the new `container_name_as_hostname` option in the CONTAINERS table of containers.conf. If set and no hostname was configured for the container, it causes the hostname to be set to a version of the container's name with the characters not valid for a hostname removed. If not set (the default), the original behavior of setting the hostname to the short container ID is preserved. * Because the Hostname() function can return the host's hostname if the container isn't running in a private UTS namespace, and we'd NEVER want to send _that_ in a DHCP request for a container, a new function NetworkHostname() was added which functions like Hostname() except that it will return an empty string instead of the host's hostname if the container is not running in a private UTS namespace. * networking_common.getNetworkOptions() now uses NetworkHostname() to set the ContainerHostname member of the NetworkOptions structure. That member was added to the structure in a corresponding commit in common/libnetwork/types/network.go. * Added test to containers_conf_test.go Signed-off-by: George Joseph --- libpod/container.go | 53 ++++++++++++++-- libpod/networking_common.go | 7 ++- test/e2e/containers_conf_test.go | 102 +++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 7 deletions(-) diff --git a/libpod/container.go b/libpod/container.go index f54b8f35fd..20a7de3b58 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -666,8 +666,12 @@ func (c *Container) RuntimeName() string { // Runtime spec accessors // Unlocked -// Hostname gets the container's hostname -func (c *Container) Hostname() string { +// hostname determines the container's hostname. +// If 'network' is true and the container isn't running in a +// private UTS namespoace, an emoty string will be returned +// instead of the host's hostname because we never want to +// send the host's hostname to a DHCP or DNS server. +func (c *Container) hostname(network bool) string { if c.config.UTSNsCtr != "" { utsNsCtr, err := c.runtime.GetContainer(c.config.UTSNsCtr) if err != nil { @@ -677,25 +681,66 @@ func (c *Container) Hostname() string { } return utsNsCtr.Hostname() } + if c.config.Spec.Hostname != "" { return c.config.Spec.Hostname } - // if the container is not running in a private UTS namespace, - // return the host's hostname. + // If the container is not running in a private UTS namespace, + // return the host's hostname unless 'network' is true in which + // case we return an empty string. privateUTS := c.hasPrivateUTS() if !privateUTS { hostname, err := os.Hostname() if err == nil { + if network { + return "" + } return hostname } + logrus.Errorf("unable to get host's hostname for container %s: %v", c.ID(), err) + return "" + } + + // If container_name_as_hostname is set in the CONTAINERS table in + // containers.conf, use a sanitized version of the container's name + // as the hostname. Since the container name must already match + // the set '[a-zA-Z0-9][a-zA-Z0-9_.-]*', we can just remove any + // underscores and limit it to 253 characters to make it a valid + // hostname. + if c.runtime.config.Containers.ContainerNameAsHostName { + sanitizedHostname := strings.ReplaceAll(c.Name(), "_", ""); + if (len(sanitizedHostname) <= 253) { + return sanitizedHostname + } + return sanitizedHostname[:253] } + + // Otherwise use the container's short ID as the hostname. if len(c.ID()) < 11 { return c.ID() } return c.ID()[:12] } +// Hostname gets the container's hostname +func (c *Container) Hostname() string { + return c.hostname(false) +} + +// If the container isn't running in a private UTS namespace, Hostname() +// will return the host's hostname as the container's hostname. If netavark +// were to try and obtain a DHCP lease with the host's hostname in an environment +// where DDNS was active, bad things could happen. NetworkHostname() on the +// other hand, will return an empty string if the container isn't running +// in a private UTS namespace. +// +// This function should only be used to populate the ContainerHostname member +// of the common.libnetwork.types.NetworkOptions struct. +func (c *Container) NetworkHostname() string { + return c.hostname(true) +} + // WorkingDir returns the containers working dir func (c *Container) WorkingDir() string { if c.config.Spec.Process != nil { diff --git a/libpod/networking_common.go b/libpod/networking_common.go index bac0d48622..b23308fa7c 100644 --- a/libpod/networking_common.go +++ b/libpod/networking_common.go @@ -55,9 +55,10 @@ func (c *Container) getNetworkOptions(networkOpts map[string]types.PerNetworkOpt nameservers = append(nameservers, ip.String()) } opts := types.NetworkOptions{ - ContainerID: c.config.ID, - ContainerName: getNetworkPodName(c), - DNSServers: nameservers, + ContainerID: c.config.ID, + ContainerName: getNetworkPodName(c), + DNSServers: nameservers, + ContainerHostname: c.NetworkHostname(), } opts.PortMappings = c.convertPortMappings() diff --git a/test/e2e/containers_conf_test.go b/test/e2e/containers_conf_test.go index e86db89622..a9b91203e7 100644 --- a/test/e2e/containers_conf_test.go +++ b/test/e2e/containers_conf_test.go @@ -763,4 +763,106 @@ var _ = Describe("Verify podman containers.conf usage", func() { Expect(inspect.OutputToString()).Should(Equal(mode)) } }) + + startContainer := func(params ...string) string { + args := []string{"create"} + for _, param := range params { + if param == "--name" { + args = append(args, "--replace") + break + } + } + args = append(args, params...) + args = append(args, ALPINE, "true") + + result := podmanTest.Podman(args) + result.WaitWithDefaultTimeout() + Expect(result).Should(ExitCleanly()) + container_id := result.OutputToString() + + return container_id + } + + getContainerConfig := func(container_id string, format_param string) string { + inspect := podmanTest.Podman([]string{"inspect", "--format", format_param, container_id}) + inspect.WaitWithDefaultTimeout() + value := inspect.OutputToString() + return value + } + + It("podman containers.conf container_name_as_hostname", func() { + + // With default containers.conf + + // Start container with no options + container_id := startContainer() + hostname := getContainerConfig(container_id, "{{ .Config.Hostname }}") + // Hostname should be the first 12 characters of the container_id + Expect(hostname).To(Equal(container_id[:12])) + + // Start container with name + container_id = startContainer("--name", "cname1") + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + // Hostname should still be the first 12 characters of the container_id + Expect(hostname).To(Equal(container_id[:12])) + + // Start container with just hostname + container_id = startContainer("--hostname", "cname1.dev") + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + // Hostname should now be "cname1.dev" + Expect(hostname).To(Equal("cname1.dev")) + + // Start container with name and hostname + container_id = startContainer("--name", "cname1", "--hostname", "cname1.dev") + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + // Hostname should now be "cname1.dev" + Expect(hostname).To(Equal("cname1.dev")) + + // Create containers.conf override with container_name_as_hostname=true + conffile := filepath.Join(podmanTest.TempDir, "container.conf") + err := os.WriteFile(conffile, []byte("[containers]\ncontainer_name_as_hostname=true\n"), 0755) + Expect(err).ToNot(HaveOccurred()) + os.Setenv("CONTAINERS_CONF_OVERRIDE", conffile) + + // Start container with no options + container_id = startContainer() + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + name := getContainerConfig(container_id, "{{ .Name }}") + // Hostname should be the auto generated container name with '_' removed + Expect(hostname).To(Equal(strings.ReplaceAll(name, "_", ""))) + + // Start container with name + container_id = startContainer("--name", "cname1") + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + // Hostname should be the container name + Expect(hostname).To(Equal("cname1")) + + // Start container with name containing '_' + container_id = startContainer("--name", "cname1_2_3") + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + // Hostname should be the set container name with all '_' removed + Expect(hostname).To(Equal("cname123")) + + // Start container with just hostname + container_id = startContainer("--hostname", "cname1.dev") + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + // Hostname should now be "cname1.dev" + Expect(hostname).To(Equal("cname1.dev")) + + // Start container with name and hostname + container_id = startContainer("--name", "cname1", "--hostname", "cname1.dev") + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + // Hostname should still be "cname1.dev" + Expect(hostname).To(Equal("cname1.dev")) + + // Start container with name = 260 characters + long_hostname := "cnabcdefghijklmnopqrstuvwxyz1234567890.abcdefghijklmnopqrstuvwxyz1234567890.abcdefghijklmnopqrstuvwxyz1234567890.abcdefghijklmnopqrstuvwxyz1234567890.abcdefghijklmnopqrstuvwxyz1234567890.abcdefghijklmnopqrstuvwxyz1234567890.abcdefghijklmnopqrstuvwxyz1234567890" + container_id = startContainer("--name", long_hostname) + hostname = getContainerConfig(container_id, "{{ .Config.Hostname }}") + name = getContainerConfig(container_id, "{{ .Name }}") + // Double check that name actually got set correctly + Expect(name).To(Equal(long_hostname)) + // Hostname should be the container name truncated to 253 characters + Expect(hostname).To(Equal(name[:253])) + }) })