Repository: hetznercloud/hcloud-cloud-controller-manager
Pull request #865 (2025-02-10 11:43:53)

feat(robot): forward InternalIPs by default on Robot nodes
Whenever a user configures the `--node-ip` flag, we forward the configured IPs. We validate if the IP already exists as an ExternalIP and does not collide with the configured address family. This improves the Robot support for HCCM, as users can configure private IPs for Robot nodes.

Forwarding only happens during the initialization phase of HCCM and should therefore not break existing clusters.

This feature can be disabled by setting the environment variable `ROBOT_FORWARD_INTERNAL_IPS` to `false`.

Co-authored-by: Alexander Block <ablock84@gmail.com>

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/hetznercloud/hcloud-cloud-controller-manager/pull/865?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) Report
Attention: Patch coverage is `78.57143%` with `3 lines` in your changes missing coverage. Please review.
> Project coverage is 70.70%. Comparing base [(`e777d81`)](https://app.codecov.io/gh/hetznercloud/hcloud-cloud-controller-manager/commit/e777d81c9145dbaa03e4a2265ba20cc32da11811?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) to head [(`11a8c73`)](https://app.codecov.io/gh/hetznercloud/hcloud-cloud-controller-manager/commit/11a8c7357dafc59f18e1085be4500d85e12773f2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud).

| [Files with missing lines](https://app.codecov.io/gh/hetznercloud/hcloud-cloud-controller-manager/pull/865?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) | Patch % | Lines |
|---|---|---|
| [internal/config/config.go](https://app.codecov.io/gh/hetznercloud/hcloud-cloud-controller-manager/pull/865?src=pr&el=tree&filepath=internal%2Fconfig%2Fconfig.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud#diff-aW50ZXJuYWwvY29uZmlnL2NvbmZpZy5nbw==) | 40.00% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/hetznercloud/hcloud-cloud-controller-manager/pull/865?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) |

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main     #865   +/-   ##
=======================================
  Coverage   70.69%   70.70%           
=======================================
  Files          31       31           
  Lines        3310     3321   +11     
=======================================
+ Hits         2340     2348    +8     
- Misses        795      797    +2     
- Partials      175      176    +1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/hetznercloud/hcloud-cloud-controller-manager/pull/865?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud).


Patch:
diff --git a/docs/robot.md b/docs/robot.md
index 76dc84f0..36c9a50c 100644
--- a/docs/robot.md
+++ b/docs/robot.md
@@ -56,6 +56,7 @@ The Node controller adds information about the server to the Node object. The va
 - Addresses
   - We add the Hostname and (depending on the configuration and availability) the IPv4 and IPv6 addresses of the server in `Node.status.addresses`.
   - For the IPv6 address we use the first address in the Network -> For the network `2a01:f48:111:4221::` we add the address `2a01:f48:111:4221::1`.
+  - By default, we pass along InternalIPs configured via the kubelet flag `--node-ip`. This can be disabled by setting the environment variable `ROBOT_FORWARD_INTERNAL_IPS` to `false`. It is not allowed to configure the same IP for InternalIP and ExternalIP.
   - Private IPs in a vSwitch are not supported.
 
 ### Node Lifecycle Controller
diff --git a/hcloud/instances.go b/hcloud/instances.go
index 7e0e5373..dbed05cc 100644
--- a/hcloud/instances.go
+++ b/hcloud/instances.go
@@ -20,11 +20,13 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 
 	hrobotmodels "github.com/syself/hrobot-go/models"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/tools/record"
 	cloudprovider "k8s.io/cloud-provider"
+	"k8s.io/klog/v2"
 
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config"
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics"
@@ -34,7 +36,8 @@ import (
 )
 
 const (
-	ProvidedBy = "instance.hetzner.cloud/provided-by"
+	ProvidedBy              = "instance.hetzner.cloud/provided-by"
+	MisconfiguredInternalIP = "MisconfiguredInternalIP"
 )
 
 type instances struct {
@@ -106,7 +109,7 @@ func (i *instances) lookupServer(
 			return nil, nil
 		}
 
-		return robotServer{server, i.robotClient}, nil
+		return robotServer{server, i.robotClient, i.recorder}, nil
 	}
 
 	// If the node has no provider ID we try to find the server by name from
@@ -139,7 +142,7 @@ func (i *instances) lookupServer(
 	case cloudServer != nil:
 		return hcloudServer{cloudServer}, nil
 	case hrobotServer != nil:
-		return robotServer{hrobotServer, i.robotClient}, nil
+		return robotServer{hrobotServer, i.robotClient, i.recorder}, nil
 	default:
 		// Both nil
 		return nil, nil
@@ -196,7 +199,7 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *corev1.Node) (*c
 			op, node.Name, errServerNotFound)
 	}
 
-	metadata, err := server.Metadata(i.networkID, i.cfg)
+	metadata, err := server.Metadata(i.networkID, node, i.cfg)
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", op, err)
 	}
