Repository: hetznercloud/hcloud-cloud-controller-manager
Pull request #549 (2023-10-26 08:47:48)

refactor: replace LoadBalancerDefaults with cfg
The struct `LoadBalancerDefaults` was a subset of `config.LoadBalancerConfiguration` and unecessary.

Also remove a duplicate test.


Patch:
diff --git a/hcloud/cloud.go b/hcloud/cloud.go
index 42fd3ad4..200ac333 100644
--- a/hcloud/cloud.go
+++ b/hcloud/cloud.go
@@ -144,11 +144,7 @@ func (c *cloud) LoadBalancer() (cloudprovider.LoadBalancer, bool) {
 		ActionClient:  &c.client.Action,
 		NetworkClient: &c.client.Network,
 		NetworkID:     c.networkID,
-		Defaults: hcops.LoadBalancerDefaults{
-			Location:     c.cfg.LoadBalancer.Location,
-			NetworkZone:  c.cfg.LoadBalancer.NetworkZone,
-			UsePrivateIP: c.cfg.LoadBalancer.UsePrivateIP,
-		},
+		Cfg:           c.cfg.LoadBalancer,
 	}
 
 	return newLoadBalancers(lbOps, c.cfg.LoadBalancer.DisablePrivateIngress, c.cfg.LoadBalancer.DisableIPv6), true
