Skip to content

Commit 1db79b4

Browse files
lukasmetznerlukasmetzner
authored
refactor: inverse developer facing negations (#989)
Features like `load-balancer.hetzner.cloud/disable-private-ingress` have a user facing negation. Up until now, these features were also negated in the codebase and inverted in conditional checks (e.g., `!disablePrivateIngress`). This makes the code harder to understand. This PR inverses this behavior in the codebase.
1 parent 25021a2 commit 1db79b4

8 files changed

Lines changed: 176 additions & 103 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.AttachedCheckEnabled {
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.PrivateIngressEnabled, c.cfg.LoadBalancer.IPv6Enabled), true
185185
}
186186

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

‎hcloud/load_balancers.go‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,16 @@ type LoadBalancerOps interface {
3131

3232
type loadBalancers struct {
3333
lbOps LoadBalancerOps
34-
disablePrivateIngressDefault bool
35-
disableIPv6Default bool
36-
useProxyProtocolDefault bool
34+
ipv6EnabledDefault bool
35+
proxyProtocolEnabledDefault bool
36+
privateIngressEnabledDefault bool
3737
}
3838

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

@@ -219,12 +219,12 @@ func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer,
219219
var ingress []corev1.LoadBalancerIngress
220220
ipMode := corev1.LoadBalancerIPModeVIP
221221

222-
useProxyProtocol, err := l.getUseProxyProtocol(svc)
222+
proxyProtocolEnabled, err := l.getProxyProtocolEnabled(svc)
223223
if err != nil {
224224
return nil, fmt.Errorf("%s: %w", op, err)
225225
}
226226

227-
if useProxyProtocol {
227+
if proxyProtocolEnabled {
228228
ipMode = corev1.LoadBalancerIPModeProxy
229229
}
230230

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

237-
disableIPV6, err := l.getDisableIPv6(svc)
237+
ipv6Enabled, err := l.getIPv6Enabled(svc)
238238
if err != nil {
239239
return nil, fmt.Errorf("%s: %w", op, err)
240240
}
241-
if !disableIPV6 {
241+
if ipv6Enabled {
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+
privateIngressEnabled, err := l.getPrivateIngressEnabled(svc)
250250
if err != nil {
251251
return nil, fmt.Errorf("%s: %w", op, err)
252252
}
253253

254-
if !disablePrivIngress {
254+
if privateIngressEnabled {
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) getPrivateIngressEnabled(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.privateIngressEnabledDefault, nil
273273
}
274-
return false, err
274+
return true, err
275275
}
276276

277-
func (l *loadBalancers) getUseProxyProtocol(svc *corev1.Service) (bool, error) {
278-
disable, err := annotation.LBSvcProxyProtocol.BoolFromService(svc)
277+
func (l *loadBalancers) getProxyProtocolEnabled(svc *corev1.Service) (bool, error) {
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) {
283-
return l.useProxyProtocolDefault, nil
283+
return l.proxyProtocolEnabledDefault, nil
284284
}
285285
return false, err
286286
}
287287

288-
func (l *loadBalancers) getDisableIPv6(svc *corev1.Service) (bool, error) {
288+
func (l *loadBalancers) getIPv6Enabled(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.ipv6EnabledDefault, 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: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,14 @@ type LoadBalancerConfiguration struct {
7979
Enabled bool
8080
Location string
8181
NetworkZone string
82-
DisablePrivateIngress bool
83-
UsePrivateIP bool
84-
DisableIPv6 bool
82+
PrivateIngressEnabled bool
83+
PrivateIPEnabled bool
84+
IPv6Enabled bool
8585
}
8686

8787
type NetworkConfiguration struct {
8888
NameOrID string
89-
DisableAttachedCheck bool
89+
AttachedCheckEnabled 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+
disablePrivateIngress, err := getEnvBool(hcloudLoadBalancersDisablePrivateIngress, false)
175176
if err != nil {
176177
errs = append(errs, err)
177178
}
178-
cfg.LoadBalancer.UsePrivateIP, err = getEnvBool(hcloudLoadBalancersUsePrivateIP, false)
179+
cfg.LoadBalancer.PrivateIngressEnabled = !disablePrivateIngress // Invert the logic, as the env var is prefixed with DISABLE_.
180+
181+
cfg.LoadBalancer.PrivateIPEnabled, err = getEnvBool(hcloudLoadBalancersUsePrivateIP, false)
179182
if err != nil {
180183
errs = append(errs, err)
181184
}
182-
cfg.LoadBalancer.DisableIPv6, err = getEnvBool(hcloudLoadBalancersDisableIPv6, false)
185+
186+
disableIPv6, err := getEnvBool(hcloudLoadBalancersDisableIPv6, false)
183187
if err != nil {
184188
errs = append(errs, err)
185189
}
190+
cfg.LoadBalancer.IPv6Enabled = !disableIPv6 // 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+
disableAttachedCheck, err := getEnvBool(hcloudNetworkDisableAttachedCheck, false)
189194
if err != nil {
190195
errs = append(errs, err)
191196
}
197+
cfg.Network.AttachedCheckEnabled = !disableAttachedCheck // 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)