From c288ace7b185cd3fad569c0848afbda7217ac269 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Tue, 16 Jul 2024 14:18:22 +0400 Subject: [PATCH] fix: be more smart when merging DNS resolver config Fixes #8690 Consider the following scenario (e.g. OpenStack): platform issues a correct list of DNS servers, which includes both IPv4 and IPv6 resolvers, and configures DHCPv4 on the interface. DHCPv4 returns a set of IPv4 resolvers (as it can't return IPv6 ones), and this list completely overrides the list from the platform, wiping out the IPv6 resolvers completely. With this change, the merge process is more smart, as it tries to preserve IPv6 resolvers for example if the next layer provides no resolvers for IPv6. Signed-off-by: Andrey Smirnov --- go.mod | 2 +- go.sum | 4 +- .../pkg/controllers/network/link_spec.go | 45 ++++-------- .../pkg/controllers/network/resolver_merge.go | 69 +++++++++++++++---- .../network/resolver_merge_test.go | 37 ++++++++++ .../controllers/network/timeserver_merge.go | 2 - pkg/machinery/go.mod | 2 +- pkg/machinery/go.sum | 4 +- 8 files changed, 112 insertions(+), 53 deletions(-) diff --git a/go.mod b/go.mod index ab89967c11..787fb65a3e 100644 --- a/go.mod +++ b/go.mod @@ -62,7 +62,7 @@ require ( github.com/containernetworking/plugins v1.5.1 github.com/coredns/coredns v1.11.2 github.com/coreos/go-iptables v0.7.0 - github.com/cosi-project/runtime v0.5.2 + github.com/cosi-project/runtime v0.5.3 github.com/distribution/reference v0.6.0 github.com/docker/docker v27.0.3+incompatible github.com/docker/go-connections v0.5.0 diff --git a/go.sum b/go.sum index 98a80253cf..48fb0be04d 100644 --- a/go.sum +++ b/go.sum @@ -187,8 +187,8 @@ github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03VsM8rvUec= github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= -github.com/cosi-project/runtime v0.5.2 h1:3UlNiXF1/jexLYu2grd8jdTR8/FrACOUdN1mxRnCKZs= -github.com/cosi-project/runtime v0.5.2/go.mod h1:m+bkfUzKYeUyoqYAQBxdce3bfgncG8BsqcbfKRbvJKs= +github.com/cosi-project/runtime v0.5.3 h1:smWlrxr37qP+myY3NeEg94Q1UbQt3ayTZsFp8q/RW1I= +github.com/cosi-project/runtime v0.5.3/go.mod h1:m+bkfUzKYeUyoqYAQBxdce3bfgncG8BsqcbfKRbvJKs= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/cpuguy83/go-md2man/v2 v2.0.4 h1:wfIWP927BUkWJb2NmU/kNDYIBTh/ziUX91+lVfRxZq4= github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= diff --git a/internal/app/machined/pkg/controllers/network/link_spec.go b/internal/app/machined/pkg/controllers/network/link_spec.go index ec7739e3bf..f106e0d39d 100644 --- a/internal/app/machined/pkg/controllers/network/link_spec.go +++ b/internal/app/machined/pkg/controllers/network/link_spec.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "sort" "github.com/cosi-project/runtime/pkg/controller" "github.com/cosi-project/runtime/pkg/resource" @@ -145,37 +144,23 @@ func (ctrl *LinkSpecController) Run(ctx context.Context, r controller.Runtime, l // SortBonds sort resources in increasing order, except it places slave interfaces right after the bond // in proper order. -func SortBonds(items LinkSpecs) { - sort.Sort(&struct { - LinkSpecs - lessLinkSpecs - }{items, lessLinkSpecs{items}}) -} - -type lessLinkSpecs struct{ LinkSpecs } - -func (ls lessLinkSpecs) Less(i, j int) bool { - left := ls.Get(i).TypedSpec() - right := ls.Get(j).TypedSpec() - - l := ordered.MakeTriple(left.Name, 0, "") - if left.BondSlave.MasterName != "" { - l = ordered.MakeTriple(left.BondSlave.MasterName, left.BondSlave.SlaveIndex, left.Name) - } - - r := ordered.MakeTriple(right.Name, 0, "") - if right.BondSlave.MasterName != "" { - r = ordered.MakeTriple(right.BondSlave.MasterName, right.BondSlave.SlaveIndex, right.Name) - } +func SortBonds(items *safe.List[*network.LinkSpec]) { + items.SortFunc(func(ll, rr *network.LinkSpec) int { + left := ll.TypedSpec() + right := rr.TypedSpec() + + l := ordered.MakeTriple(left.Name, 0, "") + if left.BondSlave.MasterName != "" { + l = ordered.MakeTriple(left.BondSlave.MasterName, left.BondSlave.SlaveIndex, left.Name) + } - return l.LessThan(r) -} + r := ordered.MakeTriple(right.Name, 0, "") + if right.BondSlave.MasterName != "" { + r = ordered.MakeTriple(right.BondSlave.MasterName, right.BondSlave.SlaveIndex, right.Name) + } -// LinkSpecs is a sortable collection of network.LinkSpec. -type LinkSpecs interface { - Len() int - Swap(i, j int) - Get(i int) *network.LinkSpec + return l.Compare(r) + }) } func findLink(links []rtnetlink.LinkMessage, name string) *rtnetlink.LinkMessage { diff --git a/internal/app/machined/pkg/controllers/network/resolver_merge.go b/internal/app/machined/pkg/controllers/network/resolver_merge.go index 7ebb016b80..8e3960083c 100644 --- a/internal/app/machined/pkg/controllers/network/resolver_merge.go +++ b/internal/app/machined/pkg/controllers/network/resolver_merge.go @@ -3,16 +3,18 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. // Package network provides controllers which manage network resources. -// -//nolint:dupl package network import ( + "cmp" "context" "fmt" + "net/netip" + "slices" "github.com/cosi-project/runtime/pkg/controller" "github.com/cosi-project/runtime/pkg/resource" + "github.com/cosi-project/runtime/pkg/safe" "github.com/cosi-project/runtime/pkg/state" "go.uber.org/zap" @@ -65,28 +67,29 @@ func (ctrl *ResolverMergeController) Run(ctx context.Context, r controller.Runti } // list source network configuration resources - list, err := r.List(ctx, resource.NewMetadata(network.ConfigNamespaceName, network.ResolverSpecType, "", resource.VersionUndefined)) + list, err := safe.ReaderList[*network.ResolverSpec](ctx, r, resource.NewMetadata(network.ConfigNamespaceName, network.ResolverSpecType, "", resource.VersionUndefined)) if err != nil { return fmt.Errorf("error listing source network addresses: %w", err) } + // sort by config layer + list.SortFunc(func(l, r *network.ResolverSpec) int { + return cmp.Compare(l.TypedSpec().ConfigLayer, r.TypedSpec().ConfigLayer) + }) + // simply merge by layers, overriding with the next configuration layer var final network.ResolverSpecSpec - for _, res := range list.Items { - spec := res.(*network.ResolverSpec) //nolint:errcheck,forcetypeassert - - if final.DNSServers != nil && spec.TypedSpec().ConfigLayer < final.ConfigLayer { - // skip this spec, as existing one is higher layer - continue - } + for iter := list.Iterator(); iter.Next(); { + spec := iter.Value().TypedSpec() - if spec.TypedSpec().ConfigLayer == final.ConfigLayer { - // merge server lists on the same level - final.DNSServers = append(final.DNSServers, spec.TypedSpec().DNSServers...) + if spec.ConfigLayer == final.ConfigLayer { + // simply append server lists on the same layer + final.DNSServers = append(final.DNSServers, spec.DNSServers...) } else { - // otherwise, replace the lists - final = *spec.TypedSpec() + // otherwise, do a smart merge across IPv4/IPv6 + final.ConfigLayer = spec.ConfigLayer + mergeDNSServers(&final.DNSServers, spec.DNSServers) } } @@ -130,3 +133,39 @@ func (ctrl *ResolverMergeController) Run(ctx context.Context, r controller.Runti r.ResetRestartBackoff() } } + +func mergeDNSServers(dst *[]netip.Addr, src []netip.Addr) { + if *dst == nil { + *dst = src + + return + } + + srcHasV4 := len(filterIPFamily(src, true)) > 0 + srcHasV6 := len(filterIPFamily(src, false)) > 0 + dstHasV4 := len(filterIPFamily(*dst, true)) > 0 + dstHasV6 := len(filterIPFamily(*dst, false)) > 0 + + // if old set has IPv4, and new one doesn't, preserve IPv4 + // and same vice versa for IPv6 + switch { + case dstHasV4 && !srcHasV4: + *dst = append(slices.Clone(src), filterIPFamily(*dst, true)...) + case dstHasV6 && !srcHasV6: + *dst = append(slices.Clone(src), filterIPFamily(*dst, false)...) + default: + *dst = src + } +} + +func filterIPFamily(src []netip.Addr, isIPv4 bool) []netip.Addr { + var dst []netip.Addr + + for _, addr := range src { + if addr.Is4() == isIPv4 { + dst = append(dst, addr) + } + } + + return dst +} diff --git a/internal/app/machined/pkg/controllers/network/resolver_merge_test.go b/internal/app/machined/pkg/controllers/network/resolver_merge_test.go index eada26b10f..e4371807b7 100644 --- a/internal/app/machined/pkg/controllers/network/resolver_merge_test.go +++ b/internal/app/machined/pkg/controllers/network/resolver_merge_test.go @@ -7,6 +7,7 @@ package network_test import ( "context" + "fmt" "net/netip" "sync" "testing" @@ -118,6 +119,42 @@ func (suite *ResolverMergeSuite) TestMerge() { ) } +func (suite *ResolverMergeSuite) TestMergeIPv46() { + def := network.NewResolverSpec(network.ConfigNamespaceName, "default/resolvers") + *def.TypedSpec() = network.ResolverSpecSpec{ + DNSServers: []netip.Addr{ + netip.MustParseAddr(constants.DefaultPrimaryResolver), + netip.MustParseAddr(constants.DefaultSecondaryResolver), + }, + ConfigLayer: network.ConfigDefault, + } + + platform := network.NewResolverSpec(network.ConfigNamespaceName, "platform/resolvers") + *platform.TypedSpec() = network.ResolverSpecSpec{ + DNSServers: []netip.Addr{netip.MustParseAddr("1.1.2.0"), netip.MustParseAddr("fe80::1")}, + ConfigLayer: network.ConfigPlatform, + } + + dhcp := network.NewResolverSpec(network.ConfigNamespaceName, "dhcp/eth1") + *dhcp.TypedSpec() = network.ResolverSpecSpec{ + DNSServers: []netip.Addr{netip.MustParseAddr("1.1.2.1")}, + ConfigLayer: network.ConfigOperator, + } + + for _, res := range []resource.Resource{def, platform, dhcp} { + suite.Require().NoError(suite.state.Create(suite.ctx, res), "%v", res.Spec()) + } + + suite.assertResolvers( + []string{ + "resolvers", + }, func(r *network.ResolverSpec, asrt *assert.Assertions) { + asrt.Equal(network.ConfigOperator, r.TypedSpec().ConfigLayer) + asrt.Equal(`["1.1.2.1" "fe80::1"]`, fmt.Sprintf("%q", r.TypedSpec().DNSServers)) + }, + ) +} + func (suite *ResolverMergeSuite) TearDownTest() { suite.T().Log("tear down") diff --git a/internal/app/machined/pkg/controllers/network/timeserver_merge.go b/internal/app/machined/pkg/controllers/network/timeserver_merge.go index a9410bdefe..4de505c296 100644 --- a/internal/app/machined/pkg/controllers/network/timeserver_merge.go +++ b/internal/app/machined/pkg/controllers/network/timeserver_merge.go @@ -3,8 +3,6 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. // Package network provides controllers which manage network resources. -// -//nolint:dupl package network import ( diff --git a/pkg/machinery/go.mod b/pkg/machinery/go.mod index 644bf54612..800e2abf3e 100644 --- a/pkg/machinery/go.mod +++ b/pkg/machinery/go.mod @@ -9,7 +9,7 @@ replace gopkg.in/yaml.v3 => github.com/unix4ever/yaml v0.0.0-20220527175918-f17b require ( github.com/blang/semver/v4 v4.0.0 github.com/containerd/go-cni v1.1.10 - github.com/cosi-project/runtime v0.5.1 + github.com/cosi-project/runtime v0.5.3 github.com/dustin/go-humanize v1.0.1 github.com/emicklei/dot v1.6.2 github.com/evanphx/json-patch v5.9.0+incompatible diff --git a/pkg/machinery/go.sum b/pkg/machinery/go.sum index fbea85d6e3..80c94a41cc 100644 --- a/pkg/machinery/go.sum +++ b/pkg/machinery/go.sum @@ -21,8 +21,8 @@ github.com/containerd/go-cni v1.1.10 h1:c2U73nld7spSWfiJwSh/8W9DK+/qQwYM2rngIhCy github.com/containerd/go-cni v1.1.10/go.mod h1:/Y/sL8yqYQn1ZG1om1OncJB1W4zN3YmjfP/ShCzG/OY= github.com/containernetworking/cni v1.2.2 h1:9IbP6KJQQxVKo4hhnm8r50YcVKrJbJu3Dqw+Rbt1vYk= github.com/containernetworking/cni v1.2.2/go.mod h1:DuLgF+aPd3DzcTQTtp/Nvl1Kim23oFKdm2okJzBQA5M= -github.com/cosi-project/runtime v0.5.1 h1:07r4lk8sgiyhLdRuqZidB6qV3jFpKYhGccWdhTYHYcc= -github.com/cosi-project/runtime v0.5.1/go.mod h1:m+bkfUzKYeUyoqYAQBxdce3bfgncG8BsqcbfKRbvJKs= +github.com/cosi-project/runtime v0.5.3 h1:smWlrxr37qP+myY3NeEg94Q1UbQt3ayTZsFp8q/RW1I= +github.com/cosi-project/runtime v0.5.3/go.mod h1:m+bkfUzKYeUyoqYAQBxdce3bfgncG8BsqcbfKRbvJKs= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=