From 84036d3e18506125e4fa7cabcab2371d341c3839 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 13 Nov 2023 12:22:04 +0100 Subject: [PATCH] daemon/config: change DNSConfig.DNS to a []net.IP Use a strong type for the DNS IP-addresses so that we can use flags.IPSliceVar, instead of implementing our own option-type and validation. Behavior should be the same, although error-messages have slightly changed: Before this patch: dockerd --dns 1.1.1.1oooo --validate Status: invalid argument "1.1.1.1oooo" for "--dns" flag: 1.1.1.1oooo is not an ip address See 'dockerd --help'., Code: 125 cat /etc/docker/daemon.json {"dns": ["1.1.1.1"]} dockerd --dns 2.2.2.2 --validate unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: dns: (from flag: [2.2.2.2], from file: [1.1.1.1]) cat /etc/docker/daemon.json {"dns": ["1.1.1.1oooo"]} dockerd --validate unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: 1.1.1.1ooooo is not an ip address With this patch: dockerd --dns 1.1.1.1oooo --validate Status: invalid argument "1.1.1.1oooo" for "--dns" flag: invalid string being converted to IP address: 1.1.1.1oooo See 'dockerd --help'., Code: 125 cat /etc/docker/daemon.json {"dns": ["1.1.1.1"]} dockerd --dns 2.2.2.2 --validate unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: dns: (from flag: [2.2.2.2], from file: [1.1.1.1]) cat /etc/docker/daemon.json {"dns": ["1.1.1.1oooo"]} dockerd --validate unable to configure the Docker daemon with file /etc/docker/daemon.json: invalid IP address: 1.1.1.1oooo Signed-off-by: Sebastiaan van Stijn --- builder/builder-next/executor_linux.go | 11 ++++- cmd/dockerd/config.go | 2 +- daemon/config/config.go | 9 +--- daemon/config/config_test.go | 62 ++++++++++++-------------- daemon/container_operations.go | 12 ++++- 5 files changed, 51 insertions(+), 45 deletions(-) diff --git a/builder/builder-next/executor_linux.go b/builder/builder-next/executor_linux.go index 015ee557b3aa1..6bd1bbb9810cb 100644 --- a/builder/builder-next/executor_linux.go +++ b/builder/builder-next/executor_linux.go @@ -2,6 +2,7 @@ package buildkit import ( "context" + "net" "os" "path/filepath" "strconv" @@ -166,10 +167,18 @@ func (iface *lnInterface) Close() error { func getDNSConfig(cfg config.DNSConfig) *oci.DNSConfig { if cfg.DNS != nil || cfg.DNSSearch != nil || cfg.DNSOptions != nil { return &oci.DNSConfig{ - Nameservers: cfg.DNS, + Nameservers: ipAddresses(cfg.DNS), SearchDomains: cfg.DNSSearch, Options: cfg.DNSOptions, } } return nil } + +func ipAddresses(ips []net.IP) []string { + var addrs []string + for _, ip := range ips { + addrs = append(addrs, ip.String()) + } + return addrs +} diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go index ead7265e799dd..449a293fe490d 100644 --- a/cmd/dockerd/config.go +++ b/cmd/dockerd/config.go @@ -45,7 +45,7 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { _ = flags.MarkHidden("network-diagnostic-port") flags.BoolVar(&conf.RawLogs, "raw-logs", false, "Full timestamps without ANSI coloring") - flags.Var(opts.NewListOptsRef(&conf.DNS, opts.ValidateIPAddress), "dns", "DNS server to use") + flags.IPSliceVar(&conf.DNS, "dns", conf.DNS, "DNS server to use") flags.Var(opts.NewNamedListOptsRef("dns-opts", &conf.DNSOptions, nil), "dns-opt", "DNS options to use") flags.Var(opts.NewListOptsRef(&conf.DNSSearch, opts.ValidateDNSSearch), "dns-search", "DNS search domains to use") flags.IPVar(&conf.HostGatewayIP, "host-gateway-ip", nil, "IP address that the special 'host-gateway' string in --add-host resolves to. Defaults to the IP address of the default bridge") diff --git a/daemon/config/config.go b/daemon/config/config.go index aea80755280aa..761f242fd3a03 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -131,7 +131,7 @@ type TLSOptions struct { // DNSConfig defines the DNS configurations. type DNSConfig struct { - DNS []string `json:"dns,omitempty"` + DNS []net.IP `json:"dns,omitempty"` DNSOptions []string `json:"dns-opts,omitempty"` DNSSearch []string `json:"dns-search,omitempty"` HostGatewayIP net.IP `json:"host-gateway-ip,omitempty"` @@ -609,13 +609,6 @@ func Validate(config *Config) error { } } - // validate DNS - for _, dns := range config.DNS { - if _, err := opts.ValidateIPAddress(dns); err != nil { - return err - } - } - // validate DNSSearch for _, dnsSearch := range config.DNSSearch { if _, err := opts.ValidateDNSSearch(dnsSearch); err != nil { diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index afb8a1c15cee5..b6c362206d9a7 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -1,6 +1,7 @@ package config // import "github.com/docker/docker/daemon/config" import ( + "encoding/json" "os" "path/filepath" "reflect" @@ -238,28 +239,6 @@ func TestValidateConfigurationErrors(t *testing.T) { }, expectedErr: "bad attribute format: one", }, - { - name: "single DNS, invalid IP-address", - config: &Config{ - CommonConfig: CommonConfig{ - DNSConfig: DNSConfig{ - DNS: []string{"1.1.1.1o"}, - }, - }, - }, - expectedErr: "IP address is not correctly formatted: 1.1.1.1o", - }, - { - name: "multiple DNS, invalid IP-address", - config: &Config{ - CommonConfig: CommonConfig{ - DNSConfig: DNSConfig{ - DNS: []string{"2.2.2.2", "1.1.1.1o"}, - }, - }, - }, - expectedErr: "IP address is not correctly formatted: 1.1.1.1o", - }, { name: "single DNSSearch", config: &Config{ @@ -420,17 +399,6 @@ func TestValidateConfiguration(t *testing.T) { }, }, }, - { - name: "with dns", - field: "DNSConfig", - config: &Config{ - CommonConfig: CommonConfig{ - DNSConfig: DNSConfig{ - DNS: []string{"1.1.1.1"}, - }, - }, - }, - }, { name: "with dns-search", field: "DNSConfig", @@ -534,6 +502,34 @@ func TestValidateConfiguration(t *testing.T) { } } +func TestConfigInvalidDNS(t *testing.T) { + tests := []struct { + doc string + input string + expectedErr string + }{ + { + doc: "single DNS, invalid IP-address", + input: `{"dns": ["1.1.1.1o"]}`, + expectedErr: `invalid IP address: 1.1.1.1o`, + }, + { + doc: "multiple DNS, invalid IP-address", + input: `{"dns": ["2.2.2.2", "1.1.1.1o"]}`, + expectedErr: `invalid IP address: 1.1.1.1o`, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + var cfg Config + err := json.Unmarshal([]byte(tc.input), &cfg) + assert.Check(t, is.Error(err, tc.expectedErr)) + }) + } +} + func field(field string) cmp.Option { tmp := reflect.TypeOf(Config{}) ignoreFields := make([]string, 0, tmp.NumField()) diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 74d1c3ccdfa13..0d1596fe80805 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -30,6 +30,14 @@ import ( "github.com/docker/go-connections/nat" ) +func ipAddresses(ips []net.IP) []string { + var addrs []string + for _, ip := range ips { + addrs = append(addrs, ip.String()) + } + return addrs +} + func (daemon *Daemon) buildSandboxOptions(cfg *config.Config, container *container.Container) ([]libnetwork.SandboxOption, error) { var sboxOptions []libnetwork.SandboxOption sboxOptions = append(sboxOptions, libnetwork.OptionHostname(container.Config.Hostname), libnetwork.OptionDomainname(container.Config.Domainname)) @@ -49,7 +57,7 @@ func (daemon *Daemon) buildSandboxOptions(cfg *config.Config, container *contain if len(container.HostConfig.DNS) > 0 { sboxOptions = append(sboxOptions, libnetwork.OptionDNS(container.HostConfig.DNS)) } else if len(cfg.DNS) > 0 { - sboxOptions = append(sboxOptions, libnetwork.OptionDNS(cfg.DNS)) + sboxOptions = append(sboxOptions, libnetwork.OptionDNS(ipAddresses(cfg.DNS))) } if len(container.HostConfig.DNSSearch) > 0 { sboxOptions = append(sboxOptions, libnetwork.OptionDNSSearch(container.HostConfig.DNSSearch)) @@ -726,7 +734,7 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. // TODO(thaJeztah): should this fail early if no sandbox was found? sb, _ := daemon.netController.GetSandbox(container.ID) - createOptions, err := buildCreateEndpointOptions(container, n, endpointConfig, sb, cfg.DNS) + createOptions, err := buildCreateEndpointOptions(container, n, endpointConfig, sb, ipAddresses(cfg.DNS)) if err != nil { return err }