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_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/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 { 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},