pkg/k8s: do not read k8s node annotations if they are not written

When there is an annotation in the k8s node object, the annotation `io.cilium.network.ipv4-cilium-host` is used as the CiliumInternal IP address of the CiliumNode object in [1]. Whenever Cilium is updating any state into the CiliumNode it retrieves all IP address from k8s node, including the ones from annotations, and appends the local node's IP addresses, including the newly correct internal / router IP address, in [2]. Since this is a list, the annotation's IP address is always used first and all other Cilium agents will wrongly use it for any operation. [1] https://github.com/cilium/cilium/blob/927bd8c26904ff92e42c61cec6d00ea8ac062c05/pkg/nodediscovery/nodediscovery.go#L453-L459 [2] https://github.com/cilium/cilium/blob/927bd8c26904ff92e42c61cec6d00ea8ac062c05/pkg/nodediscovery/nodediscovery.go#L474-L489 Fixes: 73d6cae ("install: default AnnotateK8sNode to false") Signed-off-by: André Martins <andre@cilium.io>
tklauser · Nov 22, 2022 · 0696874 · 0696874
1 parent d5ada37
commit 0696874
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 33 deletions.
diff --git a/pkg/k8s/init_test.go b/pkg/k8s/init_test.go
@@ -20,10 +20,17 @@ import (
 	"github.com/cilium/cilium/pkg/checker"
 	slim_corev1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/api/core/v1"
 	"github.com/cilium/cilium/pkg/node"
+	"github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/source"
 )
 
 func (s *K8sSuite) TestUseNodeCIDR(c *C) {
+	prevAnnotateK8sNode := option.Config.AnnotateK8sNode
+	option.Config.AnnotateK8sNode = true
+	defer func() {
+		option.Config.AnnotateK8sNode = prevAnnotateK8sNode
+	}()
+
 	// Test IPv4
 	node1 := v1.Node{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/pkg/k8s/node.go b/pkg/k8s/node.go
@@ -77,38 +77,11 @@ func ParseNode(k8sNode *slim_corev1.Node, source source.Source) *nodeTypes.Node
 		}
 		addrs = append(addrs, na)
 	}
-
-	k8sNodeAddHostIP := func(annotation string) {
-		if ciliumInternalIP, ok := k8sNode.Annotations[annotation]; !ok || ciliumInternalIP == "" {
-			scopedLog.Debugf("Missing %s. Annotation required when IPSec Enabled", annotation)
-		} else if ip := net.ParseIP(ciliumInternalIP); ip == nil {
-			scopedLog.Debugf("ParseIP %s error", ciliumInternalIP)
-		} else {
-			na := nodeTypes.Address{
-				Type: addressing.NodeCiliumInternalIP,
-				IP:   ip,
-			}
-			addrs = append(addrs, na)
-			scopedLog.Debugf("Add NodeCiliumInternalIP: %s", ip)
-		}
-	}
-
-	k8sNodeAddHostIP(annotation.CiliumHostIP)
-	k8sNodeAddHostIP(annotation.CiliumHostIPv6)
-
-	encryptKey := uint8(0)
-	if key, ok := k8sNode.Annotations[annotation.CiliumEncryptionKey]; ok {
-		if u, err := strconv.ParseUint(key, 10, 8); err == nil {
-			encryptKey = uint8(u)
-		}
-	}
-
 	newNode := &nodeTypes.Node{
-		Name:          k8sNode.Name,
-		Cluster:       option.Config.ClusterName,
-		IPAddresses:   addrs,
-		Source:        source,
-		EncryptionKey: encryptKey,
+		Name:        k8sNode.Name,
+		Cluster:     option.Config.ClusterName,
+		IPAddresses: addrs,
+		Source:      source,
 	}
 
 	if len(k8sNode.Spec.PodCIDRs) != 0 {
@@ -138,6 +111,40 @@ func ParseNode(k8sNode *slim_corev1.Node, source source.Source) *nodeTypes.Node
 			}
 		}
 	}
