Repository: FoundationDB/fdb-kubernetes-operator (go)

Issue:
Panic when adding annotations to ConfigMap
### What happened?

When adding annotations to the cluster's config map via the [`configMap` property of `FoundationDBClusterSpec`](https://github.com/FoundationDB/fdb-kubernetes-operator/blob/main/docs/cluster_spec.md#foundationdbclusterspec), the cluster controller panics with the following:

```
panic: assignment to entry in nil map [recovered]
	panic: assignment to entry in nil map

goroutine 205 [running]:
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.7/pkg/internal/controller/controller.go:119 +0x1da
panic({0x16e5ee0?, 0x1b44360?})
	/usr/local/go/src/runtime/panic.go:785 +0x132
github.com/FoundationDB/fdb-kubernetes-operator/controllers.mergeMap(...)
	/workspace/controllers/controllers.go:75
github.com/FoundationDB/fdb-kubernetes-operator/controllers.mergeAnnotations(...)
	/workspace/controllers/controllers.go:65
github.com/FoundationDB/fdb-kubernetes-operator/controllers.updateConfigMap.reconcile({}, {0x1b60fd8, 0xc0008e8360}, 0xc000218380, 0xc000845508, 0x0?, {{0x1b64350?, 0xc000a8e000?}, 0xc000a8e000?})
	/workspace/controllers/update_config_map.go:68 +0x526
github.com/FoundationDB/fdb-kubernetes-operator/controllers.runClusterSubReconciler({0x1b60fd8, 0xc0008e8360}, {{0x1b64350?, 0xc0008e8630?}, 0xc000a82100?}, {0x1b47820, 0x27ebd20}, 0xc000218380, 0xc000845508, 0x0)
	/workspace/controllers/cluster_controller.go:260 +0x2e6
github.com/FoundationDB/fdb-kubernetes-operator/controllers.(*FoundationDBClusterReconciler).Reconcile(0xc000218380, {0x1b60fd8, 0xc0008e8360}, {{{0xc0002c89d0?, 0x0?}, {0xc0005ea0d8?, 0xc0008e8360?}}})
	/workspace/controllers/cluster_controller.go:214 +0xe93
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x1b60fd8?, {0x1b60fd8?, 0xc0008e8360?}, {{{0xc0002c89d0?, 0x1633b60?}, {0xc0005ea0d8?, 0x4172cb?}}})
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.7/pkg/internal/controller/controller.go:122 +0xa5
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc000349860, {0x1b61010, 0xc0004c0fa0}, {0x17441e0, 0xc0008acbe0})
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.7/pkg/internal/controller/controller.go:323 +0x325
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc000349860, {0x1b61010, 0xc0004c0fa0})
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.7/pkg/internal/controller/controller.go:274 +0x1a8
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.7/pkg/internal/controller/controller.go:235 +0x73
created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2 in goroutine 92
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.7/pkg/internal/controller/controller.go:231 +0x50c
```

### What did you expect to happen?

The controller should not crash, and the custom annotations should be added to the `ConfigMap` managed by the controller.

Note that this _does_ work with custom _labels._ The problem appears to be in `mergeMap`: https://github.com/FoundationDB/fdb-kubernetes-operator/blob/0651fe849a1e9bd86f61cc3d83bad568c67ec01c/controllers/controllers.go#L68-L80

The default set of annotations for the `ConfigMap` is empty (i.e. `nil`), but the default set of labels is non-empty. Attempting to add a label adds a new value to an existing map, but adding a new annotation tries to add to a `nil` map and panics.

### How can we reproduce it (as minimally and precisely as possible)?

Add a `configMap` property to a cluster definition that adds a new annotation:

```yaml
configMap:
  metadata:
    annotations:
      panics: "unfortunately yes"
```

### Anything else we need to know?

_No response_

### FDB Kubernetes operator

Operator version 1.55.0 (but it looks like it would still be an issue in 2.0.0).

### Kubernetes version

Client Version: v1.31.4
Kustomize Version: v5.4.2
Server Version: v1.31.5-eks-8cce635

