diff --git a/libnetwork/internal/resolvconf/resolvconf.go b/libnetwork/internal/resolvconf/resolvconf.go index c418492324..634b9df7ad 100644 --- a/libnetwork/internal/resolvconf/resolvconf.go +++ b/libnetwork/internal/resolvconf/resolvconf.go @@ -293,21 +293,17 @@ func (rc *ResolvConf) TransformForIntNS( rc.md.UsedDefaultNS = true } - // Validate the ndots option from host config or overrides, if present. - // TODO(robmry) - pre-existing behaviour, but ... - // Validating ndots from an override is good, but not-liking something in the - // host's resolv.conf isn't a reason to fail - just remove? (And it'll be - // replaced by the value in reqdOptions, if given.) - if ndots, exists := rc.Option("ndots"); exists { - if n, err := strconv.Atoi(ndots); err != nil || n < 0 { - return nil, errdefs.InvalidParameter( - fmt.Errorf("invalid number for ndots option: %v", ndots)) - } - } - // For each option required by the nameserver, add it if not already - // present (if the option already has a value don't change it). + // For each option required by the nameserver, add it if not already present. If + // the option is already present, don't override it. Apart from ndots - if the + // ndots value is invalid and an ndots option is required, replace the existing + // value. for _, opt := range reqdOptions { optName, _, _ := strings.Cut(opt, ":") + if optName == "ndots" { + rc.options = removeInvalidNDots(rc.options) + // No need to update rc.md.NDotsFrom, if there is no ndots option remaining, + // it'll be set to "internal" when the required value is added. + } if _, exists := rc.Option(optName); !exists { rc.AddOption(opt) } @@ -496,3 +492,22 @@ func defaultNSAddrs(ipv6 bool) []netip.Addr { } return addrs } + +// removeInvalidNDots filters ill-formed "ndots" settings from options. +// The backing array of the options slice is reused. +func removeInvalidNDots(options []string) []string { + n := 0 + for _, opt := range options { + k, v, _ := strings.Cut(opt, ":") + if k == "ndots" { + ndots, err := strconv.Atoi(v) + if err != nil || ndots < 0 { + continue + } + } + options[n] = opt + n++ + } + clear(options[n:]) // Zero out the obsolete elements, for GC. + return options[:n] +} diff --git a/libnetwork/internal/resolvconf/resolvconf_test.go b/libnetwork/internal/resolvconf/resolvconf_test.go index 21605178df..6b7e480066 100644 --- a/libnetwork/internal/resolvconf/resolvconf_test.go +++ b/libnetwork/internal/resolvconf/resolvconf_test.go @@ -500,6 +500,60 @@ func TestRCTransformForIntNS(t *testing.T) { } } +// Check that invalid ndots options in the host's file are ignored, unless +// starting the internal resolver (which requires an ndots option), in which +// case invalid ndots should be replaced. +func TestRCTransformForIntNSInvalidNdots(t *testing.T) { + testcases := []struct { + name string + options string + reqdOptions []string + expVal string + expOptions []string + expNDotsFrom string + }{ + { + name: "Negative value", + options: "options ndots:-1", + expOptions: []string{"ndots:-1"}, + expVal: "-1", + expNDotsFrom: "host", + }, + { + name: "Invalid values with reqd ndots", + options: "options ndots:-1 foo:bar ndots ndots:", + reqdOptions: []string{"ndots:2"}, + expVal: "2", + expNDotsFrom: "internal", + expOptions: []string{"foo:bar", "ndots:2"}, + }, + { + name: "Valid value with reqd ndots", + options: "options ndots:1 foo:bar ndots ndots:", + reqdOptions: []string{"ndots:2"}, + expVal: "1", + expNDotsFrom: "host", + expOptions: []string{"ndots:1", "foo:bar"}, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + content := "nameserver 8.8.8.8\n" + tc.options + rc, err := Parse(bytes.NewBuffer([]byte(content)), "/etc/resolv.conf") + assert.NilError(t, err) + _, err = rc.TransformForIntNS(false, netip.MustParseAddr("127.0.0.11"), tc.reqdOptions) + assert.NilError(t, err) + + val, found := rc.Option("ndots") + assert.Check(t, is.Equal(found, true)) + assert.Check(t, is.Equal(val, tc.expVal)) + assert.Check(t, is.Equal(rc.md.NDotsFrom, tc.expNDotsFrom)) + assert.Check(t, is.DeepEqual(rc.options, tc.expOptions)) + }) + } +} + func TestRCRead(t *testing.T) { d := t.TempDir() path := filepath.Join(d, "resolv.conf") diff --git a/libnetwork/sandbox_dns_unix_test.go b/libnetwork/sandbox_dns_unix_test.go index d092ab53fd..ef14aeb15d 100644 --- a/libnetwork/sandbox_dns_unix_test.go +++ b/libnetwork/sandbox_dns_unix_test.go @@ -77,11 +77,19 @@ func TestDNSOptions(t *testing.T) { err = sb2.setupDNS() assert.NilError(t, err) err = sb2.rebuildDNS() - assert.Error(t, err, "invalid number for ndots option: foobar") + assert.NilError(t, err) + currRC, err = resolvconf.GetSpecific(sb2.config.resolvConfPath) + assert.NilError(t, err) + dnsOptionsList = resolvconf.GetOptions(currRC.Content) + assert.Check(t, is.DeepEqual([]string{"ndots:0"}, dnsOptionsList)) sb2.config.dnsOptionsList = []string{"ndots:-1"} err = sb2.setupDNS() assert.NilError(t, err) err = sb2.rebuildDNS() - assert.Error(t, err, "invalid number for ndots option: -1") + assert.NilError(t, err) + currRC, err = resolvconf.GetSpecific(sb2.config.resolvConfPath) + assert.NilError(t, err) + dnsOptionsList = resolvconf.GetOptions(currRC.Content) + assert.Check(t, is.DeepEqual([]string{"ndots:0"}, dnsOptionsList)) }