@@ -254,13 +257,12 @@ func hcloudNodeAddresses(
 
 func robotNodeAddresses(
 	server *hrobotmodels.Server,
+	node *corev1.Node,
 	cfg config.HCCMConfiguration,
+	recorder record.EventRecorder,
 ) []corev1.NodeAddress {
 	var addresses []corev1.NodeAddress
-	addresses = append(
-		addresses,
-		corev1.NodeAddress{Type: corev1.NodeHostName, Address: server.Name},
-	)
+	addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeHostName, Address: server.Name})
 
 	dualStack := cfg.Instance.AddressFamily == config.AddressFamilyDualStack
 	ipv4 := cfg.Instance.AddressFamily == config.AddressFamilyIPv4 || dualStack
@@ -269,18 +271,56 @@ func robotNodeAddresses(
 	if ipv6 {
 		// For a given IPv6 network of 2a01:f48:111:4221::, the instance address is 2a01:f48:111:4221::1
 		hostAddress := server.ServerIPv6Net + "1"
-
-		addresses = append(
-			addresses,
-			corev1.NodeAddress{Type: corev1.NodeExternalIP, Address: hostAddress},
-		)
+		addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeExternalIP, Address: hostAddress})
 	}
 
 	if ipv4 {
-		addresses = append(
-			addresses,
-			corev1.NodeAddress{Type: corev1.NodeExternalIP, Address: server.ServerIP},
-		)
+		addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeExternalIP, Address: server.ServerIP})
+	}
+
+	if cfg.Robot.ForwardInternalIPs {
+	OUTER:
+		for _, currentAddress := range node.Status.Addresses {
+			if currentAddress.Type != corev1.NodeInternalIP {
+				continue
+			}
+
+			ip := net.ParseIP(currentAddress.Address)
+			isIPv4 := ip.To4() != nil
+
+			var warnMsg string
+			if isIPv4 && ipv6 && !dualStack {
+				warnMsg = fmt.Sprintf(
+					"Configured InternalIP is IPv4 even though IPv6 only is configured. As a result, %s is not added as an InternalIP",
+					currentAddress.Address,
+				)
+			} else if !isIPv4 && ipv4 && !dualStack {
+				warnMsg = fmt.Sprintf(
+					"Configured InternalIP is IPv6 even though IPv4 only is configured. As a result, %s is not added as an InternalIP",
+					currentAddress.Address,
+				)
+			}
+
+			if warnMsg != "" {
+				recorder.Event(node, corev1.EventTypeWarning, MisconfiguredInternalIP, warnMsg)
+				klog.Warning(warnMsg)
+				continue
+			}
+
+			for _, address := range addresses {
+				if currentAddress.Address == address.Address {
+					warnMsg := fmt.Sprintf(
+						"Configured InternalIP already exists as an ExternalIP. As a result, %s is not added as an InternalIP",
+						currentAddress.Address,
+					)
+					recorder.Event(node, corev1.EventTypeWarning, MisconfiguredInternalIP, warnMsg)
+					klog.Warning(warnMsg)
+					continue OUTER
+				}
+			}
+
+			addresses = append(addresses, currentAddress)
+		}
 	}
 
 	return addresses