diff --git a/internal/hcops/load_balancer.go b/internal/hcops/load_balancer.go
index b8f9001d..2a6aa33f 100644
--- a/internal/hcops/load_balancer.go
+++ b/internal/hcops/load_balancer.go
@@ -11,6 +11,7 @@ import (
 	"k8s.io/klog/v2"
 
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation"
+	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config"
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics"
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/providerid"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
@@ -70,14 +71,7 @@ type LoadBalancerOps struct {
 	CertOps       *CertificateOps
 	RetryDelay    time.Duration
 	NetworkID     int64
-	Defaults      LoadBalancerDefaults
-}
-
-// LoadBalancerDefaults stores cluster-wide default values for load balancers.
-type LoadBalancerDefaults struct {
-	Location     string
-	NetworkZone  string
-	UsePrivateIP bool
+	Cfg           config.LoadBalancerConfiguration
 }
 
 // GetByK8SServiceUID tries to find a Load Balancer by its Kubernetes service
@@ -170,8 +164,8 @@ func (l *LoadBalancerOps) Create(
 	if v, ok := annotation.LBType.StringFromService(svc); ok {
 		opts.LoadBalancerType.Name = v
 	}
-	if l.Defaults.Location != "" {
-		opts.Location = &hcloud.Location{Name: l.Defaults.Location}
+	if l.Cfg.Location != "" {
+		opts.Location = &hcloud.Location{Name: l.Cfg.Location}
 	}
 	if v, ok := annotation.LBLocation.StringFromService(svc); ok {
 		if v == "" {
@@ -182,7 +176,7 @@ func (l *LoadBalancerOps) Create(
 			opts.Location = &hcloud.Location{Name: v}
 		}
 	}
-	opts.NetworkZone = hcloud.NetworkZone(l.Defaults.NetworkZone)
+	opts.NetworkZone = hcloud.NetworkZone(l.Cfg.NetworkZone)
 	if v, ok := annotation.LBNetworkZone.StringFromService(svc); ok {
 		opts.NetworkZone = hcloud.NetworkZone(v)
 	}
@@ -664,7 +658,7 @@ func (l *LoadBalancerOps) getUsePrivateIP(svc *corev1.Service) (bool, error) {
 	usePrivateIP, err := annotation.LBUsePrivateIP.BoolFromService(svc)
 	if err != nil {
 		if errors.Is(err, annotation.ErrNotSet) {
-			return l.Defaults.UsePrivateIP, nil
+			return l.Cfg.UsePrivateIP, nil
 		}
 		return false, err
 	}


Test patch:
diff --git a/hcloud/cloud_test.go b/hcloud/cloud_test.go
index e9d6c24d..5dc7e1fe 100644
--- a/hcloud/cloud_test.go
+++ b/hcloud/cloud_test.go
@@ -80,23 +80,7 @@ func TestNewCloud(t *testing.T) {
 	})
 	var config bytes.Buffer
 	_, err := newCloud(&config)
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-}
-
-func TestNewCloudWrongTokenSize(t *testing.T) {
-	resetEnv := testsupport.Setenv(t,
-		"HCLOUD_TOKEN", "0123456789abcdef",
-		"HCLOUD_METRICS_ENABLED", "false",
-	)
-	defer resetEnv()
-
-	var config bytes.Buffer
-	_, err := newCloud(&config)
-	if err == nil || err.Error() != "entered token is invalid (must be exactly 64 characters long)" {
-		t.Fatalf("Unexpected error: %v", err)
-	}
+	assert.NoError(t, err)
 }
 
 func TestNewCloudConnectionNotPossible(t *testing.T) {
diff --git a/internal/hcops/load_balancer_test.go b/internal/hcops/load_balancer_test.go
index e220be6c..f8bc5a2a 100644
--- a/internal/hcops/load_balancer_test.go
+++ b/internal/hcops/load_balancer_test.go
@@ -15,6 +15,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation"
+	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config"
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 )
@@ -243,7 +244,7 @@ func TestGetByK8SServiceUID(t *testing.T) {
 func TestLoadBalancerOps_Create(t *testing.T) {
 	type testCase struct {
 		name               string
-		defaults           hcops.LoadBalancerDefaults
+		cfg                config.LoadBalancerConfiguration
 		serviceAnnotations map[annotation.Name]interface{}
 		createOpts         hcloud.LoadBalancerCreateOpts
 		mock               func(t *testing.T, tt *testCase, fx *hcops.LoadBalancerOpsFixture)
@@ -253,7 +254,7 @@ func TestLoadBalancerOps_Create(t *testing.T) {
 	tests := []testCase{
 		{
 			name: "create with with location name (and default set)",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				Location: "hel1",
 			},
 			serviceAnnotations: map[annotation.Name]interface{}{
@@ -273,7 +274,7 @@ func TestLoadBalancerOps_Create(t *testing.T) {
 		},
 		{
 			name: "create with network zone name only (and default set)",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				NetworkZone: "eu-central",
 			},
 			serviceAnnotations: map[annotation.Name]interface{}{
@@ -291,7 +292,7 @@ func TestLoadBalancerOps_Create(t *testing.T) {
 		},
 		{
 			name: "create with location as default only",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				Location: "fsn1",
 			},
 			createOpts: hcloud.LoadBalancerCreateOpts{
@@ -308,7 +309,7 @@ func TestLoadBalancerOps_Create(t *testing.T) {
 		},
 		{
 			name: "create with network zone as default only",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				NetworkZone: "eu-central",
 			},
 			createOpts: hcloud.LoadBalancerCreateOpts{
@@ -323,7 +324,7 @@ func TestLoadBalancerOps_Create(t *testing.T) {
 		},
 		{
 			name: "create with network zone and reset default location",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				Location: "hel1",
 			},
 			serviceAnnotations: map[annotation.Name]interface{}{
@@ -342,7 +343,7 @@ func TestLoadBalancerOps_Create(t *testing.T) {
 		},
 		{
 			name: "create with location and reset default network zone",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				NetworkZone: "eu-central",
 			},
 			serviceAnnotations: map[annotation.Name]interface{}{
@@ -511,7 +512,7 @@ func TestLoadBalancerOps_Create(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			fx := hcops.NewLoadBalancerOpsFixture(t)
 
-			fx.LBOps.Defaults = tt.defaults
+			fx.LBOps.Cfg = tt.cfg
 
 			if tt.mock == nil {
 				tt.mock = func(t *testing.T, tt *testCase, fx *hcops.LoadBalancerOpsFixture) {
@@ -589,7 +590,7 @@ func TestLoadBalancerOps_Delete(t *testing.T) {
 
 type LBReconcilementTestCase struct {
 	name               string
-	defaults           hcops.LoadBalancerDefaults
+	cfg                config.LoadBalancerConfiguration
 	serviceUID         string
 	serviceAnnotations map[annotation.Name]interface{}
 	servicePorts       []corev1.ServicePort
@@ -607,7 +608,7 @@ func (tt *LBReconcilementTestCase) run(t *testing.T) {
 	t.Helper()
 
 	tt.fx = hcops.NewLoadBalancerOpsFixture(t)
-	tt.fx.LBOps.Defaults = tt.defaults
+	tt.fx.LBOps.Cfg = tt.cfg
 
 	if tt.service == nil {
 		tt.service = &corev1.Service{
@@ -1188,7 +1189,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) {
 		},
 		{
 			name: "enable use of private network via default",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				// Make sure the annotation overrides the default
 				UsePrivateIP: true,
 			},
@@ -1218,7 +1219,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) {
 		},
 		{
 			name: "enable use of private network via annotation",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				// Make sure the annotation overrides the default
 				UsePrivateIP: false,
 			},
@@ -1251,7 +1252,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) {
 		},
 		{
 			name: "disable use of private network via annotation",
-			defaults: hcops.LoadBalancerDefaults{
+			cfg: config.LoadBalancerConfiguration{
 				// Make sure the annotation overrides the default
 				UsePrivateIP: true,
 			},