### Cloud provider

AWS/EKS

Pull request:
Avoid panics when merging custom annotation/labels
This checks that the target maps when merging custom annotations/labels are not `nil`, thereby avoiding panics. It fixes #2226.

Patch:
diff --git a/controllers/controllers.go b/controllers/controllers.go
index e0eadd63..c34a4b84 100644
--- a/controllers/controllers.go
+++ b/controllers/controllers.go
@@ -49,19 +49,27 @@ func metadataMatches(currentMetadata metav1.ObjectMeta, desiredMetadata metav1.O
 	return containsAll(currentMetadata.Labels, desiredMetadata.Labels) && containsAll(currentMetadata.Annotations, desiredMetadata.Annotations)
 }
 
-// mergeLabels merges the the labels specified by the operator into
+// mergeLabels merges the labels specified by the operator into
 // on object's metadata.
 //
 // This will return whether the target's labels have changed.
 func mergeLabelsInMetadata(target *metav1.ObjectMeta, desired metav1.ObjectMeta) bool {
+	if target.Labels == nil && len(desired.Labels) > 0 {
+		target.Labels = map[string]string{}
+	}
+
 	return mergeMap(target.Labels, desired.Labels)
 }
 
-// mergeAnnotations merges the the annotations specified by the operator into
+// mergeAnnotations merges the annotations specified by the operator into
 // on object's metadata.
 //
 // This will return whether the target's annotations have changed.
 func mergeAnnotations(target *metav1.ObjectMeta, desired metav1.ObjectMeta) bool {
+	if target.Annotations == nil && len(desired.Annotations) > 0 {
+		target.Annotations = map[string]string{}
+	}
+
 	return mergeMap(target.Annotations, desired.Annotations)
 }
 


