diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index 06120511..83cea5ad 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -51,8 +51,10 @@ var _ = Describe("Deployment controller Suite", func() { var ownerRef metav1.OwnerReference var cm1 *corev1.ConfigMap var cm2 *corev1.ConfigMap + var cm3 *corev1.ConfigMap var s1 *corev1.Secret var s2 *corev1.Secret + var s3 *corev1.Secret var waitForDeploymentReconciled = func(obj core.Object) { request := reconcile.Request{ @@ -80,17 +82,23 @@ var _ = Describe("Deployment controller Suite", func() { // Create some configmaps and secrets cm1 = utils.ExampleConfigMap1.DeepCopy() cm2 = utils.ExampleConfigMap2.DeepCopy() + cm3 = utils.ExampleConfigMap3.DeepCopy() s1 = utils.ExampleSecret1.DeepCopy() s2 = utils.ExampleSecret2.DeepCopy() + s3 = utils.ExampleSecret3.DeepCopy() m.Create(cm1).Should(Succeed()) m.Create(cm2).Should(Succeed()) + m.Create(cm3).Should(Succeed()) m.Create(s1).Should(Succeed()) m.Create(s2).Should(Succeed()) + m.Create(s3).Should(Succeed()) m.Get(cm1, timeout).Should(Succeed()) m.Get(cm2, timeout).Should(Succeed()) + m.Get(cm3, timeout).Should(Succeed()) m.Get(s1, timeout).Should(Succeed()) m.Get(s2, timeout).Should(Succeed()) + m.Get(s3, timeout).Should(Succeed()) deployment = utils.ExampleDeployment.DeepCopy() @@ -160,7 +168,7 @@ var _ = Describe("Deployment controller Suite", func() { }) It("Adds OwnerReferences to all children", func() { - for _, obj := range []core.Object{cm1, cm2, s1, s2} { + for _, obj := range []core.Object{cm1, cm2, cm3, s1, s2, s3} { m.Eventually(obj, timeout).Should(utils.WithOwnerReferences(ContainElement(ownerRef))) } }) @@ -181,7 +189,7 @@ var _ = Describe("Deployment controller Suite", func() { return event.Message } - hashMessage := "Configuration hash updated to 198df8455a4fd702fc0c7fdfa4bdb213363b96240bfd48b7b098d936499315a1" + hashMessage := "Configuration hash updated to ebabf80ef45218b27078a41ca16b35a4f91cb5672f389e520ae9da6ee3df3b1c" m.Eventually(events, timeout).Should(utils.WithItems(ContainElement(WithTransform(eventMessage, Equal(hashMessage))))) }) @@ -212,7 +220,7 @@ var _ = Describe("Deployment controller Suite", func() { }) It("Updates the config hash in the Pod Template", func() { - m.Eventually(deployment, timeout).ShouldNot(utils.WithAnnotations(HaveKeyWithValue(core.ConfigHashAnnotation, originalHash))) + m.Eventually(deployment, timeout).ShouldNot(utils.WithPodTemplateAnnotations(HaveKeyWithValue(core.ConfigHashAnnotation, originalHash))) }) }) diff --git a/pkg/core/children.go b/pkg/core/children.go index 8a824cf4..9828d4cf 100644 --- a/pkg/core/children.go +++ b/pkg/core/children.go @@ -28,69 +28,88 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +// configMetadata contains information about ConfigMaps/Secrets referenced +// within PodTemplates +// +// maps of configMetadata are return from the getChildNamesByType method +// configMetadata is also used to pass info through the getObject methods +type configMetadata struct { + required bool + allKeys bool + keys map[string]struct{} +} + // getResult is returned from the getObject method as a helper struct to be // passed into a channel type getResult struct { - err error - obj Object + err error + obj Object + metadata configMetadata } // getCurrentChildren returns a list of all Secrets and ConfigMaps that are -// referenced in the Deployment's spec -func (h *Handler) getCurrentChildren(obj *appsv1.Deployment) ([]Object, error) { +// referenced in the Deployment's spec. Any reference to a whole ConfigMap or Secret +// (i.e. via an EnvFrom or a Volume) will result in one entry in the list, irrespective of +// whether individual elements are also references (i.e. via an Env entry). +func (h *Handler) getCurrentChildren(obj *appsv1.Deployment) ([]configObject, error) { configMaps, secrets := getChildNamesByType(obj) // get all of ConfigMaps and Secrets resultsChan := make(chan getResult) - for name := range configMaps { - go func(name string) { - resultsChan <- h.getConfigMap(obj.GetNamespace(), name) - }(name) + for name, metadata := range configMaps { + go func(name string, metadata configMetadata) { + resultsChan <- h.getConfigMap(obj.GetNamespace(), name, metadata) + }(name, metadata) } - for name := range secrets { - go func(name string) { - resultsChan <- h.getSecret(obj.GetNamespace(), name) - }(name) + for name, metadata := range secrets { + go func(name string, metadata configMetadata) { + resultsChan <- h.getSecret(obj.GetNamespace(), name, metadata) + }(name, metadata) } // Range over and collect results from the gets var errs []string - var children []Object + var children []configObject for i := 0; i < len(configMaps)+len(secrets); i++ { result := <-resultsChan if result.err != nil { errs = append(errs, result.err.Error()) } if result.obj != nil { - children = append(children, result.obj) + children = append(children, configObject{ + object: result.obj, + required: result.metadata.required, + allKeys: result.metadata.allKeys, + keys: result.metadata.keys, + }) } } // If there were any errors, don't return any children if len(errs) > 0 { - return []Object{}, fmt.Errorf("error(s) encountered when geting children: %s", strings.Join(errs, ", ")) + return []configObject{}, fmt.Errorf("error(s) encountered when geting children: %s", strings.Join(errs, ", ")) } // No errors, return the list of children return children, nil } -// getChildNamesByType parses the Deployment object and returns two sets, -// the first containing the names of all referenced ConfigMaps, -// the second containing the names of all referenced Secrets -func getChildNamesByType(obj *appsv1.Deployment) (map[string]struct{}, map[string]struct{}) { +// getChildNamesByType parses the Deployment object and returns two maps, +// the first containing ConfigMap metadata for all referenced ConfigMaps, keyed on the name of the ConfigMap, +// the second containing Secret metadata for all referenced Secrets, keyed on the name of the Secrets +func getChildNamesByType(obj *appsv1.Deployment) (map[string]configMetadata, map[string]configMetadata) { // Create sets for storing the names fo the ConfigMaps/Secrets - configMaps := make(map[string]struct{}) - secrets := make(map[string]struct{}) + configMaps := make(map[string]configMetadata) + secrets := make(map[string]configMetadata) // Range through all Volumes and check the VolumeSources for ConfigMaps // and Secrets for _, vol := range obj.Spec.Template.Spec.Volumes { if cm := vol.VolumeSource.ConfigMap; cm != nil { - configMaps[cm.Name] = struct{}{} + configMaps[cm.Name] = configMetadata{required: true, allKeys: true} } if s := vol.VolumeSource.Secret; s != nil { - secrets[s.SecretName] = struct{}{} + secrets[s.SecretName] = configMetadata{required: true, allKeys: true} } } @@ -99,10 +118,24 @@ func getChildNamesByType(obj *appsv1.Deployment) (map[string]struct{}, map[strin for _, container := range obj.Spec.Template.Spec.Containers { for _, env := range container.EnvFrom { if cm := env.ConfigMapRef; cm != nil { - configMaps[cm.Name] = struct{}{} + configMaps[cm.Name] = configMetadata{required: true, allKeys: true} } if s := env.SecretRef; s != nil { - secrets[s.Name] = struct{}{} + secrets[s.Name] = configMetadata{required: true, allKeys: true} + } + } + } + + // Range through all Containers and their respective Env + for _, container := range obj.Spec.Template.Spec.Containers { + for _, env := range container.Env { + if valFrom := env.ValueFrom; valFrom != nil { + if cm := valFrom.ConfigMapKeyRef; cm != nil { + configMaps[cm.Name] = parseConfigMapKeyRef(configMaps[cm.Name], cm) + } + if s := valFrom.SecretKeyRef; s != nil { + secrets[s.Name] = parseSecretKeyRef(secrets[s.Name], s) + } } } } @@ -110,27 +143,58 @@ func getChildNamesByType(obj *appsv1.Deployment) (map[string]struct{}, map[strin return configMaps, secrets } +// parseConfigMapKeyRef updates the metadata for a ConfigMap to include the keys specified in this ConfigMapKeySelector +func parseConfigMapKeyRef(metadata configMetadata, cm *corev1.ConfigMapKeySelector) configMetadata { + if !metadata.allKeys { + if metadata.keys == nil { + metadata.keys = make(map[string]struct{}) + } + if cm.Optional == nil || !*cm.Optional { + metadata.required = true + } + metadata.keys[cm.Key] = struct{}{} + } + return metadata +} + +// parseSecretKeyRef updates the metadata for a Secret to include the keys specified in this SecretKeySelector +func parseSecretKeyRef(metadata configMetadata, s *corev1.SecretKeySelector) configMetadata { + if !metadata.allKeys { + if metadata.keys == nil { + metadata.keys = make(map[string]struct{}) + } + if s.Optional == nil || !*s.Optional { + metadata.required = true + } + metadata.keys[s.Key] = struct{}{} + } + return metadata +} + // getConfigMap gets a ConfigMap with the given name and namespace from the // API server. -func (h *Handler) getConfigMap(namespace, name string) getResult { - return h.getObject(namespace, name, &corev1.ConfigMap{}) +func (h *Handler) getConfigMap(namespace, name string, metadata configMetadata) getResult { + return h.getObject(namespace, name, metadata, &corev1.ConfigMap{}) } // getSecret gets a Secret with the given name and namespace from the // API server. -func (h *Handler) getSecret(namespace, name string) getResult { - return h.getObject(namespace, name, &corev1.Secret{}) +func (h *Handler) getSecret(namespace, name string, metadata configMetadata) getResult { + return h.getObject(namespace, name, metadata, &corev1.Secret{}) } // getObject gets the Object with the given name and namespace from the API // server -func (h *Handler) getObject(namespace, name string, obj Object) getResult { - key := types.NamespacedName{Namespace: namespace, Name: name} - err := h.Get(context.TODO(), key, obj) +func (h *Handler) getObject(namespace, name string, metadata configMetadata, obj Object) getResult { + objectName := types.NamespacedName{Namespace: namespace, Name: name} + err := h.Get(context.TODO(), objectName, obj) if err != nil { - return getResult{err: err} + if metadata.required { + return getResult{err: err} + } + return getResult{metadata: metadata} } - return getResult{obj: obj} + return getResult{obj: obj, metadata: metadata} } // getExistingChildren returns a list of all Secrets and ConfigMaps that are diff --git a/pkg/core/children_test.go b/pkg/core/children_test.go index 1b71ed7a..c97084ee 100644 --- a/pkg/core/children_test.go +++ b/pkg/core/children_test.go @@ -35,7 +35,8 @@ var _ = Describe("Wave children Suite", func() { var h *Handler var m utils.Matcher var deployment *appsv1.Deployment - var children []Object + var existingChildren []Object + var currentChildren []configObject var mgrStopped *sync.WaitGroup var stopMgr chan struct{} @@ -43,8 +44,12 @@ var _ = Describe("Wave children Suite", func() { var cm1 *corev1.ConfigMap var cm2 *corev1.ConfigMap + var cm3 *corev1.ConfigMap + var cm4 *corev1.ConfigMap var s1 *corev1.Secret var s2 *corev1.Secret + var s3 *corev1.Secret + var s4 *corev1.Secret BeforeEach(func() { mgr, err := manager.New(cfg, manager.Options{}) @@ -56,13 +61,21 @@ var _ = Describe("Wave children Suite", func() { // Create some configmaps and secrets cm1 = utils.ExampleConfigMap1.DeepCopy() cm2 = utils.ExampleConfigMap2.DeepCopy() + cm3 = utils.ExampleConfigMap3.DeepCopy() + cm4 = utils.ExampleConfigMap4.DeepCopy() s1 = utils.ExampleSecret1.DeepCopy() s2 = utils.ExampleSecret2.DeepCopy() + s3 = utils.ExampleSecret3.DeepCopy() + s4 = utils.ExampleSecret4.DeepCopy() m.Create(cm1).Should(Succeed()) m.Create(cm2).Should(Succeed()) + m.Create(cm3).Should(Succeed()) + m.Create(cm4).Should(Succeed()) m.Create(s1).Should(Succeed()) m.Create(s2).Should(Succeed()) + m.Create(s3).Should(Succeed()) + m.Create(s4).Should(Succeed()) deployment = utils.ExampleDeployment.DeepCopy() m.Create(deployment).Should(Succeed()) @@ -72,8 +85,13 @@ var _ = Describe("Wave children Suite", func() { // Ensure the caches have synced m.Get(cm1, timeout).Should(Succeed()) m.Get(cm2, timeout).Should(Succeed()) + m.Get(cm3, timeout).Should(Succeed()) + m.Get(cm4, timeout).Should(Succeed()) m.Get(s1, timeout).Should(Succeed()) m.Get(s2, timeout).Should(Succeed()) + m.Get(s3, timeout).Should(Succeed()) + m.Get(s4, timeout).Should(Succeed()) + m.Get(deployment, timeout).Should(Succeed()) }) AfterEach(func() { @@ -90,28 +108,86 @@ var _ = Describe("Wave children Suite", func() { Context("getCurrentChildren", func() { BeforeEach(func() { var err error - children, err = h.getCurrentChildren(deployment) + currentChildren, err = h.getCurrentChildren(deployment) Expect(err).NotTo(HaveOccurred()) }) It("returns ConfigMaps referenced in Volumes", func() { - Expect(children).To(ContainElement(cm1)) + Expect(currentChildren).To(ContainElement(configObject{ + object: cm1, + required: true, + allKeys: true, + })) }) It("returns ConfigMaps referenced in EnvFrom", func() { - Expect(children).To(ContainElement(cm2)) + Expect(currentChildren).To(ContainElement(configObject{ + object: cm2, + required: true, + allKeys: true, + })) + }) + + It("returns ConfigMaps referenced in Env", func() { + Expect(currentChildren).To(ContainElement(configObject{ + object: cm3, + required: true, + allKeys: false, + keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + "key4": {}, + }, + })) + Expect(currentChildren).To(ContainElement(configObject{ + object: cm4, + required: false, + allKeys: false, + keys: map[string]struct{}{ + "key1": {}, + }, + })) }) It("returns Secrets referenced in Volumes", func() { - Expect(children).To(ContainElement(s1)) + Expect(currentChildren).To(ContainElement(configObject{ + object: s1, + required: true, + allKeys: true, + })) }) It("returns Secrets referenced in EnvFrom", func() { - Expect(children).To(ContainElement(s2)) + Expect(currentChildren).To(ContainElement(configObject{ + object: s2, + required: true, + allKeys: true, + })) + }) + + It("returns Secrets referenced in Env", func() { + Expect(currentChildren).To(ContainElement(configObject{ + object: s3, + required: true, + allKeys: false, + keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + "key4": {}, + }, + })) + Expect(currentChildren).To(ContainElement(configObject{ + object: s4, + required: false, + allKeys: false, + keys: map[string]struct{}{ + "key1": {}, + }, + })) }) It("does not return duplicate children", func() { - Expect(children).To(HaveLen(4)) + Expect(currentChildren).To(HaveLen(8)) }) It("returns an error if one of the referenced children is missing", func() { @@ -126,32 +202,56 @@ var _ = Describe("Wave children Suite", func() { }) Context("getChildNamesByType", func() { - var configMaps map[string]struct{} - var secrets map[string]struct{} + var configMaps map[string]configMetadata + var secrets map[string]configMetadata BeforeEach(func() { configMaps, secrets = getChildNamesByType(deployment) }) It("returns ConfigMaps referenced in Volumes", func() { - Expect(configMaps).To(HaveKey(cm1.GetName())) + Expect(configMaps).To(HaveKeyWithValue(cm1.GetName(), configMetadata{required: true, allKeys: true})) }) It("returns ConfigMaps referenced in EnvFrom", func() { - Expect(configMaps).To(HaveKey(cm2.GetName())) + Expect(configMaps).To(HaveKeyWithValue(cm2.GetName(), configMetadata{required: true, allKeys: true})) + }) + + It("returns ConfigMaps referenced in Env", func() { + Expect(configMaps).To(HaveKeyWithValue(cm3.GetName(), configMetadata{ + required: true, + allKeys: false, + keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + "key4": {}, + }, + })) }) It("returns Secrets referenced in Volumes", func() { - Expect(secrets).To(HaveKey(s1.GetName())) + Expect(secrets).To(HaveKeyWithValue(s1.GetName(), configMetadata{required: true, allKeys: true})) }) It("returns Secrets referenced in EnvFrom", func() { - Expect(secrets).To(HaveKey(s2.GetName())) + Expect(secrets).To(HaveKeyWithValue(s2.GetName(), configMetadata{required: true, allKeys: true})) + }) + + It("returns Secrets referenced in Env", func() { + Expect(configMaps).To(HaveKeyWithValue(s3.GetName(), configMetadata{ + required: true, + allKeys: false, + keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + "key4": {}, + }, + })) }) It("does not return extra children", func() { - Expect(configMaps).To(HaveLen(2)) - Expect(secrets).To(HaveLen(2)) + Expect(configMaps).To(HaveLen(4)) + Expect(secrets).To(HaveLen(4)) }) }) @@ -168,28 +268,32 @@ var _ = Describe("Wave children Suite", func() { } var err error - children, err = h.getExistingChildren(deployment) + existingChildren, err = h.getExistingChildren(deployment) Expect(err).NotTo(HaveOccurred()) }) It("returns ConfigMaps with the correct OwnerReference", func() { - Expect(children).To(ContainElement(cm1)) + Expect(existingChildren).To(ContainElement(cm1)) }) It("doesn't return ConfigMaps without OwnerReferences", func() { - Expect(children).NotTo(ContainElement(cm2)) + Expect(existingChildren).NotTo(ContainElement(cm2)) + Expect(existingChildren).NotTo(ContainElement(cm3)) + Expect(existingChildren).NotTo(ContainElement(cm4)) }) It("returns Secrets with the correct OwnerReference", func() { - Expect(children).To(ContainElement(s1)) + Expect(existingChildren).To(ContainElement(s1)) }) It("doesn't return Secrets without OwnerReferences", func() { - Expect(children).NotTo(ContainElement(s2)) + Expect(existingChildren).NotTo(ContainElement(s2)) + Expect(existingChildren).NotTo(ContainElement(s3)) + Expect(existingChildren).NotTo(ContainElement(s4)) }) It("does not return duplicate children", func() { - Expect(children).To(HaveLen(2)) + Expect(existingChildren).To(HaveLen(2)) }) }) diff --git a/pkg/core/handler_test.go b/pkg/core/handler_test.go index 7b89a0b5..9ba9c3e6 100644 --- a/pkg/core/handler_test.go +++ b/pkg/core/handler_test.go @@ -49,8 +49,10 @@ var _ = Describe("Wave controller Suite", func() { var ownerRef metav1.OwnerReference var cm1 *corev1.ConfigMap var cm2 *corev1.ConfigMap + var cm3 *corev1.ConfigMap var s1 *corev1.Secret var s2 *corev1.Secret + var s3 *corev1.Secret BeforeEach(func() { mgr, err := manager.New(cfg, manager.Options{}) @@ -64,17 +66,23 @@ var _ = Describe("Wave controller Suite", func() { // Create some configmaps and secrets cm1 = utils.ExampleConfigMap1.DeepCopy() cm2 = utils.ExampleConfigMap2.DeepCopy() + cm3 = utils.ExampleConfigMap3.DeepCopy() s1 = utils.ExampleSecret1.DeepCopy() s2 = utils.ExampleSecret2.DeepCopy() + s3 = utils.ExampleSecret3.DeepCopy() m.Create(cm1).Should(Succeed()) m.Create(cm2).Should(Succeed()) + m.Create(cm3).Should(Succeed()) m.Create(s1).Should(Succeed()) m.Create(s2).Should(Succeed()) + m.Create(s3).Should(Succeed()) m.Get(cm1, timeout).Should(Succeed()) m.Get(cm2, timeout).Should(Succeed()) + m.Get(cm3, timeout).Should(Succeed()) m.Get(s1, timeout).Should(Succeed()) m.Get(s2, timeout).Should(Succeed()) + m.Get(s3, timeout).Should(Succeed()) deployment = utils.ExampleDeployment.DeepCopy() @@ -147,7 +155,7 @@ var _ = Describe("Wave controller Suite", func() { }) It("Adds OwnerReferences to all children", func() { - for _, obj := range []Object{cm1, cm2, s1, s2} { + for _, obj := range []Object{cm1, cm2, cm3, s1, s2, s3} { m.Eventually(obj, timeout).Should(utils.WithOwnerReferences(ContainElement(ownerRef))) } }) @@ -168,7 +176,7 @@ var _ = Describe("Wave controller Suite", func() { return event.Message } - hashMessage := "Configuration hash updated to 198df8455a4fd702fc0c7fdfa4bdb213363b96240bfd48b7b098d936499315a1" + hashMessage := "Configuration hash updated to ebabf80ef45218b27078a41ca16b35a4f91cb5672f389e520ae9da6ee3df3b1c" m.Eventually(events, timeout).Should(utils.WithItems(ContainElement(WithTransform(eventMessage, Equal(hashMessage))))) }) @@ -226,7 +234,7 @@ var _ = Describe("Wave controller Suite", func() { }) It("Updates the config hash in the Pod Template", func() { - m.Eventually(deployment, timeout).ShouldNot(utils.WithAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) + m.Eventually(deployment, timeout).ShouldNot(utils.WithPodTemplateAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) }) }) @@ -244,7 +252,43 @@ var _ = Describe("Wave controller Suite", func() { }) It("Updates the config hash in the Pod Template", func() { - m.Eventually(deployment, timeout).ShouldNot(utils.WithAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) + m.Eventually(deployment, timeout).ShouldNot(utils.WithPodTemplateAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) + }) + }) + + Context("A ConfigMap Env for a key being used is updated", func() { + BeforeEach(func() { + m.Get(cm3, timeout).Should(Succeed()) + cm3.Data["key1"] = "modified" + m.Update(cm3).Should(Succeed()) + + _, err := h.HandleDeployment(deployment) + Expect(err).NotTo(HaveOccurred()) + + // Get the updated Deployment + m.Get(deployment, timeout).Should(Succeed()) + }) + + It("Updates the config hash in the Pod Template", func() { + m.Eventually(deployment, timeout).ShouldNot(utils.WithPodTemplateAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) + }) + }) + + Context("A ConfigMap Env for a key that is not being used is updated", func() { + BeforeEach(func() { + m.Get(cm3, timeout).Should(Succeed()) + cm3.Data["key3"] = "modified" + m.Update(cm3).Should(Succeed()) + + _, err := h.HandleDeployment(deployment) + Expect(err).NotTo(HaveOccurred()) + + // Get the updated Deployment + m.Get(deployment, timeout).Should(Succeed()) + }) + + It("Does not update the config hash in the Pod Template", func() { + m.Consistently(deployment, consistentlyTimeout).Should(utils.WithPodTemplateAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) }) }) @@ -265,7 +309,7 @@ var _ = Describe("Wave controller Suite", func() { }) It("Updates the config hash in the Pod Template", func() { - m.Eventually(deployment, timeout).ShouldNot(utils.WithAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) + m.Eventually(deployment, timeout).ShouldNot(utils.WithPodTemplateAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) }) }) @@ -286,7 +330,49 @@ var _ = Describe("Wave controller Suite", func() { }) It("Updates the config hash in the Pod Template", func() { - m.Eventually(deployment, timeout).ShouldNot(utils.WithAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) + m.Eventually(deployment, timeout).ShouldNot(utils.WithPodTemplateAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) + }) + }) + + Context("A Secret Env for a key being used is updated", func() { + BeforeEach(func() { + m.Get(s3, timeout).Should(Succeed()) + if s3.StringData == nil { + s3.StringData = make(map[string]string) + } + s3.StringData["key1"] = "modified" + m.Update(s3).Should(Succeed()) + + _, err := h.HandleDeployment(deployment) + Expect(err).NotTo(HaveOccurred()) + + // Get the updated Deployment + m.Get(deployment, timeout).Should(Succeed()) + }) + + It("Updates the config hash in the Pod Template", func() { + m.Eventually(deployment, timeout).ShouldNot(utils.WithPodTemplateAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) + }) + }) + + Context("A Secret Env for a key that is not being used is updated", func() { + BeforeEach(func() { + m.Get(s3, timeout).Should(Succeed()) + if s3.StringData == nil { + s3.StringData = make(map[string]string) + } + s3.StringData["key3"] = "modified" + m.Update(s3).Should(Succeed()) + + _, err := h.HandleDeployment(deployment) + Expect(err).NotTo(HaveOccurred()) + + // Get the updated Deployment + m.Get(deployment, timeout).Should(Succeed()) + }) + + It("Does not update the config hash in the Pod Template", func() { + m.Consistently(deployment, consistentlyTimeout).Should(utils.WithPodTemplateAnnotations(HaveKeyWithValue(ConfigHashAnnotation, originalHash))) }) }) }) diff --git a/pkg/core/hash.go b/pkg/core/hash.go index b3e409b6..8f178f82 100644 --- a/pkg/core/hash.go +++ b/pkg/core/hash.go @@ -28,7 +28,7 @@ import ( // calculateConfigHash uses sha256 to hash the configuration within the child // objects and returns a hash as a string -func calculateConfigHash(children []Object) (string, error) { +func calculateConfigHash(children []configObject) (string, error) { // hashSource contains all the data to be hashed hashSource := struct { ConfigMaps map[string]map[string]string `json:"configMaps"` @@ -41,16 +41,16 @@ func calculateConfigHash(children []Object) (string, error) { // Add the data from each child to the hashSource // All children should be in the same namespace so each one should have a // unique name - for _, obj := range children { - switch child := obj.(type) { - case *corev1.ConfigMap: - cm := corev1.ConfigMap(*child) - hashSource.ConfigMaps[child.GetName()] = cm.Data - case *corev1.Secret: - s := corev1.Secret(*child) - hashSource.Secrets[child.GetName()] = s.Data - default: - return "", fmt.Errorf("passed unknown type: %v", reflect.TypeOf(child)) + for _, child := range children { + if child.object != nil { + switch child.object.(type) { + case *corev1.ConfigMap: + hashSource.ConfigMaps[child.object.GetName()] = getConfigMapData(child) + case *corev1.Secret: + hashSource.Secrets[child.object.GetName()] = getSecretData(child) + default: + return "", fmt.Errorf("passed unknown type: %v", reflect.TypeOf(child)) + } } } @@ -64,6 +64,38 @@ func calculateConfigHash(children []Object) (string, error) { return fmt.Sprintf("%x", hashBytes), nil } +// getConfigMapData extracts all the relevant data from the ConfigMap, whether that is +// the whole ConfigMap or only the specified keys. +func getConfigMapData(child configObject) map[string]string { + cm := *child.object.(*corev1.ConfigMap) + if child.allKeys { + return cm.Data + } + keyData := make(map[string]string) + for key := range child.keys { + if value, exists := cm.Data[key]; exists { + keyData[key] = value + } + } + return keyData +} + +// getSecretData extracts all the relevant data from the Secret, whether that is +// the whole Secret or only the specified keys. +func getSecretData(child configObject) map[string][]byte { + s := *child.object.(*corev1.Secret) + if child.allKeys { + return s.Data + } + keyData := make(map[string][]byte) + for key := range child.keys { + if value, exists := s.Data[key]; exists { + keyData[key] = value + } + } + return keyData +} + // setConfigHash upates the configuration hash of the given Deployment to the // given string func setConfigHash(obj *appsv1.Deployment, hash string) { diff --git a/pkg/core/hash_test.go b/pkg/core/hash_test.go index 286a4bcc..5d680b0b 100644 --- a/pkg/core/hash_test.go +++ b/pkg/core/hash_test.go @@ -41,8 +41,10 @@ var _ = Describe("Wave hash Suite", func() { var cm1 *corev1.ConfigMap var cm2 *corev1.ConfigMap + var cm3 *corev1.ConfigMap var s1 *corev1.Secret var s2 *corev1.Secret + var s3 *corev1.Secret BeforeEach(func() { mgr, err := manager.New(cfg, manager.Options{}) @@ -54,18 +56,24 @@ var _ = Describe("Wave hash Suite", func() { cm1 = utils.ExampleConfigMap1.DeepCopy() cm2 = utils.ExampleConfigMap2.DeepCopy() + cm3 = utils.ExampleConfigMap3.DeepCopy() s1 = utils.ExampleSecret1.DeepCopy() s2 = utils.ExampleSecret2.DeepCopy() + s3 = utils.ExampleSecret3.DeepCopy() m.Create(cm1).Should(Succeed()) m.Create(cm2).Should(Succeed()) + m.Create(cm3).Should(Succeed()) m.Create(s1).Should(Succeed()) m.Create(s2).Should(Succeed()) + m.Create(s3).Should(Succeed()) m.Get(cm1, timeout).Should(Succeed()) m.Get(cm2, timeout).Should(Succeed()) + m.Get(cm3, timeout).Should(Succeed()) m.Get(s1, timeout).Should(Succeed()) m.Get(s2, timeout).Should(Succeed()) + m.Get(s3, timeout).Should(Succeed()) }) AfterEach(func() { @@ -79,22 +87,105 @@ var _ = Describe("Wave hash Suite", func() { ) }) - It("returns a different hash when a child's data is updated", func() { - c := []Object{cm1, cm2, s1, s2} + It("returns a different hash when an allKeys child's data is updated", func() { + c := []configObject{ + {object: cm1, allKeys: true}, + {object: cm2, allKeys: true}, + {object: s1, allKeys: true}, + {object: s2, allKeys: true}, + } + + h1, err := calculateConfigHash(c) + Expect(err).NotTo(HaveOccurred()) + + cm1.Data["key1"] = "modified" + m.Update(cm1).Should(Succeed()) + h2, err := calculateConfigHash(c) + Expect(err).NotTo(HaveOccurred()) + + Expect(h2).NotTo(Equal(h1)) + }) + + It("returns a different hash when an all-field child's data is updated", func() { + c := []configObject{ + {object: cm1, allKeys: true}, + {object: cm2, allKeys: true}, + {object: s1, allKeys: true}, + {object: s2, allKeys: true}, + } + + h1, err := calculateConfigHash(c) + Expect(err).NotTo(HaveOccurred()) + + cm1.Data["key1"] = "modified" + m.Update(cm1).Should(Succeed()) + h2, err := calculateConfigHash(c) + Expect(err).NotTo(HaveOccurred()) + + Expect(h2).NotTo(Equal(h1)) + }) + + It("returns a different hash when a single-field child's data is updated", func() { + c := []configObject{ + {object: cm1, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + }, + }, + {object: cm2, allKeys: true}, + {object: s1, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + }, + }, + {object: s2, allKeys: true}, + } h1, err := calculateConfigHash(c) Expect(err).NotTo(HaveOccurred()) cm1.Data["key1"] = "modified" m.Update(cm1).Should(Succeed()) + s1.Data["key1"] = []byte("modified") + m.Update(s1).Should(Succeed()) h2, err := calculateConfigHash(c) Expect(err).NotTo(HaveOccurred()) Expect(h2).NotTo(Equal(h1)) }) + It("returns the same hash when a single-field child's data is updated but not for that field", func() { + c := []configObject{ + {object: cm1, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + }, + }, + {object: cm2, allKeys: true}, + {object: s1, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + }, + }, + {object: s2, allKeys: true}, + } + + h1, err := calculateConfigHash(c) + Expect(err).NotTo(HaveOccurred()) + + cm1.Data["key3"] = "modified" + m.Update(cm1).Should(Succeed()) + s1.Data["key3"] = []byte("modified") + m.Update(s1).Should(Succeed()) + h2, err := calculateConfigHash(c) + Expect(err).NotTo(HaveOccurred()) + + Expect(h2).To(Equal(h1)) + }) + It("returns the same hash when a child's metadata is updated", func() { - c := []Object{cm1, cm2, s1, s2} + c := []configObject{ + {object: cm1, allKeys: true}, + {object: cm2, allKeys: true}, + {object: s1, allKeys: true}, + {object: s2, allKeys: true}, + } h1, err := calculateConfigHash(c) Expect(err).NotTo(HaveOccurred()) @@ -108,8 +199,38 @@ var _ = Describe("Wave hash Suite", func() { }) It("returns the same hash independent of child ordering", func() { - c1 := []Object{cm1, cm2, s1, s2} - c2 := []Object{cm1, s2, cm2, s1} + c1 := []configObject{ + {object: cm1, allKeys: true}, + {object: cm2, allKeys: true}, + {object: cm3, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + }, + }, + {object: s1, allKeys: true}, + {object: s2, allKeys: true}, + {object: s3, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + }, + }, + } + c2 := []configObject{ + {object: cm1, allKeys: true}, + {object: s2, allKeys: true}, + {object: s3, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + }, + }, + {object: cm2, allKeys: true}, + {object: s1, allKeys: true}, + {object: cm3, allKeys: false, keys: map[string]struct{}{ + "key2": {}, + "key1": {}, + }, + }, + } h1, err := calculateConfigHash(c1) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/core/owner_references.go b/pkg/core/owner_references.go index 0bf0287e..ea38a048 100644 --- a/pkg/core/owner_references.go +++ b/pkg/core/owner_references.go @@ -55,13 +55,13 @@ func (h *Handler) removeOwnerReferences(obj *appsv1.Deployment, children []Objec // updateOwnerReferences determines which children need to have their // OwnerReferences added/updated and which need to have their OwnerReferences // removed and then performs all updates -func (h *Handler) updateOwnerReferences(owner *appsv1.Deployment, existing, current []Object) error { +func (h *Handler) updateOwnerReferences(owner *appsv1.Deployment, existing []Object, current []configObject) error { // Add an owner reference to each child object errChan := make(chan error) for _, obj := range current { go func(child Object) { errChan <- h.updateOwnerReference(owner, child) - }(obj) + }(obj.object) } // Return any errors encountered updating the child objects @@ -110,7 +110,7 @@ func (h *Handler) updateOwnerReference(owner *appsv1.Deployment, child Object) e // getOrphans creates a slice of orphaned child objects that need their // OwnerReferences removing -func getOrphans(existing, current []Object) []Object { +func getOrphans(existing []Object, current []configObject) []Object { orphans := []Object{} for _, child := range existing { if !isIn(current, child) { @@ -135,9 +135,9 @@ func getOwnerReference(obj *appsv1.Deployment) metav1.OwnerReference { } // isIn checks whether a child object exists within a slice of objects -func isIn(list []Object, child Object) bool { +func isIn(list []configObject, child Object) bool { for _, obj := range list { - if obj.GetUID() == child.GetUID() { + if obj.object.GetUID() == child.GetUID() { return true } } diff --git a/pkg/core/owner_references_test.go b/pkg/core/owner_references_test.go index fcc78b2c..4db985a2 100644 --- a/pkg/core/owner_references_test.go +++ b/pkg/core/owner_references_test.go @@ -43,8 +43,10 @@ var _ = Describe("Wave owner references Suite", func() { var cm1 *corev1.ConfigMap var cm2 *corev1.ConfigMap + var cm3 *corev1.ConfigMap var s1 *corev1.Secret var s2 *corev1.Secret + var s3 *corev1.Secret var ownerRef metav1.OwnerReference BeforeEach(func() { @@ -57,13 +59,17 @@ var _ = Describe("Wave owner references Suite", func() { // Create some configmaps and secrets cm1 = utils.ExampleConfigMap1.DeepCopy() cm2 = utils.ExampleConfigMap2.DeepCopy() + cm3 = utils.ExampleConfigMap3.DeepCopy() s1 = utils.ExampleSecret1.DeepCopy() s2 = utils.ExampleSecret2.DeepCopy() + s3 = utils.ExampleSecret3.DeepCopy() m.Create(cm1).Should(Succeed()) m.Create(cm2).Should(Succeed()) + m.Create(cm3).Should(Succeed()) m.Create(s1).Should(Succeed()) m.Create(s2).Should(Succeed()) + m.Create(s3).Should(Succeed()) deployment = utils.ExampleDeployment.DeepCopy() m.Create(deployment).Should(Succeed()) @@ -143,20 +149,30 @@ var _ = Describe("Wave owner references Suite", func() { m.Eventually(obj, timeout).Should(utils.WithOwnerReferences(ContainElement(ownerRef))) } - existing := []Object{cm2, s1, s2} - current := []Object{cm1, s1} + existing := []Object{cm2, cm3, s1, s2} + current := []configObject{ + {object: cm1, allKeys: true}, + {object: s1, allKeys: true}, + {object: s3, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + }, + }, + } err := h.updateOwnerReferences(deployment, existing, current) Expect(err).NotTo(HaveOccurred()) }) It("removes owner references from those not in current", func() { m.Eventually(cm2, timeout).ShouldNot(utils.WithOwnerReferences(ContainElement(ownerRef))) + m.Eventually(cm3, timeout).ShouldNot(utils.WithOwnerReferences(ContainElement(ownerRef))) m.Eventually(s2, timeout).ShouldNot(utils.WithOwnerReferences(ContainElement(ownerRef))) }) It("adds owner references to those in current", func() { m.Eventually(cm1, timeout).Should(utils.WithOwnerReferences(ContainElement(ownerRef))) m.Eventually(s1, timeout).Should(utils.WithOwnerReferences(ContainElement(ownerRef))) + m.Eventually(s3, timeout).Should(utils.WithOwnerReferences(ContainElement(ownerRef))) }) }) @@ -217,24 +233,84 @@ var _ = Describe("Wave owner references Suite", func() { Context("getOrphans", func() { It("returns an empty list when current and existing match", func() { - current := []Object{cm1, cm2, s1, s2} - existing := current + current := []configObject{ + {object: cm1, allKeys: true}, + {object: cm2, allKeys: true}, + {object: s1, allKeys: true}, + {object: s2, allKeys: true}, + } + existing := []Object{cm1, cm2, s1, s2} Expect(getOrphans(existing, current)).To(BeEmpty()) }) It("returns an empty list when existing is a subset of current", func() { + current := []configObject{ + {object: cm1, allKeys: true}, + {object: cm2, allKeys: true}, + {object: s1, allKeys: true}, + {object: s2, allKeys: true}, + } existing := []Object{cm1, s2} - current := append(existing, cm2, s1) Expect(getOrphans(existing, current)).To(BeEmpty()) }) It("returns the correct objects when current is a subset of existing", func() { - current := []Object{cm1, s2} - existing := append(current, cm2, s1) + current := []configObject{ + {object: cm1, allKeys: true}, + {object: s2, allKeys: true}, + } + existing := []Object{cm1, cm2, s1, s2} orphans := getOrphans(existing, current) Expect(orphans).To(ContainElement(cm2)) Expect(orphans).To(ContainElement(s1)) }) + + Context("when current contains multiple singleField entries", func() { + It("returns an empty list when current and existing match", func() { + current := []configObject{ + {object: cm1, allKeys: true}, + {object: cm2, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + }, + }, + {object: s1, allKeys: true}, + {object: s2, allKeys: true}, + } + existing := []Object{cm1, cm2, s1, s2} + Expect(getOrphans(existing, current)).To(BeEmpty()) + }) + + It("returns an empty list when existing is a subset of current", func() { + current := []configObject{ + {object: cm1, allKeys: true}, + {object: cm2, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + }, + }, + {object: s1, allKeys: true}, + {object: s2, allKeys: true}, + } + existing := []Object{cm1, s2} + Expect(getOrphans(existing, current)).To(BeEmpty()) + }) + + It("returns the correct objects when current is a subset of existing", func() { + current := []configObject{ + {object: cm1, allKeys: true}, + {object: s2, allKeys: false, keys: map[string]struct{}{ + "key1": {}, + "key2": {}, + }, + }, + } + existing := []Object{cm1, cm2, s1, s2} + orphans := getOrphans(existing, current) + Expect(orphans).To(ContainElement(cm2)) + Expect(orphans).To(ContainElement(s1)) + }) + }) }) Context("getOwnerReference", func() { diff --git a/pkg/core/types.go b/pkg/core/types.go index ae9bc4bb..6ae646a0 100644 --- a/pkg/core/types.go +++ b/pkg/core/types.go @@ -42,3 +42,12 @@ type Object interface { runtime.Object metav1.Object } + +// configObject is used as a container of an "Object" along with metadata +// that Wave uses to determine what to use from that Object. +type configObject struct { + object Object + required bool + allKeys bool + keys map[string]struct{} +} diff --git a/test/utils/test_objects.go b/test/utils/test_objects.go index f7186ba3..8545aaac 100644 --- a/test/utils/test_objects.go +++ b/test/utils/test_objects.go @@ -26,6 +26,8 @@ var labels = map[string]string{ "app": "example", } +var trueValue = true + // ExampleDeployment is an example Deployment object for use within test suites var ExampleDeployment = &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -66,6 +68,111 @@ var ExampleDeployment = &appsv1.Deployment{ { Name: "container1", Image: "container1", + Env: []corev1.EnvVar{ + { + Name: "example1_key1", + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example1", + }, + Key: "key1", + }, + }, + }, + { + Name: "example1_key1_new_name", + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example1", + }, + Key: "key1", + }, + }, + }, + { + Name: "example3_key1", + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example3", + }, + Key: "key1", + }, + }, + }, + { + Name: "example3_key4", + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example3", + }, + Key: "key4", + Optional: &trueValue, + }, + }, + }, + { + Name: "example4_key1", + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example4", + }, + Key: "key1", + Optional: &trueValue, + }, + }, + }, + { + Name: "example1_secret_key1", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example1", + }, + Key: "key1", + }, + }, + }, + { + Name: "example3_secret_key1", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example3", + }, + Key: "key1", + }, + }, + }, + { + Name: "example3_secret_key4", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example3", + }, + Key: "key4", + Optional: &trueValue, + }, + }, + }, + { + Name: "example4_secret_key1", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example4", + }, + Key: "key1", + Optional: &trueValue, + }, + }, + }, + }, EnvFrom: []corev1.EnvFromSource{ { ConfigMapRef: &corev1.ConfigMapEnvSource{ @@ -86,6 +193,30 @@ var ExampleDeployment = &appsv1.Deployment{ { Name: "container2", Image: "container2", + Env: []corev1.EnvVar{ + { + Name: "example3_key2", + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example3", + }, + Key: "key2", + }, + }, + }, + { + Name: "example3_secret_key2", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "example3", + }, + Key: "key2", + }, + }, + }, + }, EnvFrom: []corev1.EnvFromSource{ { ConfigMapRef: &corev1.ConfigMapEnvSource{ @@ -137,6 +268,34 @@ var ExampleConfigMap2 = &corev1.ConfigMap{ }, } +// ExampleConfigMap3 is an example ConfigMap object for use within test suites +var ExampleConfigMap3 = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example3", + Namespace: "default", + Labels: labels, + }, + Data: map[string]string{ + "key1": "example3:key1", + "key2": "example3:key2", + "key3": "example3:key3", + }, +} + +// ExampleConfigMap4 is an example ConfigMap object for use within test suites +var ExampleConfigMap4 = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example4", + Namespace: "default", + Labels: labels, + }, + Data: map[string]string{ + "key1": "example4:key1", + "key2": "example4:key2", + "key3": "example4:key3", + }, +} + // ExampleSecret1 is an example Secret object for use within test suites var ExampleSecret1 = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -164,3 +323,31 @@ var ExampleSecret2 = &corev1.Secret{ "key3": "example2:key3", }, } + +// ExampleSecret3 is an example Secret object for use within test suites +var ExampleSecret3 = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example3", + Namespace: "default", + Labels: labels, + }, + StringData: map[string]string{ + "key1": "example3:key1", + "key2": "example3:key2", + "key3": "example3:key3", + }, +} + +// ExampleSecret4 is an example Secret object for use within test suites +var ExampleSecret4 = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example4", + Namespace: "default", + Labels: labels, + }, + StringData: map[string]string{ + "key1": "example4:key1", + "key2": "example4:key2", + "key3": "example4:key3", + }, +}