Skip to content

Commit

Permalink
daemon/config: change DNSConfig.DNS to a []net.IP
Browse files Browse the repository at this point in the history
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 <github@gone.nl>
  • Loading branch information
thaJeztah committed Nov 13, 2023
1 parent 34e923e commit 84036d3
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 45 deletions.
11 changes: 10 additions & 1 deletion builder/builder-next/executor_linux.go
Expand Up @@ -2,6 +2,7 @@ package buildkit

import (
"context"
"net"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion cmd/dockerd/config.go
Expand Up @@ -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")
Expand Down
9 changes: 1 addition & 8 deletions daemon/config/config.go
Expand Up @@ -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"`
Expand Down Expand Up @@ -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 {
Expand Down
62 changes: 29 additions & 33 deletions 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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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())
Expand Down
12 changes: 10 additions & 2 deletions daemon/container_operations.go
Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 84036d3

Please sign in to comment.