Test patch:
diff --git a/controllers/controllers_test.go b/controllers/controllers_test.go
new file mode 100644
index 00000000..61f5a7cb
--- /dev/null
+++ b/controllers/controllers_test.go
@@ -0,0 +1,212 @@
+package controllers
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+var _ = Describe("mergeLabelsInMetadata", func() {
+	var metadata *metav1.ObjectMeta
+
+	When("the target map is populated", func() {
+		BeforeEach(func() {
+			metadata = &metav1.ObjectMeta{}
+			metadata.Labels = map[string]string{
+				"existing-label": "existing-value",
+			}
+		})
+
+		Context("and the desired map contains a new label", func() {
+			var desired = metav1.ObjectMeta{}
+			desired.Labels = map[string]string{
+				"new-label": "new-value",
+			}
+
+			It("should add the new label to the target", func() {
+				Expect(mergeLabelsInMetadata(metadata, desired)).To(Equal(true))
+				Expect(metadata.Labels).To(Equal(map[string]string{
+					"existing-label": "existing-value",
+					"new-label":      "new-value",
+				}))
+			})
+		})
+	})
+
+	When("the target map is nil", func() {
+		BeforeEach(func() {
+			metadata = &metav1.ObjectMeta{}
+		})
+
+		Context("and the desired map contains a new label", func() {
+			var desired = metav1.ObjectMeta{}
+			desired.Labels = map[string]string{
+				"new-label": "new-value",
+			}
+
+			It("should add the new label to the target", func() {
+				Expect(mergeLabelsInMetadata(metadata, desired)).To(Equal(true))
+				Expect(metadata.Labels).To(Equal(map[string]string{
+					"new-label": "new-value",
+				}))
+			})
+		})
+	})
+})
+
+var _ = Describe("mergeAnnotations", func() {
+	var metadata *metav1.ObjectMeta
+
+	When("the target map is populated", func() {
+		BeforeEach(func() {
+			metadata = &metav1.ObjectMeta{}
+			metadata.Annotations = map[string]string{
+				"existing-annotation": "existing-value",
+			}
+		})
+
+		Context("and the desired map contains a new annotation", func() {
+			var desired = metav1.ObjectMeta{}
+			desired.Annotations = map[string]string{
+				"new-annotation": "new-value",
+			}
+
+			It("should add the new annotation to the target", func() {
+				Expect(mergeAnnotations(metadata, desired)).To(Equal(true))
+				Expect(metadata.Annotations).To(Equal(map[string]string{
+					"existing-annotation": "existing-value",
+					"new-annotation":      "new-value",
+				}))
+			})
+		})
+	})
+
+	When("the target map is nil", func() {
+		BeforeEach(func() {
+			metadata = &metav1.ObjectMeta{}
+		})
+
+		Context("and the desired map contains a new annotation", func() {
+			var desired = metav1.ObjectMeta{}
+			desired.Annotations = map[string]string{
+				"new-annotation": "new-value",
+			}
+
+			It("should add the new annotation to the target", func() {
+				Expect(mergeAnnotations(metadata, desired)).To(Equal(true))
+				Expect(metadata.Annotations).To(Equal(map[string]string{
+					"new-annotation": "new-value",
+				}))
+			})
+		})
+	})
+})
+
+var _ = Describe("mergeMap", func() {
+	When("the target map is populated", func() {
+		var target map[string]string
+
+		BeforeEach(func() {
+			target = map[string]string{
+				"test-key": "test-value",
+			}
+		})
+
+		Context("and the desired map is populated with a new key/value pair", func() {
+			var desired = map[string]string{
+				"new-key": "new-value",
+			}
+
+			It("should add the new value to the target map", func() {
+				Expect(mergeMap(target, desired)).To(Equal(true))
+				Expect(target).To(Equal(map[string]string{
+					"test-key": "test-value",
+					"new-key":  "new-value",
+				}))
+			})
+		})
+
+		Context("and the desired map is populated with a new value for an existing key", func() {
+			var desired = map[string]string{
+				"test-key": "new-value",
+			}
+
+			It("should add the new value to the target map", func() {
+				Expect(mergeMap(target, desired)).To(Equal(true))
+				Expect(target).To(Equal(map[string]string{
+					"test-key": "new-value",
+				}))
+			})
+		})
+
+		Context("and the desired map is populated with the same value for an existing key", func() {
+			var desired = map[string]string{
+				"test-key": "test-value",
+			}
+
+			It("should not change the target map", func() {
+				Expect(mergeMap(target, desired)).To(Equal(false))
+				Expect(target).To(Equal(map[string]string{
+					"test-key": "test-value",
+				}))
+			})
+		})
+
+		Context("and the desired map is empty", func() {
+			var desired = map[string]string{}
+
+			It("should not change the target map", func() {
+				Expect(mergeMap(target, desired)).To(Equal(false))
+				Expect(target).To(Equal(map[string]string{
+					"test-key": "test-value",
+				}))
+			})
+		})
+
+		Context("and the desired map is nil", func() {
+			It("should not change the target map", func() {
+				Expect(mergeMap(target, nil)).To(Equal(false))
+				Expect(target).To(Equal(map[string]string{
+					"test-key": "test-value",
+				}))
+			})
+		})
+	})
+
+	When("the target map is empty", func() {
+		var target map[string]string
+
+		BeforeEach(func() {
+			target = map[string]string{}
+		})
+
+		Context("and the desired map is populated with a new key/value pair", func() {
+			var desired = map[string]string{
+				"new-key": "new-value",
+			}
+
+			It("should add the new value to the target map", func() {
+				Expect(mergeMap(target, desired)).To(Equal(true))
+				Expect(target).To(Equal(map[string]string{
+					"new-key": "new-value",
+				}))
+			})
+		})
+
+		Context("and the desired map is empty", func() {
+			var desired = map[string]string{}
+
+			It("should not change the target map", func() {
+				Expect(mergeMap(target, desired)).To(Equal(false))
+				Expect(target).To(Equal(map[string]string{}))
+			})
+		})
+
+		Context("and the desired map is nil", func() {
+			It("should not change the target map", func() {
+				Expect(mergeMap(target, nil)).To(Equal(false))
+				Expect(target).To(Equal(map[string]string{}))
+			})
+		})
+	})
+})