+
+	newNode.Labels = k8sNode.GetLabels()
+
+	if !option.Config.AnnotateK8sNode {
+		return newNode
+	}
+
+	// Any code bellow this line will depend on k8s node annotations. If we are
+	// not annotating the node then we should not use any annotations.
+
+	k8sNodeAddHostIP := func(annotation string) {
+		if ciliumInternalIP, ok := k8sNode.Annotations[annotation]; !ok || ciliumInternalIP == "" {
+			scopedLog.Debugf("Missing %s. Annotation required when IPSec Enabled", annotation)
+		} else if ip := net.ParseIP(ciliumInternalIP); ip == nil {
+			scopedLog.Debugf("ParseIP %s error", ciliumInternalIP)
+		} else {
+			na := nodeTypes.Address{
+				Type: addressing.NodeCiliumInternalIP,
+				IP:   ip,
+			}
+			addrs = append(addrs, na)
+			scopedLog.Debugf("Add NodeCiliumInternalIP: %s", ip)
+		}
+	}
+
+	k8sNodeAddHostIP(annotation.CiliumHostIP)
+	k8sNodeAddHostIP(annotation.CiliumHostIPv6)
+
+	if key, ok := k8sNode.Annotations[annotation.CiliumEncryptionKey]; ok {
+		if u, err := strconv.ParseUint(key, 10, 8); err == nil {
+			newNode.EncryptionKey = uint8(u)
+		}
+	}
+
 	// Spec.PodCIDR takes precedence since it's
 	// the CIDR assigned by k8s controller manager
 	// In case it's invalid or empty then we fall back to our annotations.
@@ -207,7 +214,5 @@ func ParseNode(k8sNode *slim_corev1.Node, source source.Source) *nodeTypes.Node
 		}
 	}
 
-	newNode.Labels = k8sNode.GetLabels()
-
 	return newNode
 }
diff --git a/pkg/k8s/node_test.go b/pkg/k8s/node_test.go
@@ -13,10 +13,17 @@ import (
 	slim_corev1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/api/core/v1"
 	slim_metav1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/apis/meta/v1"
 	nodeAddressing "github.com/cilium/cilium/pkg/node/addressing"
+	"github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/source"
 )
 
 func (s *K8sSuite) TestParseNode(c *C) {
+	prevAnnotateK8sNode := option.Config.AnnotateK8sNode
+	option.Config.AnnotateK8sNode = true
+	defer func() {
+		option.Config.AnnotateK8sNode = prevAnnotateK8sNode
+	}()
+
 	// PodCIDR takes precedence over annotations
 	k8sNode := &slim_corev1.Node{
 		ObjectMeta: slim_metav1.ObjectMeta{
@@ -82,6 +89,57 @@ func (s *K8sSuite) TestParseNode(c *C) {
 	c.Assert(n.IPv6AllocCIDR.String(), Equals, "f00d:aaaa:bbbb:cccc:dddd:eeee::/112")
 }
 
+func (s *K8sSuite) TestParseNodeWithoutAnnotations(c *C) {
+	prevAnnotateK8sNode := option.Config.AnnotateK8sNode
+	option.Config.AnnotateK8sNode = false
+	defer func() {
+		option.Config.AnnotateK8sNode = prevAnnotateK8sNode
+	}()
+
+	// PodCIDR takes precedence over annotations
+	k8sNode := &slim_corev1.Node{
+		ObjectMeta: slim_metav1.ObjectMeta{
+			Name: "node1",
+			Annotations: map[string]string{
+				annotation.V4CIDRName: "10.254.0.0/16",
+				annotation.V6CIDRName: "f00d:aaaa:bbbb:cccc:dddd:eeee::/112",
+			},
+			Labels: map[string]string{
+				"type": "m5.xlarge",
+			},
+		},
+		Spec: slim_corev1.NodeSpec{
+			PodCIDR: "10.1.0.0/16",
+		},
+	}
+
+	n := ParseNode(k8sNode, source.Local)
+	c.Assert(n.Name, Equals, "node1")
+	c.Assert(n.IPv4AllocCIDR, NotNil)
+	c.Assert(n.IPv4AllocCIDR.String(), Equals, "10.1.0.0/16")
+	c.Assert(n.IPv6AllocCIDR, IsNil)
+	c.Assert(n.Labels["type"], Equals, "m5.xlarge")
+
+	// No IPv6 annotation but PodCIDR with v6
+	k8sNode = &slim_corev1.Node{
+		ObjectMeta: slim_metav1.ObjectMeta{
+			Name: "node2",
+			Annotations: map[string]string{
+				annotation.V4CIDRName: "10.254.0.0/16",
+			},
+		},
+		Spec: slim_corev1.NodeSpec{
+			PodCIDR: "f00d:aaaa:bbbb:cccc:dddd:eeee::/112",
+		},
+	}
+
+	n = ParseNode(k8sNode, source.Local)
+	c.Assert(n.Name, Equals, "node2")
+	c.Assert(n.IPv4AllocCIDR, IsNil)
+	c.Assert(n.IPv6AllocCIDR, NotNil)
+	c.Assert(n.IPv6AllocCIDR.String(), Equals, "f00d:aaaa:bbbb:cccc:dddd:eeee::/112")
+}
+
 func Test_ParseNodeAddressType(t *testing.T) {
 	type args struct {
 		k8sNodeType slim_corev1.NodeAddressType