Skip to content

Commit

Permalink
fix: panic when patching empty secret
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alemorcuq committed Feb 29, 2024
1 parent b47834d commit 912b404
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 2 deletions.
44 changes: 44 additions & 0 deletions integration/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
12 changes: 10 additions & 2 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 912b404

Please sign in to comment.