From 5d758afff8ac1e118fadef20e1c22d7afc2ae1d3 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 29 Nov 2024 22:08:04 +0400 Subject: [PATCH] feat: implement new address sorting algorithm Fixes #9725 See #9749 Signed-off-by: Andrey Smirnov --- .../pkg/controllers/k8s/nodeip_test.go | 22 +++ .../internal/addressutil/addressutil.go | 67 +++++++++ .../internal/addressutil/addressutil_test.go | 105 ++++++++++++++ .../network/internal/addressutil/compare.go | 73 ++++++++++ .../internal/addressutil/compare_test.go | 81 +++++++++++ .../pkg/controllers/network/node_address.go | 135 ++++++------------ .../controllers/network/node_address_test.go | 88 ++++++------ 7 files changed, 435 insertions(+), 136 deletions(-) create mode 100644 internal/app/machined/pkg/controllers/network/internal/addressutil/addressutil.go create mode 100644 internal/app/machined/pkg/controllers/network/internal/addressutil/addressutil_test.go create mode 100644 internal/app/machined/pkg/controllers/network/internal/addressutil/compare.go create mode 100644 internal/app/machined/pkg/controllers/network/internal/addressutil/compare_test.go diff --git a/internal/app/machined/pkg/controllers/k8s/nodeip_test.go b/internal/app/machined/pkg/controllers/k8s/nodeip_test.go index 2c412aae320..522787c25a8 100644 --- a/internal/app/machined/pkg/controllers/k8s/nodeip_test.go +++ b/internal/app/machined/pkg/controllers/k8s/nodeip_test.go @@ -99,6 +99,28 @@ func (suite *NodeIPSuite) TestReconcileNoMatch() { }) } +func (suite *NodeIPSuite) TestReconcileIPv6Denies() { + cfg := k8s.NewNodeIPConfig(k8s.NamespaceName, k8s.KubeletID) + cfg.TypedSpec().ValidSubnets = []string{"::/0", "!fd01:cafe::f14c:9fa1:8496:557f/128"} + suite.Require().NoError(suite.State().Create(suite.Ctx(), cfg)) + + addresses := network.NewNodeAddress( + network.NamespaceName, + network.FilteredNodeAddressID(network.NodeAddressRoutedID, k8s.NodeAddressFilterNoK8s), + ) + + addresses.TypedSpec().Addresses = []netip.Prefix{ + netip.MustParsePrefix("fd01:cafe::f14c:9fa1:8496:557f/128"), + netip.MustParsePrefix("fd01:cafe::5054:ff:fe1f:c7bd/64"), + } + + suite.Require().NoError(suite.State().Create(suite.Ctx(), addresses)) + + rtestutils.AssertResources(suite.Ctx(), suite.T(), suite.State(), []resource.ID{k8s.KubeletID}, func(nodeIP *k8s.NodeIP, asrt *assert.Assertions) { + asrt.Equal("[fd01:cafe::5054:ff:fe1f:c7bd]", fmt.Sprintf("%s", nodeIP.TypedSpec().Addresses)) + }) +} + func TestNodeIPSuite(t *testing.T) { t.Parallel() diff --git a/internal/app/machined/pkg/controllers/network/internal/addressutil/addressutil.go b/internal/app/machined/pkg/controllers/network/internal/addressutil/addressutil.go new file mode 100644 index 00000000000..92784c34881 --- /dev/null +++ b/internal/app/machined/pkg/controllers/network/internal/addressutil/addressutil.go @@ -0,0 +1,67 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +// Package addressutil contains helpers working with addresses. +package addressutil + +import "net/netip" + +// DeduplicateIPPrefixes removes duplicates from the given list of prefixes. +// +// The input list must be sorted. +// DeduplicateIPPrefixes performs in-place deduplication. +func DeduplicateIPPrefixes(in []netip.Prefix) []netip.Prefix { + // assumes that current is sorted + n := 0 + + var prev netip.Prefix + + for _, x := range in { + if prev != x { + in[n] = x + n++ + } + + prev = x + } + + return in[:n] +} + +// FilterIPs filters the given list of IP prefixes based on the given include and exclude subnets. +// +// If includeSubnets is not empty, only IPs that are in one of the subnets are included. +// If excludeSubnets is not empty, IPs that are in one of the subnets are excluded. +func FilterIPs(addrs []netip.Prefix, includeSubnets, excludeSubnets []netip.Prefix) []netip.Prefix { + result := make([]netip.Prefix, 0, len(addrs)) + +outer: + for _, ip := range addrs { + if len(includeSubnets) > 0 { + matchesAny := false + + for _, subnet := range includeSubnets { + if subnet.Contains(ip.Addr()) { + matchesAny = true + + break + } + } + + if !matchesAny { + continue outer + } + } + + for _, subnet := range excludeSubnets { + if subnet.Contains(ip.Addr()) { + continue outer + } + } + + result = append(result, ip) + } + + return result +} diff --git a/internal/app/machined/pkg/controllers/network/internal/addressutil/addressutil_test.go b/internal/app/machined/pkg/controllers/network/internal/addressutil/addressutil_test.go new file mode 100644 index 00000000000..70d1a55f92e --- /dev/null +++ b/internal/app/machined/pkg/controllers/network/internal/addressutil/addressutil_test.go @@ -0,0 +1,105 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +package addressutil_test + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/network/internal/addressutil" +) + +func TestDeduplicateIPPrefixes(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + in []netip.Prefix + + out []netip.Prefix + }{ + { + name: "empty", + }, + { + name: "single", + in: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32"), netip.MustParsePrefix("1.2.3.4/32")}, + + out: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32")}, + }, + { + name: "many", + in: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32"), netip.MustParsePrefix("1.2.3.4/24"), netip.MustParsePrefix("2000::aebc/64"), netip.MustParsePrefix("2000::aebc/64")}, + + out: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32"), netip.MustParsePrefix("1.2.3.4/24"), netip.MustParsePrefix("2000::aebc/64")}, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + got := addressutil.DeduplicateIPPrefixes(test.in) + + assert.Equal(t, test.out, got) + }) + } +} + +// TestFilterIPs tests the FilterIPs function. +func TestFilterIPs(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + + in []netip.Prefix + include []netip.Prefix + exclude []netip.Prefix + + out []netip.Prefix + }{ + { + name: "empty filters", + + in: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32"), netip.MustParsePrefix("2000::aebc/64")}, + + out: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32"), netip.MustParsePrefix("2000::aebc/64")}, + }, + { + name: "v4 only", + + in: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32"), netip.MustParsePrefix("2000::aebc/64")}, + include: []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0")}, + + out: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32")}, + }, + { + name: "v6 only", + + in: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32"), netip.MustParsePrefix("2000::aebc/64")}, + exclude: []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0")}, + + out: []netip.Prefix{netip.MustParsePrefix("2000::aebc/64")}, + }, + { + name: "include and exclude", + + in: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32"), netip.MustParsePrefix("3.4.5.6/24"), netip.MustParsePrefix("2000::aebc/64")}, + include: []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0")}, + exclude: []netip.Prefix{netip.MustParsePrefix("3.0.0.0/8")}, + + out: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32")}, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + got := addressutil.FilterIPs(test.in, test.include, test.exclude) + + assert.Equal(t, test.out, got) + }) + } +} diff --git a/internal/app/machined/pkg/controllers/network/internal/addressutil/compare.go b/internal/app/machined/pkg/controllers/network/internal/addressutil/compare.go new file mode 100644 index 00000000000..dec6131c901 --- /dev/null +++ b/internal/app/machined/pkg/controllers/network/internal/addressutil/compare.go @@ -0,0 +1,73 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +package addressutil + +import ( + "cmp" + "net/netip" + + "github.com/siderolabs/talos/pkg/machinery/resources/network" +) + +// ComparePrefixesLegacy is the old way to sort prefixes. +// +// It only compares addresses and does not take prefix length into account. +func ComparePrefixesLegacy(a, b netip.Prefix) int { + if c := a.Addr().Compare(b.Addr()); c != 0 { + return c + } + + // note: this was missing in the previous implementation, but this makes sorting stable + return cmp.Compare(a.Bits(), b.Bits()) +} + +func family(a netip.Prefix) int { + if a.Addr().Is4() { + return 4 + } + + return 6 +} + +// ComparePrefixNew compares two prefixes by address family, address, and prefix length. +// +// It prefers more specific prefixes. +func ComparePrefixNew(a, b netip.Prefix) int { + // first, compare address families + if c := cmp.Compare(family(a), family(b)); c != 0 { + return c + } + + // if addresses are equal, Contains will report that one prefix contains the other + if a.Addr() == b.Addr() { + return -cmp.Compare(a.Bits(), b.Bits()) + } + + // if one prefix contains another, the more specific one should come first + aContainsB := a.Contains(b.Addr()) + bContainsA := b.Contains(a.Addr()) + + // if both prefixes contain each other, proceed to compare addresses/prefix lengths + switch { + case aContainsB && !bContainsA: + return 1 + case !aContainsB && bContainsA: + return -1 + } + + // compare addresses, they are not equal + return a.Addr().Compare(b.Addr()) +} + +// CompareAddressStatuses compares two address statuses with the prefix comparison func. +func CompareAddressStatuses(comparePrefixes func(a, b netip.Prefix) int) func(a, b *network.AddressStatus) int { + return func(a, b *network.AddressStatus) int { + if c := cmp.Compare(a.TypedSpec().LinkName, b.TypedSpec().LinkName); c != 0 { + return c + } + + return comparePrefixes(a.TypedSpec().Address, b.TypedSpec().Address) + } +} diff --git a/internal/app/machined/pkg/controllers/network/internal/addressutil/compare_test.go b/internal/app/machined/pkg/controllers/network/internal/addressutil/compare_test.go new file mode 100644 index 00000000000..60e4ebc5e08 --- /dev/null +++ b/internal/app/machined/pkg/controllers/network/internal/addressutil/compare_test.go @@ -0,0 +1,81 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +package addressutil_test + +import ( + "math/rand/v2" + "net/netip" + "slices" + "testing" + + "github.com/siderolabs/gen/xslices" + "github.com/stretchr/testify/assert" + + "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/network/internal/addressutil" +) + +func toNetip(prefixes ...string) []netip.Prefix { + return xslices.Map(prefixes, netip.MustParsePrefix) +} + +func toString(prefixes []netip.Prefix) []string { + return xslices.Map(prefixes, netip.Prefix.String) +} + +func TestCompare(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + + in []netip.Prefix + + outLegacy []netip.Prefix + outNew []netip.Prefix + }{ + { + name: "ipv4", + + in: toNetip("10.3.4.1/24", "10.3.4.5/24", "10.3.4.5/32", "1.2.3.4/26", "192.168.35.11/24", "192.168.36.10/24"), + + outLegacy: toNetip("1.2.3.4/26", "10.3.4.1/24", "10.3.4.5/24", "10.3.4.5/32", "192.168.35.11/24", "192.168.36.10/24"), + outNew: toNetip("1.2.3.4/26", "10.3.4.5/32", "10.3.4.1/24", "10.3.4.5/24", "192.168.35.11/24", "192.168.36.10/24"), + }, + { + name: "ipv6", + + in: toNetip("2001:db8::1/64", "2001:db8::1/128", "2001:db8::2/64", "2001:db8::2/128", "2001:db8::3/64", "2001:db8::3/128"), + + outLegacy: toNetip("2001:db8::1/64", "2001:db8::1/128", "2001:db8::2/64", "2001:db8::2/128", "2001:db8::3/64", "2001:db8::3/128"), + outNew: toNetip("2001:db8::1/128", "2001:db8::2/128", "2001:db8::3/128", "2001:db8::1/64", "2001:db8::2/64", "2001:db8::3/64"), + }, + { + name: "mixed", + + in: toNetip("fd01:cafe::5054:ff:fe1f:c7bd/64", "fd01:cafe::f14c:9fa1:8496:557f/128", "192.168.3.4/24", "10.5.0.0/16"), + + outLegacy: toNetip("10.5.0.0/16", "192.168.3.4/24", "fd01:cafe::5054:ff:fe1f:c7bd/64", "fd01:cafe::f14c:9fa1:8496:557f/128"), + outNew: toNetip("10.5.0.0/16", "192.168.3.4/24", "fd01:cafe::f14c:9fa1:8496:557f/128", "fd01:cafe::5054:ff:fe1f:c7bd/64"), + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + // add more randomness to ensure the sorting is stable + in := slices.Clone(test.in) + rand.Shuffle(len(in), func(i, j int) { in[i], in[j] = in[j], in[i] }) + + legacy := slices.Clone(in) + slices.SortFunc(legacy, addressutil.ComparePrefixesLegacy) + + assert.Equal(t, test.outLegacy, legacy, "expected %q but got %q", toString(test.outLegacy), toString(legacy)) + + newer := slices.Clone(in) + slices.SortFunc(newer, addressutil.ComparePrefixNew) + + assert.Equal(t, test.outNew, newer, "expected %q but got %q", toString(test.outNew), toString(newer)) + }) + } +} diff --git a/internal/app/machined/pkg/controllers/network/node_address.go b/internal/app/machined/pkg/controllers/network/node_address.go index e4bbe9ed4c4..8b22bada393 100644 --- a/internal/app/machined/pkg/controllers/network/node_address.go +++ b/internal/app/machined/pkg/controllers/network/node_address.go @@ -9,14 +9,15 @@ import ( "fmt" "net/netip" "slices" - "sort" "github.com/cosi-project/runtime/pkg/controller" "github.com/cosi-project/runtime/pkg/resource" "github.com/cosi-project/runtime/pkg/safe" "github.com/siderolabs/gen/value" + "github.com/siderolabs/gen/xslices" "go.uber.org/zap" + "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/network/internal/addressutil" "github.com/siderolabs/talos/pkg/machinery/nethelpers" "github.com/siderolabs/talos/pkg/machinery/resources/network" ) @@ -76,7 +77,7 @@ func (ctrl *NodeAddressController) Run(ctx context.Context, r controller.Runtime } // fetch link and address status resources - links, err := r.List(ctx, resource.NewMetadata(network.NamespaceName, network.LinkStatusType, "", resource.VersionUndefined)) + links, err := safe.ReaderListAll[*network.LinkStatus](ctx, r) if err != nil { return fmt.Errorf("error listing links: %w", err) } @@ -84,9 +85,7 @@ func (ctrl *NodeAddressController) Run(ctx context.Context, r controller.Runtime // build "link up" lookup table linksUp := make(map[uint32]struct{}) - for _, r := range links.Items { - link := r.(*network.LinkStatus) //nolint:forcetypeassert - + for link := range links.All() { if link.TypedSpec().OperationalState == nethelpers.OperStateUp || link.TypedSpec().OperationalState == nethelpers.OperStateUnknown { // skip physical interfaces without carrier if !link.TypedSpec().Physical() || link.TypedSpec().LinkState { @@ -96,42 +95,51 @@ func (ctrl *NodeAddressController) Run(ctx context.Context, r controller.Runtime } // fetch list of filters - filters, err := r.List(ctx, resource.NewMetadata(network.NamespaceName, network.NodeAddressFilterType, "", resource.VersionUndefined)) + filters, err := safe.ReaderListAll[*network.NodeAddressFilter](ctx, r) if err != nil { return fmt.Errorf("error listing address filters: %w", err) } - addresses, err := r.List(ctx, resource.NewMetadata(network.NamespaceName, network.AddressStatusType, "", resource.VersionUndefined)) + addressesList, err := safe.ReaderListAll[*network.AddressStatus](ctx, r) if err != nil { return fmt.Errorf("error listing links: %w", err) } - var ( - defaultAddress netip.Prefix - defaultAddrLinkName string - current []netip.Prefix - routed []netip.Prefix - accumulative []netip.Prefix - ) + addresses := safe.ToSlice(addressesList, func(a *network.AddressStatus) *network.AddressStatus { return a }) - for _, r := range addresses.Items { - addr := r.(*network.AddressStatus) //nolint:forcetypeassert + compareFunc := addressutil.ComparePrefixesLegacy + // filter out addresses which should be ignored + xslices.FilterInPlace(addresses, func(addr *network.AddressStatus) bool { if addr.TypedSpec().Scope >= nethelpers.ScopeLink { - continue + return false } ip := addr.TypedSpec().Address if ip.Addr().IsLoopback() || ip.Addr().IsMulticast() || ip.Addr().IsLinkLocalUnicast() { - continue + return false } + return true + }) + + slices.SortFunc(addresses, addressutil.CompareAddressStatuses(compareFunc)) + + var ( + defaultAddress netip.Prefix + current []netip.Prefix + routed []netip.Prefix + accumulative []netip.Prefix + ) + + for _, addr := range addresses { + ip := addr.TypedSpec().Address + // set defaultAddress to the smallest IP from the alphabetically first link if addr.Metadata().Owner() == addressStatusControllerName { - if value.IsZero(defaultAddress) || addr.TypedSpec().LinkName < defaultAddrLinkName || (addr.TypedSpec().LinkName == defaultAddrLinkName && ip.Addr().Compare(defaultAddress.Addr()) < 0) { + if value.IsZero(defaultAddress) { defaultAddress = ip - defaultAddrLinkName = addr.TypedSpec().LinkName } } @@ -151,12 +159,12 @@ func (ctrl *NodeAddressController) Run(ctx context.Context, r controller.Runtime } // sort current addresses - sort.Slice(current, func(i, j int) bool { return current[i].Addr().Compare(current[j].Addr()) < 0 }) - sort.Slice(routed, func(i, j int) bool { return routed[i].Addr().Compare(routed[j].Addr()) < 0 }) + slices.SortFunc(current, compareFunc) + slices.SortFunc(routed, compareFunc) // remove duplicates from current addresses - current = deduplicateIPPrefixes(current) - routed = deduplicateIPPrefixes(routed) + current = addressutil.DeduplicateIPPrefixes(current) + routed = addressutil.DeduplicateIPPrefixes(routed) touchedIDs := make(map[resource.ID]struct{}) @@ -183,42 +191,42 @@ func (ctrl *NodeAddressController) Run(ctx context.Context, r controller.Runtime touchedIDs[network.NodeAddressDefaultID] = struct{}{} } - if err = updateCurrentAddresses(ctx, r, network.NodeAddressCurrentID, current); err != nil { + if err = ctrl.updateCurrentAddresses(ctx, r, network.NodeAddressCurrentID, current); err != nil { return err } touchedIDs[network.NodeAddressCurrentID] = struct{}{} - if err = updateCurrentAddresses(ctx, r, network.NodeAddressRoutedID, routed); err != nil { + if err = ctrl.updateCurrentAddresses(ctx, r, network.NodeAddressRoutedID, routed); err != nil { return err } touchedIDs[network.NodeAddressRoutedID] = struct{}{} - if err = updateAccumulativeAddresses(ctx, r, network.NodeAddressAccumulativeID, accumulative); err != nil { + if err = ctrl.updateAccumulativeAddresses(ctx, r, network.NodeAddressAccumulativeID, accumulative); err != nil { return err } touchedIDs[network.NodeAddressAccumulativeID] = struct{}{} // update filtered resources - for _, res := range filters.Items { - filterID := res.Metadata().ID() - filter := res.(*network.NodeAddressFilter).TypedSpec() + for filterRes := range filters.All() { + filterID := filterRes.Metadata().ID() + filter := filterRes.TypedSpec() - filteredCurrent := filterIPs(current, filter.IncludeSubnets, filter.ExcludeSubnets) - filteredRouted := filterIPs(routed, filter.IncludeSubnets, filter.ExcludeSubnets) - filteredAccumulative := filterIPs(accumulative, filter.IncludeSubnets, filter.ExcludeSubnets) + filteredCurrent := addressutil.FilterIPs(current, filter.IncludeSubnets, filter.ExcludeSubnets) + filteredRouted := addressutil.FilterIPs(routed, filter.IncludeSubnets, filter.ExcludeSubnets) + filteredAccumulative := addressutil.FilterIPs(accumulative, filter.IncludeSubnets, filter.ExcludeSubnets) - if err = updateCurrentAddresses(ctx, r, network.FilteredNodeAddressID(network.NodeAddressCurrentID, filterID), filteredCurrent); err != nil { + if err = ctrl.updateCurrentAddresses(ctx, r, network.FilteredNodeAddressID(network.NodeAddressCurrentID, filterID), filteredCurrent); err != nil { return err } - if err = updateCurrentAddresses(ctx, r, network.FilteredNodeAddressID(network.NodeAddressRoutedID, filterID), filteredRouted); err != nil { + if err = ctrl.updateCurrentAddresses(ctx, r, network.FilteredNodeAddressID(network.NodeAddressRoutedID, filterID), filteredRouted); err != nil { return err } - if err = updateAccumulativeAddresses(ctx, r, network.FilteredNodeAddressID(network.NodeAddressAccumulativeID, filterID), filteredAccumulative); err != nil { + if err = ctrl.updateAccumulativeAddresses(ctx, r, network.FilteredNodeAddressID(network.NodeAddressAccumulativeID, filterID), filteredAccumulative); err != nil { return err } @@ -249,58 +257,7 @@ func (ctrl *NodeAddressController) Run(ctx context.Context, r controller.Runtime } } -func deduplicateIPPrefixes(current []netip.Prefix) []netip.Prefix { - // assumes that current is sorted - n := 0 - - var prev netip.Prefix - - for _, x := range current { - if prev != x { - current[n] = x - n++ - } - - prev = x - } - - return current[:n] -} - -func filterIPs(addrs []netip.Prefix, includeSubnets, excludeSubnets []netip.Prefix) []netip.Prefix { - result := make([]netip.Prefix, 0, len(addrs)) - -outer: - for _, ip := range addrs { - if len(includeSubnets) > 0 { - matchesAny := false - - for _, subnet := range includeSubnets { - if subnet.Contains(ip.Addr()) { - matchesAny = true - - break - } - } - - if !matchesAny { - continue outer - } - } - - for _, subnet := range excludeSubnets { - if subnet.Contains(ip.Addr()) { - continue outer - } - } - - result = append(result, ip) - } - - return result -} - -func updateCurrentAddresses(ctx context.Context, r controller.Runtime, id resource.ID, current []netip.Prefix) error { +func (ctrl *NodeAddressController) updateCurrentAddresses(ctx context.Context, r controller.Runtime, id resource.ID, current []netip.Prefix) error { if err := safe.WriterModify(ctx, r, network.NewNodeAddress(network.NamespaceName, id), func(r *network.NodeAddress) error { spec := r.TypedSpec() @@ -314,7 +271,7 @@ func updateCurrentAddresses(ctx context.Context, r controller.Runtime, id resour return nil } -func updateAccumulativeAddresses(ctx context.Context, r controller.Runtime, id resource.ID, accumulative []netip.Prefix) error { +func (ctrl *NodeAddressController) updateAccumulativeAddresses(ctx context.Context, r controller.Runtime, id resource.ID, accumulative []netip.Prefix) error { if err := safe.WriterModify(ctx, r, network.NewNodeAddress(network.NamespaceName, id), func(r *network.NodeAddress) error { spec := r.TypedSpec() diff --git a/internal/app/machined/pkg/controllers/network/node_address_test.go b/internal/app/machined/pkg/controllers/network/node_address_test.go index bc0435acb38..340440b79b2 100644 --- a/internal/app/machined/pkg/controllers/network/node_address_test.go +++ b/internal/app/machined/pkg/controllers/network/node_address_test.go @@ -15,11 +15,13 @@ import ( "github.com/cosi-project/runtime/pkg/resource" "github.com/cosi-project/runtime/pkg/resource/rtestutils" "github.com/cosi-project/runtime/pkg/state" + "github.com/siderolabs/gen/xslices" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/ctest" netctrl "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/network" + "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/network/internal/addressutil" "github.com/siderolabs/talos/pkg/machinery/nethelpers" "github.com/siderolabs/talos/pkg/machinery/resources/network" runtimeres "github.com/siderolabs/talos/pkg/machinery/resources/runtime" @@ -53,9 +55,7 @@ func (suite *NodeAddressSuite) TestDefaults() { asrt.True( slices.IsSortedFunc( addrs, - func(a, b netip.Prefix) int { - return a.Addr().Compare(b.Addr()) - }, + addressutil.ComparePrefixesLegacy, ), "addresses %s", addrs, ) @@ -161,42 +161,42 @@ func (suite *NodeAddressSuite) TestFilters() { switch r.Metadata().ID() { case network.NodeAddressDefaultID: - asrt.Equal(addrs, ipList("10.0.0.1/8")) + asrt.Equal("10.0.0.1/8", stringifyIPs(addrs)) case network.NodeAddressCurrentID: asrt.Equal( - ipList("1.2.3.4/32 10.0.0.1/8 25.3.7.9/32 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64 fdae:41e4:649b:9303:7886:731d:1ce9:4d4/128"), - addrs, + "1.2.3.4/32 10.0.0.1/8 25.3.7.9/32 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64 fdae:41e4:649b:9303:7886:731d:1ce9:4d4/128", + stringifyIPs(addrs), ) case network.NodeAddressRoutedID: asrt.Equal( - ipList("10.0.0.1/8 25.3.7.9/32 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64"), - addrs, + "10.0.0.1/8 25.3.7.9/32 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64", + stringifyIPs(addrs), ) case network.NodeAddressAccumulativeID: asrt.Equal( - ipList("1.2.3.4/32 10.0.0.1/8 10.0.0.2/8 25.3.7.9/32 192.168.3.7/24 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64 fdae:41e4:649b:9303:7886:731d:1ce9:4d4/128"), - addrs, + "1.2.3.4/32 10.0.0.1/8 10.0.0.2/8 25.3.7.9/32 192.168.3.7/24 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64 fdae:41e4:649b:9303:7886:731d:1ce9:4d4/128", + stringifyIPs(addrs), ) case network.FilteredNodeAddressID(network.NodeAddressCurrentID, filter1.Metadata().ID()): asrt.Equal( - ipList("1.2.3.4/32 25.3.7.9/32 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64 fdae:41e4:649b:9303:7886:731d:1ce9:4d4/128"), - addrs, + "1.2.3.4/32 25.3.7.9/32 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64 fdae:41e4:649b:9303:7886:731d:1ce9:4d4/128", + stringifyIPs(addrs), ) case network.FilteredNodeAddressID(network.NodeAddressRoutedID, filter1.Metadata().ID()): asrt.Equal( - ipList("25.3.7.9/32 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64"), - addrs, + "25.3.7.9/32 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64", + stringifyIPs(addrs), ) case network.FilteredNodeAddressID(network.NodeAddressAccumulativeID, filter1.Metadata().ID()): asrt.Equal( - ipList("1.2.3.4/32 25.3.7.9/32 192.168.3.7/24 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64 fdae:41e4:649b:9303:7886:731d:1ce9:4d4/128"), - addrs, + "1.2.3.4/32 25.3.7.9/32 192.168.3.7/24 2001:470:6d:30e:4a62:b3ba:180b:b5b8/64 fdae:41e4:649b:9303:7886:731d:1ce9:4d4/128", + stringifyIPs(addrs), ) case network.FilteredNodeAddressID(network.NodeAddressCurrentID, filter2.Metadata().ID()), network.FilteredNodeAddressID(network.NodeAddressRoutedID, filter2.Metadata().ID()): - asrt.Equal(addrs, ipList("10.0.0.1/8")) + asrt.Equal("10.0.0.1/8", stringifyIPs(addrs)) case network.FilteredNodeAddressID(network.NodeAddressAccumulativeID, filter2.Metadata().ID()): - asrt.Equal(addrs, ipList("10.0.0.1/8 10.0.0.2/8 192.168.3.7/24")) + asrt.Equal("10.0.0.1/8 10.0.0.2/8 192.168.3.7/24", stringifyIPs(addrs)) } }, ) @@ -256,22 +256,22 @@ func (suite *NodeAddressSuite) TestFilterOverlappingSubnets() { switch r.Metadata().ID() { case network.NodeAddressCurrentID, network.NodeAddressRoutedID, network.NodeAddressAccumulativeID: asrt.Equal( - ipList("10.0.0.1/8 10.96.0.2/32 25.3.7.9/32"), - addrs, + "10.0.0.1/8 10.96.0.2/32 25.3.7.9/32", + stringifyIPs(addrs), ) case network.FilteredNodeAddressID(network.NodeAddressCurrentID, filter1.Metadata().ID()), network.FilteredNodeAddressID(network.NodeAddressRoutedID, filter1.Metadata().ID()), network.FilteredNodeAddressID(network.NodeAddressAccumulativeID, filter1.Metadata().ID()): asrt.Equal( - ipList("10.0.0.1/8 25.3.7.9/32"), - addrs, + "10.0.0.1/8 25.3.7.9/32", + stringifyIPs(addrs), ) case network.FilteredNodeAddressID(network.NodeAddressCurrentID, filter2.Metadata().ID()), network.FilteredNodeAddressID(network.NodeAddressRoutedID, filter2.Metadata().ID()), network.FilteredNodeAddressID(network.NodeAddressAccumulativeID, filter2.Metadata().ID()): asrt.Equal( - ipList("10.96.0.2/32"), - addrs, + "10.96.0.2/32", + stringifyIPs(addrs), ) } }, @@ -320,16 +320,16 @@ func (suite *NodeAddressSuite) TestDefaultAddressChange() { switch r.Metadata().ID() { case network.NodeAddressDefaultID: - asrt.Equal(addrs, ipList("10.0.0.5/8")) + asrt.Equal("10.0.0.5/8", stringifyIPs(addrs)) case network.NodeAddressCurrentID: asrt.Equal( - addrs, - ipList("10.0.0.5/8 25.3.7.9/32"), + "10.0.0.5/8 25.3.7.9/32", + stringifyIPs(addrs), ) case network.NodeAddressAccumulativeID: asrt.Equal( - addrs, - ipList("10.0.0.5/8 25.3.7.9/32"), + "10.0.0.5/8 25.3.7.9/32", + stringifyIPs(addrs), ) } }, @@ -348,16 +348,16 @@ func (suite *NodeAddressSuite) TestDefaultAddressChange() { switch r.Metadata().ID() { case network.NodeAddressDefaultID: - asrt.Equal(addrs, ipList("10.0.0.5/8")) + asrt.Equal("10.0.0.5/8", stringifyIPs(addrs)) case network.NodeAddressCurrentID: asrt.Equal( - addrs, - ipList("1.1.1.1/32 10.0.0.5/8 25.3.7.9/32"), + "1.1.1.1/32 10.0.0.5/8 25.3.7.9/32", + stringifyIPs(addrs), ) case network.NodeAddressAccumulativeID: asrt.Equal( - addrs, - ipList("1.1.1.1/32 10.0.0.5/8 25.3.7.9/32"), + "1.1.1.1/32 10.0.0.5/8 25.3.7.9/32", + stringifyIPs(addrs), ) } }, @@ -379,16 +379,16 @@ func (suite *NodeAddressSuite) TestDefaultAddressChange() { switch r.Metadata().ID() { case network.NodeAddressDefaultID: - asrt.Equal(addrs, ipList("1.1.1.1/32")) + asrt.Equal("1.1.1.1/32", stringifyIPs(addrs)) case network.NodeAddressCurrentID: asrt.Equal( - addrs, - ipList("1.1.1.1/32 25.3.7.9/32"), + "1.1.1.1/32 25.3.7.9/32", + stringifyIPs(addrs), ) case network.NodeAddressAccumulativeID: asrt.Equal( - addrs, - ipList("1.1.1.1/32 10.0.0.5/8 25.3.7.9/32"), + "1.1.1.1/32 10.0.0.5/8 25.3.7.9/32", + stringifyIPs(addrs), ) } }, @@ -408,12 +408,6 @@ func TestNodeAddressSuite(t *testing.T) { }) } -func ipList(ips string) []netip.Prefix { - var result []netip.Prefix //nolint:prealloc - - for _, ip := range strings.Split(ips, " ") { - result = append(result, netip.MustParsePrefix(ip)) - } - - return result +func stringifyIPs(ips []netip.Prefix) string { + return strings.Join(xslices.Map(ips, netip.Prefix.String), " ") }