@@ -290,6 +330,7 @@ type genericServer interface {
 	IsShutdown() (bool, error)
 	Metadata(
 		networkID int64,
+		node *corev1.Node,
 		cfg config.HCCMConfiguration,
 	) (*cloudprovider.InstanceMetadata, error)
 }
@@ -302,7 +343,7 @@ func (s hcloudServer) IsShutdown() (bool, error) {
 	return s.Status == hcloud.ServerStatusOff, nil
 }
 
-func (s hcloudServer) Metadata(networkID int64, cfg config.HCCMConfiguration) (*cloudprovider.InstanceMetadata, error) {
+func (s hcloudServer) Metadata(networkID int64, _ *corev1.Node, cfg config.HCCMConfiguration) (*cloudprovider.InstanceMetadata, error) {
 	return &cloudprovider.InstanceMetadata{
 		ProviderID:    providerid.FromCloudServerID(s.ID),
 		InstanceType:  s.ServerType.Name,
@@ -318,6 +359,7 @@ func (s hcloudServer) Metadata(networkID int64, cfg config.HCCMConfiguration) (*
 type robotServer struct {
 	*hrobotmodels.Server
 	robotClient robot.Client
+	recorder    record.EventRecorder
 }
 
 func (s robotServer) IsShutdown() (bool, error) {
@@ -331,11 +373,11 @@ func (s robotServer) IsShutdown() (bool, error) {
 	return resetStatus.OperatingStatus == "shut off", nil
 }
 
-func (s robotServer) Metadata(_ int64, cfg config.HCCMConfiguration) (*cloudprovider.InstanceMetadata, error) {
+func (s robotServer) Metadata(_ int64, node *corev1.Node, cfg config.HCCMConfiguration) (*cloudprovider.InstanceMetadata, error) {
 	return &cloudprovider.InstanceMetadata{
 		ProviderID:    providerid.FromRobotServerNumber(s.ServerNumber),
 		InstanceType:  getInstanceTypeOfRobotServer(s.Server),
-		NodeAddresses: robotNodeAddresses(s.Server, cfg),
+		NodeAddresses: robotNodeAddresses(s.Server, node, cfg, s.recorder),
 		Zone:          getZoneOfRobotServer(s.Server),
 		Region:        getRegionOfRobotServer(s.Server),
 		AdditionalLabels: map[string]string{
diff --git a/hcloud/instances_util.go b/hcloud/instances_util.go
index b643e610..19b72a3c 100644
--- a/hcloud/instances_util.go
+++ b/hcloud/instances_util.go
@@ -24,12 +24,33 @@ import (
 
 	hrobotmodels "github.com/syself/hrobot-go/models"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics"
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/robot"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 )
 
+type MockEventRecorder struct{}
+
+func (er *MockEventRecorder) Event(_ runtime.Object, _, _, _ string) {
+}
+
+func (er *MockEventRecorder) Eventf(
+	_ runtime.Object,
+	_, _, _ string,
+	_ ...interface{},
+) {
+}
+
+func (er *MockEventRecorder) AnnotatedEventf(
+	_ runtime.Object,
+	_ map[string]string,
+	_, _, _ string,
+	_ ...interface{},
+) {
+}
+
 func getCloudServerByName(ctx context.Context, c *hcloud.Client, name string) (*hcloud.Server, error) {
 	const op = "hcloud/getCloudServerByName"
 	metrics.OperationCalled.WithLabelValues(op).Inc()
diff --git a/internal/config/config.go b/internal/config/config.go
index 3e73a005..44ad6590 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -18,11 +18,12 @@ const (
 	hcloudNetwork  = "HCLOUD_NETWORK"
 	hcloudDebug    = "HCLOUD_DEBUG"
 
-	robotEnabled           = "ROBOT_ENABLED"
-	robotUser              = "ROBOT_USER"
-	robotPassword          = "ROBOT_PASSWORD"
-	robotCacheTimeout      = "ROBOT_CACHE_TIMEOUT"
-	robotRateLimitWaitTime = "ROBOT_RATE_LIMIT_WAIT_TIME"
+	robotEnabled            = "ROBOT_ENABLED"
+	robotUser               = "ROBOT_USER"
+	robotPassword           = "ROBOT_PASSWORD"
+	robotCacheTimeout       = "ROBOT_CACHE_TIMEOUT"
+	robotRateLimitWaitTime  = "ROBOT_RATE_LIMIT_WAIT_TIME"
+	robotForwardInternalIPs = "ROBOT_FORWARD_INTERNAL_IPS"
 
 	hcloudInstancesAddressFamily = "HCLOUD_INSTANCES_ADDRESS_FAMILY"
 
@@ -53,6 +54,8 @@ type RobotConfiguration struct {
 	Password          string
 	CacheTimeout      time.Duration
 	RateLimitWaitTime time.Duration
+	// ForwardInternalIPs is enabled by default.
+	ForwardInternalIPs bool
 }
 
 type MetricsConfiguration struct {
@@ -143,6 +146,12 @@ func Read() (HCCMConfiguration, error) {
 	if err != nil {
 		errs = append(errs, err)
 	}
+	cfg.Robot.ForwardInternalIPs, err = getEnvBool(robotForwardInternalIPs, true)
+	if err != nil {
+		errs = append(errs, err)
+	}
+	// Robot needs to be enabled
+	cfg.Robot.ForwardInternalIPs = cfg.Robot.ForwardInternalIPs && cfg.Robot.Enabled
 
 	cfg.Metrics.Enabled, err = getEnvBool(hcloudMetricsEnabled, true)
 	if err != nil {


Test patch:
diff --git a/hcloud/instances_test.go b/hcloud/instances_test.go
index 8abbe9fb..9c08d57f 100644
--- a/hcloud/instances_test.go
+++ b/hcloud/instances_test.go
@@ -598,11 +598,12 @@ func TestNodeAddresses(t *testing.T) {
 
 func TestNodeAddressesRobotServer(t *testing.T) {
 	tests := []struct {
-		name           string
-		addressFamily  config.AddressFamily
-		server         *hrobotmodels.Server
-		privateNetwork int
-		expected       []corev1.NodeAddress
+		name                    string
+		addressFamily           config.AddressFamily
+		server                  *hrobotmodels.Server
+		nodeStatusNodeAddresses []corev1.NodeAddress
+		privateNetwork          int
+		expected                []corev1.NodeAddress
 	}{
 		{
 			name:          "public ipv4",
@@ -644,6 +645,61 @@ func TestNodeAddressesRobotServer(t *testing.T) {
 				{Type: corev1.NodeExternalIP, Address: "203.0.113.7"},
 			},
 		},
+		{
+			name:          "public ipv4 and internal ip",
+			addressFamily: config.AddressFamilyIPv4,
+			nodeStatusNodeAddresses: []corev1.NodeAddress{
+				{
+					Type:    corev1.NodeInternalIP,
+					Address: "10.0.1.2",
+				},
+			},
+			server: &hrobotmodels.Server{
+				Name:     "foobar",
+				ServerIP: "203.0.113.7",
+			},
+			expected: []corev1.NodeAddress{
+				{Type: corev1.NodeHostName, Address: "foobar"},
+				{Type: corev1.NodeExternalIP, Address: "203.0.113.7"},
+				{Type: corev1.NodeInternalIP, Address: "10.0.1.2"},
+			},
+		},
+		{
+			name:          "configured InternalIP is also ExternalIP",
+			addressFamily: config.AddressFamilyIPv4,
+			nodeStatusNodeAddresses: []corev1.NodeAddress{
+				{
+					Type:    corev1.NodeInternalIP,
+					Address: "203.0.113.7",
+				},
+			},
+			server: &hrobotmodels.Server{
+				Name:     "foobar",
+				ServerIP: "203.0.113.7",
+			},
+			expected: []corev1.NodeAddress{
+				{Type: corev1.NodeHostName, Address: "foobar"},
+				{Type: corev1.NodeExternalIP, Address: "203.0.113.7"},
+			},
+		},
+		{
+			name:          "configured InternalIP does not fit configured AddressFamily",
+			addressFamily: config.AddressFamilyIPv4,
+			nodeStatusNodeAddresses: []corev1.NodeAddress{
+				{
+					Type:    corev1.NodeInternalIP,
+					Address: "2001:db8:1234::",
+				},
+			},
+			server: &hrobotmodels.Server{
+				Name:     "foobar",
+				ServerIP: "203.0.113.7",
+			},
+			expected: []corev1.NodeAddress{
+				{Type: corev1.NodeHostName, Address: "foobar"},
+				{Type: corev1.NodeExternalIP, Address: "203.0.113.7"},
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -653,7 +709,16 @@ func TestNodeAddressesRobotServer(t *testing.T) {
 			cfg, err := config.Read()
 			assert.NoError(t, err)
 
-			addresses := robotNodeAddresses(test.server, cfg)
+			node := &corev1.Node{
+				Status: corev1.NodeStatus{
+					Addresses: []corev1.NodeAddress{},
+				},
+			}
+			if test.nodeStatusNodeAddresses != nil {
+				node.Status.Addresses = test.nodeStatusNodeAddresses
+			}
+
+			addresses := robotNodeAddresses(test.server, node, cfg, &MockEventRecorder{})
 
 			if !reflect.DeepEqual(addresses, test.expected) {
 				t.Fatalf("%s: expected addresses %+v but got %+v", test.name, test.expected, addresses)
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 25ca85af..a28137c2 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -64,11 +64,12 @@ func TestRead(t *testing.T) {
 			want: HCCMConfiguration{
 				HCloudClient: HCloudClientConfiguration{Token: "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq"},
 				Robot: RobotConfiguration{
-					Enabled:           false,
-					User:              "foobar",
-					Password:          "secret-password",
-					CacheTimeout:      5 * time.Minute,
-					RateLimitWaitTime: 0,
+					Enabled:            false,
+					User:               "foobar",
+					Password:           "secret-password",
+					CacheTimeout:       5 * time.Minute,
+					RateLimitWaitTime:  0,
+					ForwardInternalIPs: false,
 				},
 				Metrics:      MetricsConfiguration{Enabled: true, Address: ":8233"},
 				Instance:     InstanceConfiguration{AddressFamily: AddressFamilyIPv4},
@@ -128,11 +129,37 @@ failed to read ROBOT_PASSWORD_FILE: open /tmp/hetzner-password: no such file or
 			},
 			want: HCCMConfiguration{
 				Robot: RobotConfiguration{
-					Enabled:           true,
-					User:              "foobar",
-					Password:          "secret-password",
-					CacheTimeout:      1 * time.Minute,
-					RateLimitWaitTime: 5 * time.Minute,
+					Enabled:            true,
+					User:               "foobar",
+					Password:           "secret-password",
+					CacheTimeout:       1 * time.Minute,
+					RateLimitWaitTime:  5 * time.Minute,
+					ForwardInternalIPs: true,
+				},
+				Metrics:      MetricsConfiguration{Enabled: true, Address: ":8233"},
+				Instance:     InstanceConfiguration{AddressFamily: AddressFamilyIPv4},
+				LoadBalancer: LoadBalancerConfiguration{Enabled: true},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "robot disable forward internal ips",
+			env: []string{
+				"ROBOT_ENABLED", "true",
+				"ROBOT_USER", "foobar",
+				"ROBOT_PASSWORD", "secret-password",
+				"ROBOT_RATE_LIMIT_WAIT_TIME", "5m",
+				"ROBOT_CACHE_TIMEOUT", "1m",
+				"ROBOT_FORWARD_INTERNAL_IPS", "false",
+			},
+			want: HCCMConfiguration{
+				Robot: RobotConfiguration{
+					Enabled:            true,
+					User:               "foobar",
+					Password:           "secret-password",
+					CacheTimeout:       1 * time.Minute,
+					RateLimitWaitTime:  5 * time.Minute,
+					ForwardInternalIPs: false,
 				},
 				Metrics:      MetricsConfiguration{Enabled: true, Address: ":8233"},
 				Instance:     InstanceConfiguration{AddressFamily: AddressFamilyIPv4},