Repository: hetznercloud/hcloud-cloud-controller-manager (go)

Issue:
LoadBalancer does not populate IP field when `disable-public-network` is set.
I have created a service of type `LoadBalancer`, with the annotation `load-balancer.hetzner.cloud/disable-public-network: "true"`. The load-balancer has a private IP address assigned on my cluster's network (`172.29.0.3`), and responds correctly on this address. But, the status does not contain the LB's IP address:

```yaml
  status:
    loadBalancer: {}
```

When I create a service without the annotation, the status is correctly updated:

```yaml
  status:
    loadBalancer:
      ingress:
      - ip: 167.235.<snip>
      - ip: 2a01:4f8:<snip>
```

I think even if public access is disabled, the external IP should still be populated with the service's private network address, so that servers on my HCloud network (but outside the k8s cluster) can extract the address they can reach the service on. Also, without the load-balancer IP address, `external-dns` fails to add a record for this load-balancer.

Pull request:
fix: populate ingress private ip when disable-private-ingress is false
Closes #553

Refactor getting the ingresses for a load-balancer for both get load-balancer and ensure load-balancer, into using a shared function. This ensures we are consistent between the 2 methods.


Release notes:
The load balancer ingress IPs are now populated with the private IPs, unless the `load-balancer.hetzner.cloud/disable-private-ingress` annotation is set to `true`. Please make sure that you configured the annotation according to your needs, for example if you are using `external-dns`.



Patch:
diff --git a/go.mod b/go.mod
index a4a59ffe..f31bd450 100644
--- a/go.mod
+++ b/go.mod
@@ -6,7 +6,7 @@ toolchain go1.22.6
 
 require (
 	github.com/hetznercloud/hcloud-go/v2 v2.13.1
-	github.com/prometheus/client_golang v1.19.1
+	github.com/prometheus/client_golang v1.20.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.9.0
 	github.com/syself/hrobot-go v0.2.5
@@ -56,6 +56,8 @@ require (
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
+	github.com/klauspost/compress v1.17.9 // indirect
+	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
 	github.com/moby/term v0.5.0 // indirect
diff --git a/go.sum b/go.sum
index af10bafe..7a5659f3 100644
--- a/go.sum
+++ b/go.sum
@@ -136,6 +136,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr
 github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
+github.com/klauspost/compress v1.17.9 h1:6KIumPrER1LHsvBVuDa0r5xaG0Es51mhhB9BQB2qeMA=
+github.com/klauspost/compress v1.17.9/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw=
 github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
 github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
 github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
@@ -143,6 +145,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
+github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
 github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
 github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg=
@@ -172,6 +176,8 @@ github.com/prometheus/client_golang v1.18.0 h1:HzFfmkOzH5Q8L8G+kSJKUx5dtG87sewO+
 github.com/prometheus/client_golang v1.18.0/go.mod h1:T+GXkCk5wSJyOqMIzVgvvjFDlkOQntgjkJWKrN5txjA=
 github.com/prometheus/client_golang v1.19.1 h1:wZWJDwK+NameRJuPGDhlnFgx8e8HN3XHQeLaYJFJBOE=
 github.com/prometheus/client_golang v1.19.1/go.mod h1:mP78NwGzrVks5S2H6ab8+ZZGJLZUq1hoULYBAYBw1Ho=
+github.com/prometheus/client_golang v1.20.0 h1:jBzTZ7B099Rg24tny+qngoynol8LtVYlA2bqx3vEloI=
+github.com/prometheus/client_golang v1.20.0/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE=
 github.com/prometheus/client_model v0.5.0 h1:VQw1hfvPvk3Uv6Qf29VrPF32JB6rtbgI6cYPYQjL0Qw=
 github.com/prometheus/client_model v0.5.0/go.mod h1:dTiFglRmd66nLR9Pv9f0mZi7B7fk5Pm3gvsjB5tr+kI=
 github.com/prometheus/client_model v0.6.1 h1:ZKSh/rekM+n3CeS952MLRAdFwIKqeY8b62p8ais2e9E=
diff --git a/hcloud/load_balancers.go b/hcloud/load_balancers.go
index cf0e478a..8393a4e6 100644
--- a/hcloud/load_balancers.go
+++ b/hcloud/load_balancers.go
@@ -86,23 +86,12 @@ func (l *loadBalancers) GetLoadBalancer(
 		}, true, nil
 	}
 
-	ingresses := []corev1.LoadBalancerIngress{
-		{
-			IP: lb.PublicNet.IPv4.IP.String(),
-		},
-	}
-
-	disableIPV6, err := l.getDisableIPv6(service)
+	ingress, err := l.buildLoadBalancerStatusIngress(lb, service)
 	if err != nil {
 		return nil, false, fmt.Errorf("%s: %v", op, err)
 	}
-	if !disableIPV6 {
-		ingresses = append(ingresses, corev1.LoadBalancerIngress{
-			IP: lb.PublicNet.IPv6.IP.String(),
-		})
-	}
 
-	return &corev1.LoadBalancerStatus{Ingress: ingresses}, true, nil
+	return &corev1.LoadBalancerStatus{Ingress: ingress}, true, nil
 }
 
 func (l *loadBalancers) GetLoadBalancerName(_ context.Context, _ string, service *corev1.Service) string {
@@ -203,6 +192,18 @@ func (l *loadBalancers) EnsureLoadBalancer(
 		}, nil
 	}
 
