Skip to content

Commit 3f92f16

Browse files
HadrienPatteHadrienPatte
committed
node: Migrate IPv{4,6}IngressIP to netip
This PR migrates `Node.IPv4IngressIP` and `Node.IPv6IngressIP` from `net.IP` to `ip.Addr`, following the same approach as the health IPs migration (cilium#47001). We use the `ip.Addr` wrapper over the plain `netip.Addr` type because the `Node` struct needs generated deepcopy/deepequal methods (see cilium#46047). Relates to: * cilium#46924 * cilium#24246 Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
1 parent 40d3a5d commit 3f92f16

14 files changed

Lines changed: 73 additions & 95 deletions

File tree

‎daemon/infraendpoints/infra_ip_allocation.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ func (r *infraIPAllocator) allocateHealthIPs(ctx context.Context, oldV4HealthIP
472472
return nil
473473
}
474474

475-
func (r *infraIPAllocator) allocateIngressIPs(ctx context.Context, oldV4IngressIP net.IP, oldV6IngressIP net.IP) error {
475+
func (r *infraIPAllocator) allocateIngressIPs(ctx context.Context, oldV4IngressIP netip.Addr, oldV6IngressIP netip.Addr) error {
476476
if !r.daemonConfig.EnableEnvoyConfig {
477477
return nil
478478
}
@@ -483,8 +483,8 @@ func (r *infraIPAllocator) allocateIngressIPs(ctx context.Context, oldV4IngressI
483483
var err error
484484

485485
// Reallocate the same address as before, if possible
486-
if ingressIPv4 != nil {
487-
result, err = r.ipAllocator.AllocateIPWithoutSyncUpstream(iputil.AddrFromIP(ingressIPv4), "ingress", ipam.PoolDefault())
486+
if ingressIPv4.IsValid() {
487+
result, err = r.ipAllocator.AllocateIPWithoutSyncUpstream(ingressIPv4, "ingress", ipam.PoolDefault())
488488
if err != nil {
489489
r.logger.Warn("unable to re-allocate ingress IPv4.",
490490
logfields.Error, err,
@@ -511,8 +511,8 @@ func (r *infraIPAllocator) allocateIngressIPs(ctx context.Context, oldV4IngressI
511511
result.CIDRs = r.coalesceCIDRs(result.CIDRs)
512512
}
513513

514-
ingressIPv4 = net.IP(result.IP.AsSlice()).To16()
515-
r.localNodeStore.Update(func(n *node.LocalNode) { n.IPv4IngressIP = net.IP(result.IP.AsSlice()).To16() })
514+
ingressIPv4 = result.IP
515+
r.localNodeStore.Update(func(n *node.LocalNode) { n.IPv4IngressIP = iputil.AddrFrom(result.IP) })
516516
r.logger.Debug("Allocated IPv4 Ingress address", logfields.IPAddr, result.IP)
517517

518518
// In ENI and AlibabaCloud ENI mode, we require the gateway, CIDRs, and the
@@ -549,8 +549,8 @@ func (r *infraIPAllocator) allocateIngressIPs(ctx context.Context, oldV4IngressI
549549

550550
// Reallocate the same address as before, if possible
551551
ingressIPv6 := oldV6IngressIP
552-
if ingressIPv6 != nil {
553-
result, err = r.ipAllocator.AllocateIPWithoutSyncUpstream(iputil.AddrFromIP(ingressIPv6), "ingress", ipam.PoolDefault())
552+
if ingressIPv6.IsValid() {
553+
result, err = r.ipAllocator.AllocateIPWithoutSyncUpstream(ingressIPv6, "ingress", ipam.PoolDefault())
554554
if err != nil {
555555
r.logger.Warn("unable to re-allocate ingress IPv6.",
556556
logfields.Error, err,
@@ -565,9 +565,9 @@ func (r *infraIPAllocator) allocateIngressIPs(ctx context.Context, oldV4IngressI
565565
if result == nil {
566566
result, err = r.allocateNextFromPool(ctx, ipam.IPv6, "ingress")
567567
if err != nil {
568-
if ingressIPv4 != nil {
569-
r.ipAllocator.ReleaseIP(iputil.AddrFromIP(ingressIPv4), ipam.PoolDefault())
570-
r.localNodeStore.Update(func(n *node.LocalNode) { n.IPv4IngressIP = nil })
568+
if ingressIPv4.IsValid() {
569+
r.ipAllocator.ReleaseIP(ingressIPv4, ipam.PoolDefault())
570+
r.localNodeStore.Update(func(n *node.LocalNode) { n.IPv4IngressIP = iputil.Addr{} })
571571
}
572572
return fmt.Errorf("unable to allocate ingress IPs: %w, see https://cilium.link/ipam-range-full", err)
573573
}
@@ -581,7 +581,7 @@ func (r *infraIPAllocator) allocateIngressIPs(ctx context.Context, oldV4IngressI
581581
result.CIDRs = r.coalesceCIDRs(result.CIDRs)
582582
}
583583

584-
r.localNodeStore.Update(func(n *node.LocalNode) { n.IPv6IngressIP = net.IP(result.IP.AsSlice()).To16() })
584+
r.localNodeStore.Update(func(n *node.LocalNode) { n.IPv6IngressIP = iputil.AddrFrom(result.IP) })
585585
r.logger.Debug("Allocated IPv6 Ingress address", logfields.IPAddr, result.IP)
586586
}
587587

@@ -609,7 +609,7 @@ func (r *infraIPAllocator) AllocateIPs(ctx context.Context) error {
609609
return fmt.Errorf("failed to allocate service loopback IPs: %w", err)
610610
}
611611

612-
if err := r.allocateIngressIPs(ctx, localNode.IPv4IngressIP, localNode.IPv6IngressIP); err != nil {
612+
if err := r.allocateIngressIPs(ctx, localNode.IPv4IngressIP.Addr, localNode.IPv6IngressIP.Addr); err != nil {
613613
return fmt.Errorf("failed to allocate ingress IPs: %w", err)
614614
}
615615

‎daemon/infraendpoints/ingress_endpoint.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ func (c *ingressEndpointCreator) createIngressEndpoint(ctx context.Context, heal
8787
return fmt.Errorf("failed to get local node: %w", err)
8888
}
8989

90-
if (c.ipv4Enabled && len(ln.IPv4IngressIP) == 0) ||
91-
(c.ipv6Enabled && len(ln.IPv6IngressIP) == 0) {
90+
if (c.ipv4Enabled && !ln.IPv4IngressIP.IsValid()) ||
91+
(c.ipv6Enabled && !ln.IPv6IngressIP.IsValid()) {
9292
msg := "Ingress IPs are not available, skipping creation of the Ingress Endpoint: Policy enforcement on Cilium Ingress will not work as expected."
9393
c.logger.Warn(msg)
9494
health.Degraded(msg, nil)

‎pkg/ciliumenvoyconfig/cec_resource_parser.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"context"
88
"fmt"
99
"log/slog"
10-
"net"
1110
"strconv"
1211

1312
"github.com/cilium/hive/cell"
@@ -36,6 +35,7 @@ import (
3635
envoyCfg "github.com/cilium/cilium/pkg/envoy/config"
3736
util "github.com/cilium/cilium/pkg/envoy/util"
3837
"github.com/cilium/cilium/pkg/envoy/xds"
38+
iputil "github.com/cilium/cilium/pkg/ip"
3939
"github.com/cilium/cilium/pkg/k8s"
4040
cilium_v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
4141
"github.com/cilium/cilium/pkg/logging/logfields"
@@ -63,8 +63,8 @@ type CECResourceParser struct {
6363
logger *slog.Logger
6464
portAllocator PortAllocator
6565

66-
ingressIPv4 net.IP
67-
ingressIPv6 net.IP
66+
ingressIPv4 iputil.Addr
67+
ingressIPv6 iputil.Addr
6868

6969
defaultMaxConcurrentRetries uint32
7070
defaultMaxConnections uint32
@@ -652,12 +652,12 @@ func (r *CECResourceParser) getBPFMetadataListenerFilter(useOriginalSourceAddr b
652652
// One solution to this dilemma would be to never configure these addresses if
653653
// useOriginalSourceAddr is true and let such traffic fail.
654654
if l7lb {
655-
if r.ingressIPv4 != nil {
655+
if r.ingressIPv4.IsValid() {
656656
conf.Ipv4SourceAddress = r.ingressIPv4.String()
657657
// Enforce ingress policy for Ingress
658658
conf.EnforcePolicyOnL7Lb = true
659659
}
660-
if r.ingressIPv6 != nil {
660+
if r.ingressIPv6.IsValid() {
661661
conf.Ipv6SourceAddress = r.ingressIPv6.String()
662662
// Enforce ingress policy for Ingress
663663
conf.EnforcePolicyOnL7Lb = true

‎pkg/endpoint/creator/creator.go‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,13 @@ package creator
66
import (
77
"context"
88
"fmt"
9-
"net/netip"
109
"os"
1110
"path/filepath"
1211
"strconv"
1312
"sync"
1413

1514
"github.com/cilium/hive/cell"
1615
"github.com/cilium/lumberjack/v2"
17-
"go4.org/netipx"
1816

1917
"github.com/cilium/cilium/api/v1/models"
2018
"github.com/cilium/cilium/pkg/endpoint"
@@ -116,18 +114,13 @@ func (c *endpointCreator) AddIngressEndpoint(ctx context.Context) error {
116114
return fmt.Errorf("failed to get local node: %w", err)
117115
}
118116

119-
// Node.IPv4IngressIP has been parsed with net.ParseIP() and may be in IPv4 mapped IPv6
120-
// address format. Use netipx.FromStdIP() to make sure we get a plain IPv4 address.
121-
ingressIPv4, _ := netipx.FromStdIP(ln.IPv4IngressIP)
122-
ingressIPv6, _ := netip.AddrFromSlice(ln.IPv6IngressIP)
123-
124117
ep, err := endpoint.CreateIngressEndpoint(
125118
c.epParams,
126119
c.params.DNSRulesService,
127120
c.params.Proxy,
128121
c.policyLogger(),
129-
ingressIPv4,
130-
ingressIPv6,
122+
ln.IPv4IngressIP.Addr,
123+
ln.IPv6IngressIP.Addr,
131124
)
132125
if err != nil {
133126
return err

‎pkg/ipcache/restore/local_identity_restorer.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
cmtypes "github.com/cilium/cilium/pkg/clustermesh/types"
1616
"github.com/cilium/cilium/pkg/identity"
1717
identitycell "github.com/cilium/cilium/pkg/identity/cache/cell"
18+
iputil "github.com/cilium/cilium/pkg/ip"
1819
"github.com/cilium/cilium/pkg/ipcache"
1920
ipcachetypes "github.com/cilium/cilium/pkg/ipcache/types"
2021
"github.com/cilium/cilium/pkg/labels"
@@ -196,9 +197,9 @@ func (d *LocalIdentityRestorer) restoreIPCache(ipCache *ipcache.IPCache, localPr
196197
d.params.NodeLocalStore.Update(func(n *node.LocalNode) {
197198
addr := prefix.Addr()
198199
if addr.Is4() {
199-
n.IPv4IngressIP = addr.AsSlice()
200+
n.IPv4IngressIP = iputil.AddrFrom(addr)
200201
} else {
201-
n.IPv6IngressIP = addr.AsSlice()
202+
n.IPv6IngressIP = iputil.AddrFrom(addr)
202203
}
203204
})
204205
d.params.Logger.Info("Restored ingress IP", logfields.Ingress, prefix)

‎pkg/k8s/node.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -276,33 +276,35 @@ func ParseNode(logger *slog.Logger, k8sNode *slim_corev1.Node, source source.Sou
276276
}
277277
}
278278

279-
if newNode.IPv4IngressIP == nil {
279+
if !newNode.IPv4IngressIP.IsValid() {
280280
if ingressIP, ok := annotation.Get(k8sNode, annotation.V4IngressName, annotation.V4IngressNameAlias); !ok || ingressIP == "" {
281281
scopedLog.Debug(
282282
"Empty IPv4 Ingress annotation in node",
283283
)
284-
} else if ip := net.ParseIP(ingressIP); ip == nil {
284+
} else if addr, err := netip.ParseAddr(ingressIP); err != nil {
285285
scopedLog.Error(
286286
"BUG, invalid IPv4 Ingress annotation in node",
287287
logfields.V4IngressIP, ingressIP,
288+
logfields.Error, err,
288289
)
289290
} else {
290-
newNode.IPv4IngressIP = ip
291+
newNode.IPv4IngressIP = iputil.AddrFrom(addr)
291292
}
292293
}
293294

294-
if newNode.IPv6IngressIP == nil {
295+
if !newNode.IPv6IngressIP.IsValid() {
295296
if ingressIP, ok := annotation.Get(k8sNode, annotation.V6IngressName, annotation.V6IngressNameAlias); !ok || ingressIP == "" {
296297
scopedLog.Debug(
297298
"Empty IPv6 Ingress annotation in node",
298299
)
299-
} else if ip := net.ParseIP(ingressIP); ip == nil {
300+
} else if addr, err := netip.ParseAddr(ingressIP); err != nil {
300301
scopedLog.Error(
301302
"BUG, invalid IPv6 Ingress annotation in node",
302303
logfields.V6IngressIP, ingressIP,
304+
logfields.Error, err,
303305
)
304306
} else {
305-
newNode.IPv6IngressIP = ip
307+
newNode.IPv6IngressIP = iputil.AddrFrom(addr)
306308
}
307309
}
308310

@@ -363,8 +365,10 @@ func ParseCiliumNode(n *ciliumv2.CiliumNode) (node nodeTypes.Node) {
363365
node.IPv4HealthIP = iputil.AddrFrom(v4HealthIP)
364366
node.IPv6HealthIP = iputil.AddrFrom(v6HealthIP)
365367

366-
node.IPv4IngressIP = net.ParseIP(n.Spec.IngressAddressing.IPV4)
367-
node.IPv6IngressIP = net.ParseIP(n.Spec.IngressAddressing.IPV6)
368+
v4IngressIP, _ := netip.ParseAddr(n.Spec.IngressAddressing.IPV4)
369+
v6IngressIP, _ := netip.ParseAddr(n.Spec.IngressAddressing.IPV6)
370+
node.IPv4IngressIP = iputil.AddrFrom(v4IngressIP)
371+
node.IPv6IngressIP = iputil.AddrFrom(v6IngressIP)
368372

369373
for _, address := range n.Spec.Addresses {
370374
if ip := net.ParseIP(address.IP); ip != nil {

‎pkg/k8s/node_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ func TestParseCiliumNode(t *testing.T) {
442442
IPv6SecondaryAllocCIDRs: []nodeTypes.Prefix{nodeTypes.PrefixFrom(netip.MustParsePrefix("c0fe::/96"))},
443443
IPv4HealthIP: iputil.AddrFrom(netip.MustParseAddr("1.1.1.1")),
444444
IPv6HealthIP: iputil.AddrFrom(netip.MustParseAddr("c0de::1")),
445-
IPv4IngressIP: net.ParseIP("1.1.1.2"),
446-
IPv6IngressIP: net.ParseIP("c0de::2"),
445+
IPv4IngressIP: iputil.AddrFrom(netip.MustParseAddr("1.1.1.2")),
446+
IPv6IngressIP: iputil.AddrFrom(netip.MustParseAddr("c0de::2")),
447447
}, n)
448448
}

‎pkg/node/manager/manager.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -815,8 +815,8 @@ func (m *manager) NodeUpdated(n nodeTypes.Node) {
815815
healthIPsAdded = append(healthIPsAdded, prefixCluster.AsPrefix())
816816
}
817817

818-
for _, address := range []net.IP{n.IPv4IngressIP, n.IPv6IngressIP} {
819-
prefix := ip.IPToNetPrefix(address)
818+
for _, address := range []netip.Addr{n.IPv4IngressIP.Addr, n.IPv6IngressIP.Addr} {
819+
prefix := netip.PrefixFrom(address, address.BitLen())
820820
if !prefix.IsValid() {
821821
continue
822822
}
@@ -1059,8 +1059,8 @@ func (m *manager) removeNodeFromIPCache(oldNode nodeTypes.Node, resource ipcache
10591059
}
10601060

10611061
// Delete the old ingress IP addresses if they have changed in this node.
1062-
for _, address := range []net.IP{oldNode.IPv4IngressIP, oldNode.IPv6IngressIP} {
1063-
prefix := ip.IPToNetPrefix(address)
1062+
for _, address := range []netip.Addr{oldNode.IPv4IngressIP.Addr, oldNode.IPv6IngressIP.Addr} {
1063+
prefix := netip.PrefixFrom(address, address.BitLen())
10641064
if !prefix.IsValid() || slices.Contains(ingressIPsAdded, prefix) {
10651065
continue
10661066
}

‎pkg/node/store/store_test.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,16 @@ func TestValidatingNode(t *testing.T) {
5656
validator: NameValidator(),
5757
errstr: "name does not match key: got qux, expected fred",
5858
},
59+
{
60+
name: "ingress IPs round-trip",
61+
node: types.Node{
62+
Cluster: "foo",
63+
Name: "qux",
64+
IPv4IngressIP: iputil.AddrFrom(netip.MustParseAddr("10.0.0.5")),
65+
IPv6IngressIP: iputil.AddrFrom(netip.MustParseAddr("f00d::5")),
66+
},
67+
validator: ClusterNameValidator("foo"),
68+
},
5969
{
6070
name: "valid cluster ID",
6171
node: types.Node{Cluster: "foo", Name: "qux", ClusterID: 10},

‎pkg/node/types/node.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,13 @@ type Node struct {
6969
// cilium-health endpoint located on the node.
7070
IPv6HealthIP iputil.Addr
7171

72-
// IPv4IngressIP if not nil, this is the IPv4 address of the
72+
// IPv4IngressIP if set, this is the IPv4 address of the
7373
// Ingress listener on the node.
74-
IPv4IngressIP net.IP
74+
IPv4IngressIP iputil.Addr
7575

76-
// IPv6IngressIP if not nil, this is the IPv6 address of the
76+
// IPv6IngressIP if set, this is the IPv6 address of the
7777
// Ingress listener located on the node.
78-
IPv6IngressIP net.IP
78+
IPv6IngressIP iputil.Addr
7979

8080
// ClusterID is the unique identifier of the cluster
8181
ClusterID uint32
@@ -384,15 +384,15 @@ func (n *Node) getHealthAddresses() *models.NodeAddressing {
384384
}
385385

386386
func (n *Node) getIngressAddresses() *models.NodeAddressing {
387-
if n.IPv4IngressIP == nil && n.IPv6IngressIP == nil {
387+
if !n.IPv4IngressIP.IsValid() && !n.IPv6IngressIP.IsValid() {
388388
return nil
389389
}
390390

391391
var v4Str, v6Str string
392-
if n.IPv4IngressIP != nil {
392+
if n.IPv4IngressIP.IsValid() {
393393
v4Str = n.IPv4IngressIP.String()
394394
}
395-
if n.IPv6IngressIP != nil {
395+
if n.IPv6IngressIP.IsValid() {
396396
v6Str = n.IPv6IngressIP.String()
397397
}
398398

0 commit comments

Comments
 (0)