Skip to content

Commit 60e75f6

Browse files
Mo3m3nMo3m3n
committed
OPTIM/MAJOR: Convert annotations type to simple string
Annotations are a special type defined as: type StringW struct { Value string OldValue string Status Status } "OldValue" is not used in controller logic "Status" introduces additional code complexity by making the developer deal with configuration in terms of separate and individual annotations that requires separate actions for each status (EMPTY, DELETED, ADDED, etc). This approach requires extra effort spent in reconciling different annotations and their status. The alternative approach is to handle annotations as simple string values and try to stick to the following pattern: 1. Generate a configuration object/model/state from current annotations 2. Retrieve current configuration object/model/state 3. 3. 3. Compare both and apply the new config if needbe. Also this approach is much more easy to reconcile with the upcoming work of handling Custom Resources.
1 parent 3cd030b commit 60e75f6

23 files changed

+262
-377
lines changed

‎controller/annotations/backend.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@ import (
77
"github.com/haproxytech/kubernetes-ingress/controller/store"
88
)
99

10-
func HandleBackendAnnotations(backend *models.Backend, k8sStore store.K8s, client api.HAProxyClient, annotations ...store.MapStringW) {
10+
func HandleBackendAnnotations(backend *models.Backend, k8sStore store.K8s, client api.HAProxyClient, annotations ...map[string]string) {
1111
for _, a := range GetBackendAnnotations(client, backend) {
12-
annValue, _ := k8sStore.GetValueFromAnnotations(a.GetName(), annotations...)
13-
if annValue == nil {
12+
annValue := k8sStore.GetValueFromAnnotations(a.GetName(), annotations...)
13+
if annValue == "" {
1414
continue
1515
}
16-
HandleAnnotation(a, annValue.Value)
16+
HandleAnnotation(a, annValue)
1717
}
1818
}
1919

‎controller/annotations/global.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import (
77
"github.com/haproxytech/kubernetes-ingress/controller/store"
88
)
99

10-
func HandleGlobalAnnotations(global *models.Global, defaults *models.Defaults, k8sStore store.K8s, client api.HAProxyClient, annotations store.MapStringW) {
10+
func HandleGlobalAnnotations(global *models.Global, defaults *models.Defaults, k8sStore store.K8s, client api.HAProxyClient, annotations map[string]string) {
1111
annList := GetGlobalAnnotations(client, global, defaults)
1212
for _, a := range annList {
13-
annValue, _ := k8sStore.GetValueFromAnnotations(a.GetName(), annotations)
14-
if annValue == nil {
13+
annValue := k8sStore.GetValueFromAnnotations(a.GetName(), annotations)
14+
if annValue == "" {
1515
continue
1616
}
17-
HandleAnnotation(a, annValue.Value)
17+
HandleAnnotation(a, annValue)
1818
}
1919
}
2020

‎controller/annotations/server.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ import (
99
)
1010

1111
// HandleServerAnnotations returns a pointer to a server model holding server configuration from annotations
12-
func HandleServerAnnotations(server *models.Server, k8sStore store.K8s, client api.HAProxyClient, haproxyCerts *haproxy.Certificates, annotations ...store.MapStringW) {
12+
func HandleServerAnnotations(server *models.Server, k8sStore store.K8s, client api.HAProxyClient, haproxyCerts *haproxy.Certificates, annotations ...map[string]string) {
1313
for _, a := range GetServerAnnotations(server, k8sStore, haproxyCerts) {
14-
annValue, _ := k8sStore.GetValueFromAnnotations(a.GetName(), annotations...)
15-
if annValue == nil {
14+
annValue := k8sStore.GetValueFromAnnotations(a.GetName(), annotations...)
15+
if annValue == "" {
1616
continue
1717
}
18-
HandleAnnotation(a, annValue.Value)
18+
HandleAnnotation(a, annValue)
1919
}
2020
}
2121

‎controller/controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func (c *HAProxyController) updateHAProxy() {
175175
}
176176
_, err = c.Cfg.Certificates.HandleTLSSecret(c.Store, haproxy.SecretCtx{
177177
DefaultNS: ingress.Namespace,
178-
SecretPath: tls.SecretName.Value,
178+
SecretPath: tls.SecretName,
179179
SecretType: haproxy.FT_CERT,
180180
})
181181
logger.Error(err)

‎controller/frontend-annotations.go

Lines changed: 80 additions & 80 deletions
Large diffs are not rendered by default.

‎controller/global.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,11 @@ func (c *HAProxyController) handleGlobalConfig() (reload, restart bool) {
6565

6666
// handleDefaultService configures HAProy default backend provided via cli param "default-backend-service"
6767
func (c *HAProxyController) handleDefaultService() (reload bool) {
68-
dsvcData, _ := c.Store.GetValueFromAnnotations("default-backend-service")
69-
if dsvcData == nil {
68+
dsvcData := c.Store.GetValueFromAnnotations("default-backend-service")
69+
if dsvcData == "" {
7070
return
7171
}
72-
dsvc := strings.Split(dsvcData.Value, "/")
72+
dsvc := strings.Split(dsvcData, "/")
7373

7474
if len(dsvc) != 2 {
7575
logger.Errorf("default service '%s': invalid format", dsvcData)
@@ -91,7 +91,7 @@ func (c *HAProxyController) handleDefaultService() (reload bool) {
9191
ingress := &store.Ingress{
9292
Namespace: namespace.Name,
9393
Name: "DefaultService",
94-
Annotations: store.MapStringW{},
94+
Annotations: map[string]string{},
9595
DefaultBackend: &store.IngressPath{
9696
SvcName: service.Name,
9797
SvcPortInt: service.Ports[0].Port,
@@ -108,12 +108,12 @@ func (c *HAProxyController) handleDefaultService() (reload bool) {
108108

109109
// handleDefaultCert configures default/fallback HAProxy certificate to use for client HTTPS requests.
110110
func (c *HAProxyController) handleDefaultCert() {
111-
secretAnn, _ := c.Store.GetValueFromAnnotations("ssl-certificate", c.Store.ConfigMaps.Main.Annotations)
112-
if secretAnn == nil {
111+
secretAnn := c.Store.GetValueFromAnnotations("ssl-certificate", c.Store.ConfigMaps.Main.Annotations)
112+
if secretAnn == "" {
113113
return
114114
}
115115
_, err := c.Cfg.Certificates.HandleTLSSecret(c.Store, haproxy.SecretCtx{
116-
SecretPath: secretAnn.Value,
116+
SecretPath: secretAnn,
117117
SecretType: haproxy.FT_DEFAULT_CERT,
118118
})
119119
logger.Error(err)

‎controller/handler/errorfile.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (h ErrorFile) Update(k store.K8s, cfg *config.ControllerCfg, api api.HAProx
4040
for code, v := range k.ConfigMaps.Errorfiles.Annotations {
4141
_, ok := h.files.data[code]
4242
if ok {
43-
err = h.files.updateFile(code, v.Value)
43+
err = h.files.updateFile(code, v)
4444
if err != nil {
4545
logger.Errorf("failed updating errorfile for code '%s': %s", code, err)
4646
}
@@ -50,7 +50,7 @@ func (h ErrorFile) Update(k store.K8s, cfg *config.ControllerCfg, api api.HAProx
5050
if err != nil {
5151
logger.Errorf("failed creating errorfile for code '%s': %s", code, err)
5252
}
53-
err = h.files.newFile(code, v.Value)
53+
err = h.files.newFile(code, v)
5454
if err != nil {
5555
logger.Errorf("failed creating errorfile for code '%s': %s", code, err)
5656
}

‎controller/handler/https.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ func (h HTTPS) bindList(passhthrough bool) (binds []models.Bind) {
7272
}
7373

7474
func (h HTTPS) handleClientTLSAuth(k store.K8s, cfg *config.ControllerCfg, api api.HAProxyClient) (reload bool, err error) {
75-
annTLSAuth, _ := k.GetValueFromAnnotations("client-ca", k.ConfigMaps.Main.Annotations)
76-
annTLSVerify, _ := k.GetValueFromAnnotations("client-crt-optional", k.ConfigMaps.Main.Annotations)
77-
if annTLSAuth == nil {
78-
return
75+
annTLSAuth := k.GetValueFromAnnotations("client-ca", k.ConfigMaps.Main.Annotations)
76+
annTLSVerify := k.GetValueFromAnnotations("client-crt-optional", k.ConfigMaps.Main.Annotations)
77+
if annTLSAuth == "" {
78+
return false, nil
7979
}
8080
binds, err := api.FrontendBindsGet(cfg.FrontHTTPS)
8181
if err != nil {
@@ -85,17 +85,17 @@ func (h HTTPS) handleClientTLSAuth(k store.K8s, cfg *config.ControllerCfg, api a
8585
var caFile string
8686
caFile, err = cfg.Certificates.HandleTLSSecret(k, haproxy.SecretCtx{
8787
DefaultNS: "",
88-
SecretPath: annTLSAuth.Value,
88+
SecretPath: annTLSAuth,
8989
SecretType: haproxy.CA_CERT,
9090
})
9191
if err != nil {
9292
if errors.Is(err, haproxy.ErrCertNotFound) {
93-
logger.Warning("unable to configure TLS authentication secret '%s' not found", annTLSAuth.Value)
93+
logger.Warningf("unable to configure TLS authentication secret '%s' not found", annTLSAuth)
9494
err = nil
9595
}
9696
}
9797
verify := "required"
98-
enabled, annErr := utils.GetBoolValue("client-crt-optional", annTLSVerify.Value)
98+
enabled, annErr := utils.GetBoolValue(annTLSVerify, "client-crt-optional")
9999
logger.Error(annErr)
100100
if enabled {
101101
verify = "optional"
@@ -246,9 +246,9 @@ func (h HTTPS) toggleSSLPassthrough(passthrough bool, cfg *config.ControllerCfg,
246246

247247
func (h HTTPS) sslPassthroughRules(k store.K8s, cfg *config.ControllerCfg) error {
248248
inspectTimeout := utils.PtrInt64(5000)
249-
annTimeout, _ := k.GetValueFromAnnotations("timeout-client", k.ConfigMaps.Main.Annotations)
250-
if annTimeout != nil {
251-
if value, errParse := utils.ParseTime(annTimeout.Value); errParse == nil {
249+
annTimeout := k.GetValueFromAnnotations("timeout-client", k.ConfigMaps.Main.Annotations)
250+
if annTimeout != "" {
251+
if value, errParse := utils.ParseTime(annTimeout); errParse == nil {
252252
inspectTimeout = value
253253
} else {
254254
logger.Error(errParse)

‎controller/handler/pattern-files.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,12 @@ func (h PatternFiles) Update(k store.K8s, cfg *config.ControllerCfg, api api.HAP
4747
for name, v := range k.ConfigMaps.PatternFiles.Annotations {
4848
_, ok := h.files.data[name]
4949
if ok {
50-
err = h.files.updateFile(name, v.Value)
50+
err = h.files.updateFile(name, v)
5151
if err != nil {
5252
logger.Errorf("failed updating patternFile '%s': %s", name, err)
5353
}
5454
} else {
55-
err = h.files.newFile(name, v.Value)
55+
err = h.files.newFile(name, v)
5656
if err != nil {
5757
logger.Errorf("failed creating patternFile '%s': %s", name, err)
5858
}

‎controller/handler/proxy-protocol.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,14 @@ type ProxyProtocol struct{}
2929

3030
func (p ProxyProtocol) Update(k store.K8s, cfg *config.ControllerCfg, api api.HAProxyClient) (reload bool, err error) {
3131
// Get annotation status
32-
annProxyProtocol, _ := k.GetValueFromAnnotations("proxy-protocol", k.ConfigMaps.Main.Annotations)
33-
if annProxyProtocol == nil {
34-
return false, nil
35-
}
36-
if annProxyProtocol.Status == store.DELETED {
37-
logger.Trace("Deleting ProxyProtocol configuration")
32+
annProxyProtocol := k.GetValueFromAnnotations("proxy-protocol", k.ConfigMaps.Main.Annotations)
33+
if annProxyProtocol == "" {
3834
return false, nil
3935
}
4036
// Validate annotation
41-
mapName := "proxy-protocol-" + utils.Hash([]byte(annProxyProtocol.Value))
37+
mapName := "proxy-protocol-" + utils.Hash([]byte(annProxyProtocol))
4238
if !cfg.MapFiles.Exists(mapName) {
43-
for _, address := range strings.Split(annProxyProtocol.Value, ",") {
39+
for _, address := range strings.Split(annProxyProtocol, ",") {
4440
address = strings.TrimSpace(address)
4541
if ip := net.ParseIP(address); ip == nil {
4642
if _, _, err = net.ParseCIDR(address); err != nil {

‎controller/handler/refresh.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ type Refresh struct{}
2525

2626
func (h Refresh) Update(k store.K8s, cfg *config.ControllerCfg, api api.HAProxyClient) (reload bool, err error) {
2727
cleanCrts := true
28-
if cleanCrtAnn, _ := k.GetValueFromAnnotations("clean-certs", k.ConfigMaps.Main.Annotations); cleanCrtAnn != nil && cleanCrtAnn.Status != store.DELETED {
29-
cleanCrts, err = utils.GetBoolValue(cleanCrtAnn.Value, "clean-certs")
28+
if cleanCrtAnn := k.GetValueFromAnnotations("clean-certs", k.ConfigMaps.Main.Annotations); cleanCrtAnn != "" {
29+
cleanCrts, err = utils.GetBoolValue(cleanCrtAnn, "clean-certs")
3030
}
3131
if cleanCrts {
3232
reload = cfg.Certificates.Refresh() || reload

‎controller/handler/tcp-services.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func (t TCPServices) Update(k store.K8s, cfg *config.ControllerCfg, api api.HAPr
3535
var p tcpSvcParser
3636
for port, tcpSvcAnn := range k.ConfigMaps.TCPServices.Annotations {
3737
frontendName := fmt.Sprintf("tcp-%s", port)
38-
p, err = t.parseTCPService(k, tcpSvcAnn.Value)
38+
p, err = t.parseTCPService(k, tcpSvcAnn)
3939
if err != nil {
4040
logger.Error(err)
4141
continue
@@ -184,7 +184,7 @@ func (t TCPServices) updateTCPFrontend(api api.HAProxyClient, frontend models.Fr
184184
}
185185
ingress := &store.Ingress{
186186
Namespace: p.service.Namespace,
187-
Annotations: store.MapStringW{},
187+
Annotations: make(map[string]string),
188188
DefaultBackend: &store.IngressPath{
189189
SvcName: p.service.Name,
190190
SvcPortInt: p.port,

‎controller/ingress.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ import (
3232
func (c *HAProxyController) igClassIsSupported(ingress *store.Ingress) bool {
3333
var igClassAnn string
3434
var igClass *store.IngressClass
35-
if ann, _ := c.Store.GetValueFromAnnotations("ingress.class", ingress.Annotations); ann != nil {
36-
igClassAnn = ann.Value
37-
}
35+
igClassAnn = c.Store.GetValueFromAnnotations("ingress.class", ingress.Annotations)
3836

3937
// If ingress class is unassigned and the controller is controlling any resource without explicit ingress class then support it.
4038
if igClassAnn == "" && c.OSArgs.EmptyIngressClass {
@@ -81,16 +79,16 @@ func (c *HAProxyController) handleIngressPath(ingress *store.Ingress, host strin
8179
BackendName: backendName,
8280
SSLPassthrough: sslPassthrough,
8381
}
84-
routeACLAnn, _ := c.Store.GetValueFromAnnotations("route-acl", svc.GetService().Annotations)
85-
if routeACLAnn == nil {
82+
routeACLAnn := c.Store.GetValueFromAnnotations("route-acl", svc.GetService().Annotations)
83+
if routeACLAnn == "" {
8684
if _, ok := route.CustomRoutes[backendName]; ok {
8785
delete(route.CustomRoutes, backendName)
8886
logger.Debugf("Custom Route to backend '%s' deleted, reload required", backendName)
8987
routeReload = true
9088
}
9189
err = route.AddHostPathRoute(ingRoute, c.Cfg.MapFiles)
9290
} else {
93-
routeReload, err = route.AddCustomRoute(ingRoute, *routeACLAnn, c.Client)
91+
routeReload, err = route.AddCustomRoute(ingRoute, routeACLAnn, c.Client)
9492
}
9593
if err != nil {
9694
return
@@ -145,23 +143,23 @@ func (c *HAProxyController) setDefaultService(ingress *store.Ingress, frontends
145143
}
146144

147145
func (c *HAProxyController) sslPassthroughEnabled(ingress *store.Ingress, path *store.IngressPath) bool {
148-
var annSSLPassthrough *store.StringW
146+
var annSSLPassthrough string
149147
var service *store.Service
150148
ok := false
151149
if path != nil {
152150
service, ok = c.Store.Namespaces[ingress.Namespace].Services[path.SvcName]
153151
}
154152
if ok {
155-
annSSLPassthrough, _ = c.Store.GetValueFromAnnotations("ssl-passthrough", service.Annotations, ingress.Annotations, c.Store.ConfigMaps.Main.Annotations)
153+
annSSLPassthrough = c.Store.GetValueFromAnnotations("ssl-passthrough", service.Annotations, ingress.Annotations, c.Store.ConfigMaps.Main.Annotations)
156154
} else {
157-
annSSLPassthrough, _ = c.Store.GetValueFromAnnotations("ssl-passthrough", ingress.Annotations, c.Store.ConfigMaps.Main.Annotations)
155+
annSSLPassthrough = c.Store.GetValueFromAnnotations("ssl-passthrough", ingress.Annotations, c.Store.ConfigMaps.Main.Annotations)
158156
}
159-
enabled, err := utils.GetBoolValue(annSSLPassthrough.Value, "ssl-passthrough")
160-
if err != nil {
161-
logger.Errorf("ssl-passthrough annotation: %s", err)
157+
if annSSLPassthrough == "" {
162158
return false
163159
}
164-
if annSSLPassthrough.Status == DELETED {
160+
enabled, err := utils.GetBoolValue(annSSLPassthrough, "ssl-passthrough")
161+
if err != nil {
162+
logger.Errorf("ssl-passthrough annotation: %s", err)
165163
return false
166164
}
167165
if enabled {

‎controller/kubernetes.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ func (k *K8s) convertToEndpoints(obj interface{}, status store.Status) (*store.E
226226
}
227227
item := &store.Endpoints{
228228
Namespace: data.GetNamespace(),
229-
Service: store.StringW{Value: data.GetName()},
229+
Service: data.GetName(),
230230
Ports: make(map[string]*store.PortEndpoints),
231231
Status: status,
232232
}
@@ -363,7 +363,7 @@ func (k *K8s) EventsServices(channel chan SyncDataEvent, stop chan struct{}, inf
363363
item := &store.Service{
364364
Namespace: data.GetNamespace(),
365365
Name: data.GetName(),
366-
Annotations: store.ConvertToMapStringW(data.ObjectMeta.Annotations),
366+
Annotations: store.CopyAnnotations(data.ObjectMeta.Annotations),
367367
Ports: []store.ServicePort{},
368368
Status: status,
369369
}
@@ -398,7 +398,7 @@ func (k *K8s) EventsServices(channel chan SyncDataEvent, stop chan struct{}, inf
398398
item := &store.Service{
399399
Namespace: data.GetNamespace(),
400400
Name: data.GetName(),
401-
Annotations: store.ConvertToMapStringW(data.ObjectMeta.Annotations),
401+
Annotations: store.CopyAnnotations(data.ObjectMeta.Annotations),
402402
Status: status,
403403
}
404404
if data.Spec.Type == corev1.ServiceTypeExternalName {
@@ -435,7 +435,7 @@ func (k *K8s) EventsServices(channel chan SyncDataEvent, stop chan struct{}, inf
435435
item1 := &store.Service{
436436
Namespace: data1.GetNamespace(),
437437
Name: data1.GetName(),
438-
Annotations: store.ConvertToMapStringW(data1.ObjectMeta.Annotations),
438+
Annotations: store.CopyAnnotations(data1.ObjectMeta.Annotations),
439439
Ports: []store.ServicePort{},
440440
Status: status,
441441
}
@@ -453,7 +453,7 @@ func (k *K8s) EventsServices(channel chan SyncDataEvent, stop chan struct{}, inf
453453
item2 := &store.Service{
454454
Namespace: data2.GetNamespace(),
455455
Name: data2.GetName(),
456-
Annotations: store.ConvertToMapStringW(data2.ObjectMeta.Annotations),
456+
Annotations: store.CopyAnnotations(data2.ObjectMeta.Annotations),
457457
Ports: []store.ServicePort{},
458458
Status: status,
459459
}
@@ -499,7 +499,7 @@ func (k *K8s) EventsConfigfMaps(channel chan SyncDataEvent, stop chan struct{},
499499
item := &store.ConfigMap{
500500
Namespace: data.GetNamespace(),
501501
Name: data.GetName(),
502-
Annotations: store.ConvertToMapStringW(data.Data),
502+
Annotations: store.CopyAnnotations(data.Data),
503503
Status: status,
504504
}
505505
k.Logger.Tracef("%s %s: %s", CONFIGMAP, item.Status, item.Name)
@@ -515,7 +515,7 @@ func (k *K8s) EventsConfigfMaps(channel chan SyncDataEvent, stop chan struct{},
515515
item := &store.ConfigMap{
516516
Namespace: data.GetNamespace(),
517517
Name: data.GetName(),
518-
Annotations: store.ConvertToMapStringW(data.Data),
518+
Annotations: store.CopyAnnotations(data.Data),
519519
Status: status,
520520
}
521521
k.Logger.Tracef("%s %s: %s", CONFIGMAP, item.Status, item.Name)
@@ -536,13 +536,13 @@ func (k *K8s) EventsConfigfMaps(channel chan SyncDataEvent, stop chan struct{},
536536
item1 := &store.ConfigMap{
537537
Namespace: data1.GetNamespace(),
538538
Name: data1.GetName(),
539-
Annotations: store.ConvertToMapStringW(data1.Data),
539+
Annotations: store.CopyAnnotations(data1.Data),
540540
Status: status,
541541
}
542542
item2 := &store.ConfigMap{
543543
Namespace: data2.GetNamespace(),
544544
Name: data2.GetName(),
545-
Annotations: store.ConvertToMapStringW(data2.Data),
545+
Annotations: store.CopyAnnotations(data2.Data),
546546
Status: status,
547547
}
548548
if item2.Equal(item1) {

0 commit comments

Comments
 (0)