+	ingress, err := l.buildLoadBalancerStatusIngress(lb, svc)
+	if err != nil {
+		return nil, fmt.Errorf("%s: %w", op, err)
+	}
+
+	return &corev1.LoadBalancerStatus{Ingress: ingress}, nil
+}
+
+func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer, svc *corev1.Service) ([]corev1.LoadBalancerIngress, error) {
+	const op = "hcloud/loadBalancers.getLoadBalancerStatusIngress"
+	metrics.OperationCalled.WithLabelValues(op).Inc()
+
 	var ingress []corev1.LoadBalancerIngress
 
 	disablePubNet, err := annotation.LBDisablePublicNetwork.BoolFromService(svc)
@@ -226,13 +227,14 @@ func (l *loadBalancers) EnsureLoadBalancer(
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", op, err)
 	}
+
 	if !disablePrivIngress {
-		for _, nw := range lb.PrivateNet {
-			ingress = append(ingress, corev1.LoadBalancerIngress{IP: nw.IP.String()})
+		for _, privateNet := range lb.PrivateNet {
+			ingress = append(ingress, corev1.LoadBalancerIngress{IP: privateNet.IP.String()})
 		}
 	}
 
-	return &corev1.LoadBalancerStatus{Ingress: ingress}, nil
+	return ingress, nil
 }
 
 func (l *loadBalancers) getDisablePrivateIngress(svc *corev1.Service) (bool, error) {


Test patch:
diff --git a/hcloud/load_balancers_test.go b/hcloud/load_balancers_test.go
index 1da764de..04bdf9bd 100644
--- a/hcloud/load_balancers_test.go
+++ b/hcloud/load_balancers_test.go
@@ -105,6 +105,88 @@ func TestLoadBalancers_GetLoadBalancer(t *testing.T) {
 				assert.Equal(t, "hostname", status.Ingress[0].Hostname)
 			},
 		},
+		{
+			Name:       "get load balancer with private network",
+			ServiceUID: "1",
+			LB: &hcloud.LoadBalancer{
+				ID:               1,
+				Name:             "with-priv-net",
+				LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"},
+				Location:         &hcloud.Location{Name: "nbg1", NetworkZone: hcloud.NetworkZoneEUCentral},
+				PublicNet: hcloud.LoadBalancerPublicNet{
+					Enabled: true,
+					IPv4:    hcloud.LoadBalancerPublicNetIPv4{IP: net.ParseIP("1.2.3.4")},
+					IPv6:    hcloud.LoadBalancerPublicNetIPv6{IP: net.ParseIP("fe80::1")},
+				},
+				PrivateNet: []hcloud.LoadBalancerPrivateNet{
+					{
+						Network: &hcloud.Network{
+							ID:   4711,
+							Name: "priv-net",
+						},
+						IP: net.ParseIP("10.10.10.2"),
+					},
+				},
+			},
+			Mock: func(_ *testing.T, tt *LoadBalancerTestCase) {
+				tt.LBOps.
+					On("GetByK8SServiceUID", tt.Ctx, tt.Service).
+					Return(tt.LB, nil)
+			},
+			Perform: func(t *testing.T, tt *LoadBalancerTestCase) {
+				status, exists, err := tt.LoadBalancers.GetLoadBalancer(tt.Ctx, tt.ClusterName, tt.Service)
+				assert.NoError(t, err)
+				assert.True(t, exists)
+
+				if !assert.Len(t, status.Ingress, 3) {
+					return
+				}
+				assert.Equal(t, tt.LB.PublicNet.IPv4.IP.String(), status.Ingress[0].IP)
+				assert.Equal(t, tt.LB.PublicNet.IPv6.IP.String(), status.Ingress[1].IP)
+				assert.Equal(t, tt.LB.PrivateNet[0].IP.String(), status.Ingress[2].IP)
+			},
+		},
+		{
+			Name:                         "get load balancer with private network and without private ingress",
+			ServiceUID:                   "1",
+			DisablePrivateIngressDefault: true,
+			LB: &hcloud.LoadBalancer{
+				ID:               1,
+				Name:             "with-priv-net-without-private-ingress",
+				LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"},
+				Location:         &hcloud.Location{Name: "nbg1", NetworkZone: hcloud.NetworkZoneEUCentral},
+				PublicNet: hcloud.LoadBalancerPublicNet{
+					Enabled: true,
+					IPv4:    hcloud.LoadBalancerPublicNetIPv4{IP: net.ParseIP("1.2.3.4")},
+					IPv6:    hcloud.LoadBalancerPublicNetIPv6{IP: net.ParseIP("fe80::1")},
+				},
+				PrivateNet: []hcloud.LoadBalancerPrivateNet{
+					{
+						Network: &hcloud.Network{
+							ID:   4711,
+							Name: "priv-net",
+						},
+						IP: net.ParseIP("10.10.10.2"),
+					},
+				},
+			},
+			Mock: func(_ *testing.T, tt *LoadBalancerTestCase) {
+				tt.LBOps.
+					On("GetByK8SServiceUID", tt.Ctx, tt.Service).
+					Return(tt.LB, nil)
+			},
+			Perform: func(t *testing.T, tt *LoadBalancerTestCase) {
+				status, exists, err := tt.LoadBalancers.GetLoadBalancer(tt.Ctx, tt.ClusterName, tt.Service)
+				assert.NoError(t, err)
+				assert.True(t, exists)
+
+				if !assert.Len(t, status.Ingress, 2) {
+					return
+				}
+				assert.Equal(t, tt.LB.PublicNet.IPv4.IP.String(), status.Ingress[0].IP)
+				assert.Equal(t, tt.LB.PublicNet.IPv6.IP.String(), status.Ingress[1].IP)
+			},
+		},
 		{
 			Name:       "load balancer not found",
 			ServiceUID: "3",