Skip to content

Commit aed8a4e

Browse files
lukasmetznerlukasmetzner
committed
refactor: inverse developer facing negation
Features like `load-balancer.hetzner.cloud/disable-private-ingress` have a user facing negation. Up until now, these features where also negated in the codebase and inverted in conditional checks (e.g., `!disablePrivateIngress`). This PR inverses this behaviour in the codebase.
1 parent 82cbba0 commit aed8a4e

7 files changed

Lines changed: 171 additions & 98 deletions

File tree

‎hcloud/cloud.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func NewCloud(cidr string) (cloudprovider.Interface, error) {
108108
}
109109
networkID = n.ID
110110

111-
if !cfg.Network.DisableAttachedCheck {
111+
if cfg.Network.UseAttachedCheck {
112112
attached, err := serverIsAttachedToNetwork(metadataClient, networkID)
113113
if err != nil {
114114
return nil, fmt.Errorf("%s: checking if server is in Network not possible: %w", op, err)
@@ -181,7 +181,7 @@ func (c *cloud) LoadBalancer() (cloudprovider.LoadBalancer, bool) {
181181
Recorder: c.recorder,
182182
}
183183

184-
return newLoadBalancers(lbOps, c.cfg.LoadBalancer.DisablePrivateIngress, c.cfg.LoadBalancer.DisableIPv6), true
184+
return newLoadBalancers(lbOps, c.cfg.LoadBalancer.UsePrivateIngress, c.cfg.LoadBalancer.UseIPv6), true
185185
}
186186

187187
func (c *cloud) Clusters() (cloudprovider.Clusters, bool) {

‎hcloud/load_balancers.go‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,17 @@ type LoadBalancerOps interface {
3030
}
3131

3232
type loadBalancers struct {
33-
lbOps LoadBalancerOps
34-
disablePrivateIngressDefault bool
35-
disableIPv6Default bool
36-
useProxyProtocolDefault bool
33+
lbOps LoadBalancerOps
34+
usePrivateIngressDefault bool
35+
useIPv6Default bool
36+
useProxyProtocolDefault bool
3737
}
3838

39-
func newLoadBalancers(lbOps LoadBalancerOps, disablePrivateIngressDefault bool, disableIPv6Default bool) *loadBalancers {
39+
func newLoadBalancers(lbOps LoadBalancerOps, usePrivateIngressDefault bool, useIPv6Default bool) *loadBalancers {
4040
return &loadBalancers{
41-
lbOps: lbOps,
42-
disablePrivateIngressDefault: disablePrivateIngressDefault,
43-
disableIPv6Default: disableIPv6Default,
41+
lbOps: lbOps,
42+
usePrivateIngressDefault: usePrivateIngressDefault,
43+
useIPv6Default: useIPv6Default,
4444
}
4545
}
4646

@@ -234,24 +234,24 @@ func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer,
234234
IPMode: &ipMode,
235235
})
236236

237-
disableIPV6, err := l.getDisableIPv6(svc)
237+
useIPv6, err := l.getUseIPv6(svc)
238238
if err != nil {
239239
return nil, fmt.Errorf("%s: %w", op, err)
240240
}
241-
if !disableIPV6 {
241+
if useIPv6 {
242242
ingress = append(ingress, corev1.LoadBalancerIngress{
243243
IP: lb.PublicNet.IPv6.IP.String(),
244244
IPMode: &ipMode,
245245
})
246246
}
247247
}
248248

249-
disablePrivIngress, err := l.getDisablePrivateIngress(svc)
249+
usePrivIngress, err := l.getUsePrivateIngress(svc)
250250
if err != nil {
251251
return nil, fmt.Errorf("%s: %w", op, err)
252252
}
253253

254-
if !disablePrivIngress {
254+
if usePrivIngress {
255255
for _, privateNet := range lb.PrivateNet {
256256
ingress = append(ingress, corev1.LoadBalancerIngress{
257257
IP: privateNet.IP.String(),
@@ -263,37 +263,37 @@ func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer,
263263
return ingress, nil
264264
}
265265

266-
func (l *loadBalancers) getDisablePrivateIngress(svc *corev1.Service) (bool, error) {
266+
func (l *loadBalancers) getUsePrivateIngress(svc *corev1.Service) (bool, error) {
267267
disable, err := annotation.LBDisablePrivateIngress.BoolFromService(svc)
268268
if err == nil {
269-
return disable, nil
269+
return !disable, nil
270270
}
271271
if errors.Is(err, annotation.ErrNotSet) {
272-
return l.disablePrivateIngressDefault, nil
272+
return l.usePrivateIngressDefault, nil
273273
}
274-
return false, err
274+
return true, err
275275
}
276276

277277
func (l *loadBalancers) getUseProxyProtocol(svc *corev1.Service) (bool, error) {
278-
disable, err := annotation.LBSvcProxyProtocol.BoolFromService(svc)
278+
enable, err := annotation.LBSvcProxyProtocol.BoolFromService(svc)
279279
if err == nil {
280-
return disable, nil
280+
return enable, nil
281281
}
282282
if errors.Is(err, annotation.ErrNotSet) {
283283
return l.useProxyProtocolDefault, nil
284284
}
285285
return false, err
286286
}
287287

288-
func (l *loadBalancers) getDisableIPv6(svc *corev1.Service) (bool, error) {
288+
func (l *loadBalancers) getUseIPv6(svc *corev1.Service) (bool, error) {
289289
disable, err := annotation.LBIPv6Disabled.BoolFromService(svc)
290290
if err == nil {
291-
return disable, nil
291+
return !disable, nil
292292
}
293293
if errors.Is(err, annotation.ErrNotSet) {
294-
return l.disableIPv6Default, nil
294+
return l.useIPv6Default, nil
295295
}
296-
return false, err
296+
return true, err
297297
}
298298

299299
func (l *loadBalancers) UpdateLoadBalancer(

‎hcloud/load_balancers_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,9 @@ func TestLoadBalancers_GetLoadBalancer(t *testing.T) {
150150
},
151151
},
152152
{
153-
Name: "get load balancer with private network and without private ingress",
154-
ServiceUID: "1",
155-
DisablePrivateIngressDefault: true,
153+
Name: "get load balancer with private network and without private ingress",
154+
ServiceUID: "1",
155+
UsePrivateIngressDefault: hcloud.Ptr(false),
156156
LB: &hcloud.LoadBalancer{
157157
ID: 1,
158158
Name: "with-priv-net-without-private-ingress",
@@ -374,7 +374,7 @@ func TestLoadBalancers_EnsureLoadBalancer_CreateLoadBalancer(t *testing.T) {
374374
ServiceAnnotations: map[annotation.Name]string{
375375
annotation.LBName: "with-priv-net-no-priv-ingress",
376376
},
377-
DisablePrivateIngressDefault: true,
377+
UsePrivateIngressDefault: hcloud.Ptr(false),
378378
LB: &hcloud.LoadBalancer{
379379
ID: 1,
380380
Name: "with-priv-net-no-priv-ingress",

‎hcloud/testing.go‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@ type LoadBalancerTestCase struct {
1818
Name string
1919

2020
// Defined in test case as needed
21-
ClusterName string
22-
NetworkID int
23-
ServiceUID string
24-
ServiceAnnotations map[annotation.Name]string
25-
DisablePrivateIngressDefault bool
26-
DisableIPv6Default bool
27-
Nodes []*corev1.Node
28-
LB *hcloud.LoadBalancer
29-
LBCreateResult *hcloud.LoadBalancerCreateResult
30-
Mock func(t *testing.T, tt *LoadBalancerTestCase)
21+
ClusterName string
22+
NetworkID int
23+
ServiceUID string
24+
ServiceAnnotations map[annotation.Name]string
25+
UsePrivateIngressDefault *bool
26+
UseIPv6Default *bool
27+
Nodes []*corev1.Node
28+
LB *hcloud.LoadBalancer
29+
LBCreateResult *hcloud.LoadBalancerCreateResult
30+
Mock func(t *testing.T, tt *LoadBalancerTestCase)
3131

3232
// Defines the actual test
3333
Perform func(t *testing.T, tt *LoadBalancerTestCase)
@@ -44,6 +44,14 @@ type LoadBalancerTestCase struct {
4444
func (tt *LoadBalancerTestCase) run(t *testing.T) {
4545
t.Helper()
4646

47+
if tt.UsePrivateIngressDefault == nil {
48+
tt.UsePrivateIngressDefault = hcloud.Ptr(true)
49+
}
50+
51+
if tt.UseIPv6Default == nil {
52+
tt.UseIPv6Default = hcloud.Ptr(true)
53+
}
54+
4755
tt.LBOps = &hcops.MockLoadBalancerOps{}
4856
tt.LBOps.Test(t)
4957

@@ -70,7 +78,7 @@ func (tt *LoadBalancerTestCase) run(t *testing.T) {
7078
tt.Mock(t, tt)
7179
}
7280

73-
tt.LoadBalancers = newLoadBalancers(tt.LBOps, tt.DisablePrivateIngressDefault, tt.DisableIPv6Default)
81+
tt.LoadBalancers = newLoadBalancers(tt.LBOps, *tt.UsePrivateIngressDefault, *tt.UseIPv6Default)
7482
tt.Perform(t, tt)
7583

7684
tt.LBOps.AssertExpectations(t)

‎internal/config/config.go‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,17 @@ type InstanceConfiguration struct {
7676
}
7777

7878
type LoadBalancerConfiguration struct {
79-
Enabled bool
80-
Location string
81-
NetworkZone string
82-
DisablePrivateIngress bool
83-
UsePrivateIP bool
84-
DisableIPv6 bool
79+
Enabled bool
80+
Location string
81+
NetworkZone string
82+
UsePrivateIngress bool
83+
UsePrivateIP bool
84+
UseIPv6 bool
8585
}
8686

8787
type NetworkConfiguration struct {
88-
NameOrID string
89-
DisableAttachedCheck bool
88+
NameOrID string
89+
UseAttachedCheck bool
9090
}
9191

9292
type RouteConfiguration struct {
@@ -171,24 +171,30 @@ func Read() (HCCMConfiguration, error) {
171171
}
172172
cfg.LoadBalancer.Location = os.Getenv(hcloudLoadBalancersLocation)
173173
cfg.LoadBalancer.NetworkZone = os.Getenv(hcloudLoadBalancersNetworkZone)
174-
cfg.LoadBalancer.DisablePrivateIngress, err = getEnvBool(hcloudLoadBalancersDisablePrivateIngress, false)
174+
175+
cfg.LoadBalancer.UsePrivateIngress, err = getEnvBool(hcloudLoadBalancersDisablePrivateIngress, false)
175176
if err != nil {
176177
errs = append(errs, err)
177178
}
179+
cfg.LoadBalancer.UsePrivateIngress = !cfg.LoadBalancer.UsePrivateIngress // Invert the logic, as the env var is prefixed with DISABLE_.
180+
178181
cfg.LoadBalancer.UsePrivateIP, err = getEnvBool(hcloudLoadBalancersUsePrivateIP, false)
179182
if err != nil {
180183
errs = append(errs, err)
181184
}
182-
cfg.LoadBalancer.DisableIPv6, err = getEnvBool(hcloudLoadBalancersDisableIPv6, false)
185+
186+
cfg.LoadBalancer.UseIPv6, err = getEnvBool(hcloudLoadBalancersDisableIPv6, false)
183187
if err != nil {
184188
errs = append(errs, err)
185189
}
190+
cfg.LoadBalancer.UseIPv6 = !cfg.LoadBalancer.UseIPv6 // Invert the logic, as the env var is prefixed with DISABLE_.
186191

187192
cfg.Network.NameOrID = os.Getenv(hcloudNetwork)
188-
cfg.Network.DisableAttachedCheck, err = getEnvBool(hcloudNetworkDisableAttachedCheck, false)
193+
cfg.Network.UseAttachedCheck, err = getEnvBool(hcloudNetworkDisableAttachedCheck, false)
189194
if err != nil {
190195
errs = append(errs, err)
191196
}
197+
cfg.Network.UseAttachedCheck = !cfg.Network.UseAttachedCheck // Invert the logic, as the env var is prefixed with DISABLE_.
192198

193199
// Enabling Routes only makes sense when a Network is configured, otherwise there is no network to add the routes to.
194200
if cfg.Network.NameOrID != "" {

0 commit comments

Comments
 (0)