fix: panic when patching empty secret (#1474)

When patching an empty secret, i.e. one with no data field, the
controller will panic as it's trying to assign to a nil map. Same will
happen with the labels.

- Always initialize the Data and Labels maps if they are empty
- Add a test to cover this use case

fixes: #1282

Signed-off-by: Alejandro Moreno <[email protected]>

Author: alemorcuq

integration/controller_test.go

@@ -438,6 +438,50 @@ var _ = Describe("create", func() {
 				}, Timeout, PollingInterval).Should(WithTransform(getLabels, Equal(expectedLabels)))
 			})
 		})
+
+		Context("With patch annotation and empty secret", func() {
+			BeforeEach(func() {
+				// Empty secret has no data nor labels field
+				s.Data = nil
+				s.Labels = nil
+				s.Annotations = map[string]string{
+					ssv1alpha1.SealedSecretPatchAnnotation: "true",
+				}
+				c.Secrets(ns).Create(ctx, s, metav1.CreateOptions{})
+			})
+
+			It("should not take ownership of existing Secret while patching the Secret", func() {
+				expectedData := map[string][]byte{
+					"foo": []byte("bar"),
+				}
+				expectedAnnotations := map[string]string{
+					ssv1alpha1.SealedSecretPatchAnnotation: "true",
+				}
+				expectedLabels := map[string]string{
+					"mylabel": "myvalue",
+				}
+				Eventually(func() (*v1.EventList, error) {
+					return c.Events(ns).Search(scheme.Scheme, ss)
+				}, Timeout, PollingInterval).Should(
+					containEventWithReason(Equal("Unsealed")),
+				)
+				Eventually(func() (*ssv1alpha1.SealedSecret, error) {
+					return ssc.BitnamiV1alpha1().SealedSecrets(ns).Get(context.Background(), secretName, metav1.GetOptions{})
+				}, Timeout, PollingInterval).ShouldNot(WithTransform(getStatus, BeNil()))
+				Eventually(func() (*v1.Secret, error) {
+					return c.Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+				}, Timeout, PollingInterval).Should(WithTransform(getNumberOfOwners, Equal(0)))
+				Eventually(func() (*v1.Secret, error) {
+					return c.Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+				}, Timeout, PollingInterval).Should(WithTransform(getData, Equal(expectedData)))
+				Eventually(func() (*v1.Secret, error) {
+					return c.Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+				}, Timeout, PollingInterval).Should(WithTransform(getAnnotations, Equal(expectedAnnotations)))
+				Eventually(func() (*v1.Secret, error) {
+					return c.Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+				}, Timeout, PollingInterval).Should(WithTransform(getLabels, Equal(expectedLabels)))
+			})
+		})
 	})
 
 	Describe("Secret Recreation", func() {

pkg/controller/controller.go

@@ -359,18 +359,26 @@ func (c *Controller) unseal(ctx context.Context, key string) (unsealErr error) {
 	secret = secret.DeepCopy()
 
 	if isAnnotatedToBePatched(secret) {
+		if secret.Data == nil {
+			secret.Data = make(map[string][]byte)
+		}
+
 		for k, v := range newSecret.Data {
 			secret.Data[k] = v
 		}
 
-		for k, v := range newSecret.ObjectMeta.Annotations {
-			secret.ObjectMeta.Annotations[k] = v
+		if secret.ObjectMeta.Labels == nil {
+			secret.ObjectMeta.Labels = make(map[string]string)
 		}
 
 		for k, v := range newSecret.ObjectMeta.Labels {
 			secret.ObjectMeta.Labels[k] = v
 		}
 
+		for k, v := range newSecret.ObjectMeta.Annotations {
+			secret.ObjectMeta.Annotations[k] = v
+		}
+
 		if isAnnotatedToBeManaged(secret) {
 			secret.ObjectMeta.OwnerReferences = newSecret.ObjectMeta.OwnerReferences
 		}