From 48df98451954410686e7c6568116574ceee043db Mon Sep 17 00:00:00 2001 From: Adrian Ludwin Date: Wed, 12 Jan 2022 00:56:14 -0500 Subject: [PATCH] Initial implementation of managed labels See #47. This is the initial implementation of managed labels and annotations - that is, the ability to set a label (or annotation) in a HierarchyConfiguration object, and have that label (...) propagated to all descendants, similar to the way objects are propagated. As with objects, only allowlisted labels are propagated, as defined by the command line option '--managed-namespace-[label|annotation]'. Still to come: validator support, better conditions for conflicts, better testing for external namespaces, better testing for more regexes, documentation, end-to-end tests. Tested: see new integ tests. --- api/v1alpha2/hierarchy_types.go | 38 ++++- api/v1alpha2/zz_generated.deepcopy.go | 27 +++- cmd/manager/main.go | 12 +- .../hnc.x-k8s.io_hierarchyconfigurations.yaml | 48 ++++++ internal/config/namespace.go | 62 +++++++ internal/forest/namespace.go | 8 + internal/hierarchyconfig/reconciler.go | 152 +++++++++++++++--- internal/hierarchyconfig/reconciler_test.go | 93 +++++++++++ internal/integtest/helpers.go | 8 + 9 files changed, 416 insertions(+), 32 deletions(-) diff --git a/api/v1alpha2/hierarchy_types.go b/api/v1alpha2/hierarchy_types.go index 78045d449..84baab9bd 100644 --- a/api/v1alpha2/hierarchy_types.go +++ b/api/v1alpha2/hierarchy_types.go @@ -55,12 +55,14 @@ const ( ConditionBadConfiguration string = "BadConfiguration" // Condition reasons. - ReasonAncestor string = "AncestorHaltActivities" - ReasonDeletingCRD string = "DeletingCRD" - ReasonInCycle string = "InCycle" - ReasonParentMissing string = "ParentMissing" - ReasonIllegalParent string = "IllegalParent" - ReasonAnchorMissing string = "SubnamespaceAnchorMissing" + ReasonAncestor string = "AncestorHaltActivities" + ReasonDeletingCRD string = "DeletingCRD" + ReasonInCycle string = "InCycle" + ReasonParentMissing string = "ParentMissing" + ReasonIllegalParent string = "IllegalParent" + ReasonAnchorMissing string = "SubnamespaceAnchorMissing" + ReasonIllegalManagedLabel string = "IllegalManagedLabel" + ReasonIllegalManagedAnnotation string = "IllegalManagedAnnotation" ) // AllConditions have all the conditions by type and reason. Please keep this @@ -124,6 +126,18 @@ type HierarchyConfigurationSpec struct { // AllowCascadingDeletion indicates if the subnamespaces of this namespace are // allowed to cascading delete. AllowCascadingDeletion bool `json:"allowCascadingDeletion,omitempty"` + + // Lables is a list of labels and values to apply to the current namespace and all of its + // descendants. All label keys must match a regex specified on the command line by + // --managed-namespace-label. A namespace cannot have a KVP that conflicts with one of its + // ancestors. + Labels []MetaKVP `json:"labels,omitempty"` + + // Annotations is a list of annotations and values to apply to the current namespace and all of + // its descendants. All annotation keys must match a regex specified on the command line by + // --managed-namespace-annotation. A namespace cannot have a KVP that conflicts with one of its + // ancestors. + Annotations []MetaKVP `json:"annotations,omitempty"` } // HierarchyStatus defines the observed state of Hierarchy @@ -147,6 +161,18 @@ type HierarchyConfigurationList struct { Items []HierarchyConfiguration `json:"items"` } +// MetaKVP represents a label or annotation +type MetaKVP struct { + // Key is the name of the label or annotation. It must conform to the normal rules for Kubernetes + // label/annotation keys. + Key string `json:"key"` + + // Value is the value of the label or annotation. It must confirm to the normal rules for + // Kubernetes label or annoation values, which are far more restrictive for labels than for + // anntations. + Value string `json:"value"` +} + // metav1.Condition is introduced in k8s.io/apimachinery v0.20.0-alpha.1 and we // don't want to take a dependency on it yet, thus we copied the below struct from // https://github.com/kubernetes/apimachinery/blob/master/pkg/apis/meta/v1/types.go: diff --git a/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go index c246cfa5c..6e0a13de0 100644 --- a/api/v1alpha2/zz_generated.deepcopy.go +++ b/api/v1alpha2/zz_generated.deepcopy.go @@ -153,7 +153,7 @@ func (in *HierarchyConfiguration) DeepCopyInto(out *HierarchyConfiguration) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -210,6 +210,16 @@ func (in *HierarchyConfigurationList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HierarchyConfigurationSpec) DeepCopyInto(out *HierarchyConfigurationSpec) { *out = *in + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make([]MetaKVP, len(*in)) + copy(*out, *in) + } + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = make([]MetaKVP, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HierarchyConfigurationSpec. @@ -249,6 +259,21 @@ func (in *HierarchyConfigurationStatus) DeepCopy() *HierarchyConfigurationStatus return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MetaKVP) DeepCopyInto(out *MetaKVP) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MetaKVP. +func (in *MetaKVP) DeepCopy() *MetaKVP { + if in == nil { + return nil + } + out := new(MetaKVP) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResourceSpec) DeepCopyInto(out *ResourceSpec) { *out = *in diff --git a/cmd/manager/main.go b/cmd/manager/main.go index d88dae951..e2d152df0 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -65,6 +65,8 @@ var ( restartOnSecretRefresh bool unpropagatedAnnotations arrayArg excludedNamespaces arrayArg + managedNamespaceLabels arrayArg + managedNamespaceAnnots arrayArg includedNamespacesRegex string ) @@ -97,13 +99,19 @@ func main() { flag.IntVar(&webhookServerPort, "webhook-server-port", 443, "The port that the webhook server serves at.") flag.Var(&unpropagatedAnnotations, "unpropagated-annotation", "An annotation that, if present, will be stripped out of any propagated copies of an object. May be specified multiple times, with each instance specifying one annotation. See the user guide for more information.") flag.Var(&excludedNamespaces, "excluded-namespace", "A namespace that, if present, will be excluded from HNC management. May be specified multiple times, with each instance specifying one namespace. See the user guide for more information.") - flag.StringVar(&includedNamespacesRegex, "included-namespace-regex", ".*", "Namespace regular expression. Namespaces that match this regexp will be included and handle by HNC. As it is a regex, this parameter cannot be specified multiple times. Implicit wrapping of the expression \"^...$\" is done here") + flag.StringVar(&includedNamespacesRegex, "included-namespace-regex", ".*", "Namespace regular expression. Namespaces that match this regexp will be included and handle by HNC. The regex is implicitly wrapped by \"^...$\" and may only be specified once.") flag.BoolVar(&restartOnSecretRefresh, "cert-restart-on-secret-refresh", false, "Kills the process when secrets are refreshed so that the pod can be restarted (secrets take up to 60s to be updated by running pods)") + flag.Var(&managedNamespaceLabels, "managed-namespace-label", "A regex indicating the labels on namespaces that are managed by HNC. These labels may only be set via the HierarchyConfiguration object. All regexes are implictly wrapped by \"^...$\". This argument can be specified multiple times. See the user guide for more information.") + flag.Var(&managedNamespaceAnnots, "managed-namespace-annotation", "A regex indicating the annotations on namespaces that are managed by HNC. These annotations may only be set via the HierarchyConfiguration object. All regexes are implictly wrapped by \"^...$\". This argument can be specified multiple times. See the user guide for more information.") flag.Parse() + // Assign the array args to the configuration variables after the args are parsed. config.UnpropagatedAnnotations = unpropagatedAnnotations - config.SetNamespaces(includedNamespacesRegex, excludedNamespaces...) + if err := config.SetManagedMeta(managedNamespaceLabels, managedNamespaceAnnots); err != nil { + setupLog.Error(err, "Illegal flag values") + os.Exit(1) + } // Enable OpenCensus exporters to export metrics // to Stackdriver Monitoring. diff --git a/config/crd/bases/hnc.x-k8s.io_hierarchyconfigurations.yaml b/config/crd/bases/hnc.x-k8s.io_hierarchyconfigurations.yaml index 57e8d0b96..355fa63b1 100644 --- a/config/crd/bases/hnc.x-k8s.io_hierarchyconfigurations.yaml +++ b/config/crd/bases/hnc.x-k8s.io_hierarchyconfigurations.yaml @@ -46,6 +46,54 @@ spec: description: AllowCascadingDeletion indicates if the subnamespaces of this namespace are allowed to cascading delete. type: boolean + annotations: + description: Annotations is a list of annotations and values to apply + to the current namespace and all of its descendants. All annotation + keys must match a regex specified on the command line by --managed-namespace-annotation. + A namespace cannot have a KVP that conflicts with one of its ancestors. + items: + description: MetaKVP represents a label or annotation + properties: + key: + description: Key is the name of the label or annotation. It + must conform to the normal rules for Kubernetes label/annotation + keys. + type: string + value: + description: Value is the value of the label or annotation. + It must confirm to the normal rules for Kubernetes label or + annoation values, which are far more restrictive for labels + than for anntations. + type: string + required: + - key + - value + type: object + type: array + labels: + description: Lables is a list of labels and values to apply to the + current namespace and all of its descendants. All label keys must + match a regex specified on the command line by --managed-namespace-label. + A namespace cannot have a KVP that conflicts with one of its ancestors. + items: + description: MetaKVP represents a label or annotation + properties: + key: + description: Key is the name of the label or annotation. It + must conform to the normal rules for Kubernetes label/annotation + keys. + type: string + value: + description: Value is the value of the label or annotation. + It must confirm to the normal rules for Kubernetes label or + annoation values, which are far more restrictive for labels + than for anntations. + type: string + required: + - key + - value + type: object + type: array parent: description: Parent indicates the parent of this namespace, if any. type: string diff --git a/internal/config/namespace.go b/internal/config/namespace.go index b532b53d4..37669cb3c 100644 --- a/internal/config/namespace.go +++ b/internal/config/namespace.go @@ -1,7 +1,10 @@ package config import ( + "fmt" "regexp" + + api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2" ) var ( @@ -18,6 +21,16 @@ var ( // includedNamespacesStr is the original pattern of the regex. It must // only be used to generate error messages. includedNamespacesStr string + + // managedLabels is the list of compiled regexes for managed labels. Any label in this list is + // removed from all managed namespaces unless specifically specified by the HC of the namespace or + // one of its ancestors. + managedLabels []*regexp.Regexp + + // managedAnnotations is the list of compiled regexes for managed annotations. Any annotations in + // this list is removed from all managed namespaces unless specifically specified by the HC of the + // namespace or one of its ancestors. + managedAnnotations []*regexp.Regexp ) func SetNamespaces(regex string, excluded ...string) { @@ -63,3 +76,52 @@ func WhyUnmanaged(nm string) string { func IsManagedNamespace(nm string) bool { return WhyUnmanaged(nm) == "" } + +// SetManagedMeta sets the regexes for the managed namespace labels and annotations. The function +// ensures that all strings are valid regexes, and that they do not attempt to select for HNC +// metadata. +func SetManagedMeta(labels, annots []string) error { + if err := setManagedMeta(labels, "--managed-namespace-label", &managedLabels); err != nil { + return err + } + if err := setManagedMeta(annots, "--managed-namespace-annotation", &managedAnnotations); err != nil { + return err + } + return nil +} + +func setManagedMeta(patterns []string, option string, regexes *[]*regexp.Regexp) error { + // Reset (useful for unit tests) + *regexes = nil + + // Validate regexes + for _, p := range patterns { + r, err := regexp.Compile("^" + p + "$") + if err != nil { + return fmt.Errorf("Illegal value for %s %q: %w", option, p, err) + } + if r.MatchString(api.MetaGroup) { + return fmt.Errorf("Illegal value for %s %q: cannot specify a pattern that matches %q", option, p, api.MetaGroup) + } + *regexes = append(*regexes, r) + } + return nil +} + +func IsManagedLabel(k string) bool { + for _, regex := range managedLabels { + if regex.MatchString(k) { + return true + } + } + return false +} + +func IsManagedAnnotation(k string) bool { + for _, regex := range managedAnnotations { + if regex.MatchString(k) { + return true + } + } + return false +} diff --git a/internal/forest/namespace.go b/internal/forest/namespace.go index 47b82b9a4..0335aa224 100644 --- a/internal/forest/namespace.go +++ b/internal/forest/namespace.go @@ -32,6 +32,14 @@ type Namespace struct { // and to store the tree labels of external namespaces. labels map[string]string + // ManagedLabels are all managed labels explicitly set on this namespace (i.e., excluding anything + // set by ancestors). + ManagedLabels map[string]string + + // ManagedAnnotations are all managed annotations explicitly set on this namespace (i.e., + // excluding anything set by ancestors). + ManagedAnnotations map[string]string + // sourceObjects store the objects created by users, identified by GVK and name. // It serves as the source of truth for object controllers to propagate objects. sourceObjects objects diff --git a/internal/hierarchyconfig/reconciler.go b/internal/hierarchyconfig/reconciler.go index 8bb7877ae..ba9d50f68 100644 --- a/internal/hierarchyconfig/reconciler.go +++ b/internal/hierarchyconfig/reconciler.go @@ -311,7 +311,8 @@ func (r *Reconciler) syncWithForest(log logr.Logger, nsInst *corev1.Namespace, i initial := r.markExisting(log, ns) // Sync labels and annotations, now that the structure's been updated. - nsCustomerLabelUpdated := r.syncTreeLabels(log, nsInst, ns) + updatedLabels := r.syncLabels(log, inst, nsInst, ns) + r.syncAnnotations(log, inst, nsInst, ns) // Sync other spec and spec-like info r.syncAnchors(log, ns, anms) @@ -325,7 +326,7 @@ func (r *Reconciler) syncWithForest(log logr.Logger, nsInst *corev1.Namespace, i r.syncConditions(log, inst, ns, wasHalted) r.HNCConfigReconciler.Enqueue("namespace reconciled") - return initial || nsCustomerLabelUpdated + return initial || updatedLabels } // syncExternalNamespace sets external tree labels to the namespace in the forest @@ -488,33 +489,71 @@ func (r *Reconciler) syncAnchors(log logr.Logger, ns *forest.Namespace, anms []s } } -// Sync namespace tree labels. Return true if the labels are updated. -func (r *Reconciler) syncTreeLabels(log logr.Logger, nsInst *corev1.Namespace, ns *forest.Namespace) bool { - if ns.IsExternal() { - metadata.SetLabel(nsInst, nsInst.Name+api.LabelTreeDepthSuffix, "0") - - // Set the labels so we can retrieve the external tree labels in the future if needed - ns.SetLabels(nsInst.Labels) - return false +// Sync namespace managed and tree labels. Return true if the labels are updated, so we know to +// re-sync all objects that could be affected by exclusions. +func (r *Reconciler) syncLabels(log logr.Logger, inst *api.HierarchyConfiguration, nsInst *corev1.Namespace, ns *forest.Namespace) bool { + // Get a list of all managed labels from the config object. + managed := map[string]string{} + for _, kvp := range inst.Spec.Labels { + if !config.IsManagedLabel(kvp.Key) { + log.Info("Illegal managed label", "key", kvp.Key) + ns.SetCondition(api.ConditionBadConfiguration, api.ReasonIllegalManagedLabel, "Not a legal managed label (set via --managed-namespace-label): "+kvp.Key) + continue + } + managed[kvp.Key] = kvp.Value } - // Remove all existing depth labels. - for k := range nsInst.Labels { - if strings.HasSuffix(k, api.LabelTreeDepthSuffix) { + // External namespaces can also have both managed and tree labels set directly. All other + // namespaces must have these removed now. + for k, v := range nsInst.Labels { + if config.IsManagedLabel(k) { + if ns.IsExternal() { + managed[k] = v + } else { + delete(nsInst.Labels, k) + } + } + if !ns.IsExternal() && strings.HasSuffix(k, api.LabelTreeDepthSuffix) { delete(nsInst.Labels, k) } } + if len(managed) == 0 { + // Don't cause unnecessary updates from the default value + managed = nil + } + + // Record all managed labels in the forest. If they've changed, we need to enqueue all descendants + // to propagate the changes. + if !reflect.DeepEqual(ns.ManagedLabels, managed) { + ns.ManagedLabels = managed + log.Info("Updated managed label", "newLabels", managed) + r.enqueueAffected(log, "managed labels have changed", ns.DescendantNames()...) + } + + // For external namespaces, we're pretty much done since HNC mainly doesn't manage the metadata of + // external namespaces. Just make sure the zero-depth tree label is set if it wasn't already, and + // store all its labels so the tree labels can be retrieved later. + if ns.IsExternal() { + metadata.SetLabel(nsInst, nsInst.Name+api.LabelTreeDepthSuffix, "0") + ns.SetLabels(nsInst.Labels) + + // There are no propagated objects in external namespaces so we don't need to notify the caller + // that any propagation-relevant labels have changed. + return false + } - // Look for all ancestors. Stop as soon as we find a namespaces that has a critical condition in - // the forest (note that AncestorHaltActivities is never included in the forest). This should handle orphans - // and cycles. + // Set all managed and tree labels, starting from the current namespace and going up through the + // hierarchy. curNS := ns depth := 0 for curNS != nil { - l := curNS.Name() + api.LabelTreeDepthSuffix - metadata.SetLabel(nsInst, l, strconv.Itoa(depth)) - if curNS.IsHalted() { - break + // Set the tree label from this layer of hierarchy + metadata.SetLabel(nsInst, curNS.Name()+api.LabelTreeDepthSuffix, strconv.Itoa(depth)) + + // Add any managed labels. TODO: add conditions for conflicts. + for k, v := range curNS.ManagedLabels { + log.V(1).Info("Setting managed label", "from", curNS.Name(), "key", k, "value", v) + metadata.SetLabel(nsInst, k, v) } // If the root is an external namespace, add all its external tree labels too. @@ -524,21 +563,88 @@ func (r *Reconciler) syncTreeLabels(log logr.Logger, nsInst *corev1.Namespace, n for k, v := range curNS.GetTreeLabels() { metadata.SetLabel(nsInst, k, strconv.Itoa(depth+v)) } + + // Note that it's impossible to have an external namespace as a non-root (enforced elsewhere) + // so technically we don't need to break out of the loop here. But I find it cleaner. break } + // Stop if this namespace is halted, which could indicate a cycle or orphan. + if curNS.IsHalted() { + break + } curNS = curNS.Parent() depth++ } - // Update the labels in the forest so that we can quickly access the labels and - // compare if they match the given selector + + // Update the labels in the forest so that they can be used in object propagation. If they've + // changed, return true so that all propagated objects in this namespace can be compared to its + // new labels. if ns.SetLabels(nsInst.Labels) { - log.Info("Namespace managed and tree labels have been updated") + log.Info("Namespace's managed and/or tree labels have been updated") return true } return false } +// Sync namespace managed annotations. This is mainly a simplified version of syncLabels with all +// the tree stuff taken out. +func (r *Reconciler) syncAnnotations(log logr.Logger, inst *api.HierarchyConfiguration, nsInst *corev1.Namespace, ns *forest.Namespace) { + // Get a list of all managed annotations from the config object. + managed := map[string]string{} + for _, kvp := range inst.Spec.Annotations { + if !config.IsManagedAnnotation(kvp.Key) { + log.Info("Illegal managed annotation", "key", kvp.Key) + ns.SetCondition(api.ConditionBadConfiguration, api.ReasonIllegalManagedAnnotation, "Not a legal managed annotation (set via --managed-namespace-annotation): "+kvp.Key) + continue + } + managed[kvp.Key] = kvp.Value + } + + // External namespaces can also have managed annotations set directly. All other namespaces must + // have these removed now. + for k, v := range nsInst.Annotations { + if config.IsManagedAnnotation(k) { + if ns.IsExternal() { + managed[k] = v + } else { + delete(nsInst.Annotations, k) + } + } + } + + // Record all managed annotations in the forest. If they've changed, we need to enqueue all + // descendants to propagate the changes. + if !reflect.DeepEqual(ns.ManagedAnnotations, managed) { + ns.ManagedAnnotations = managed + r.enqueueAffected(log, "managed annotations have changed", ns.DescendantNames()...) + } + + // For external namespaces, we're done since HNC mainly doesn't manage the metadata of + // external namespaces. + if ns.IsExternal() { + return + } + + // Set all managed annotations, starting from the current namespace and going up through the + // hierarchy. + curNS := ns + depth := 0 + for curNS != nil { + // Add any managed annotations. TODO: add conditions for conflicts. + for k, v := range curNS.ManagedAnnotations { + metadata.SetAnnotation(nsInst, k, v) + } + + // Stop if this namespace is halted, which could indicate a cycle or orphan. + if curNS.IsHalted() { + break + } + curNS = curNS.Parent() + depth++ + } +} + func (r *Reconciler) syncConditions(log logr.Logger, inst *api.HierarchyConfiguration, ns *forest.Namespace, wasHalted bool) { // If the halted status has changed, notify if ns.IsHalted() != wasHalted { diff --git a/internal/hierarchyconfig/reconciler_test.go b/internal/hierarchyconfig/reconciler_test.go index 7b2cd3260..b503b8ae1 100644 --- a/internal/hierarchyconfig/reconciler_test.go +++ b/internal/hierarchyconfig/reconciler_test.go @@ -32,6 +32,7 @@ var _ = Describe("Hierarchy", func() { fooName = CreateNS(ctx, "foo") barName = CreateNS(ctx, "bar") config.SetNamespaces("") + config.SetManagedMeta(nil, nil) }) It("should set a child on the parent", func() { @@ -405,4 +406,96 @@ var _ = Describe("Hierarchy", func() { // Verify the label is eventually updated to have the right value. Eventually(GetLabel(ctx, barName, api.LabelIncludedNamespace)).Should(Equal("true")) }) + + It("should propagate managed labels and not unmanaged labels", func() { + config.SetManagedMeta([]string{"legal-.*"}, nil) + + // Set the managed label and confirm that it only exists on one namespace + fooHier := NewHierarchy(fooName) + fooHier.Spec.Labels = []api.MetaKVP{ + {Key: "legal-label", Value: "val-1"}, + {Key: "unpropagated-label", Value: "not-allowed"}, + } + UpdateHierarchy(ctx, fooHier) + Eventually(GetLabel(ctx, fooName, "legal-label")).Should(Equal("val-1")) + Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal("")) + + // Add 'bar' as a child and verify the right labels are propagated + barHier := NewHierarchy(barName) + barHier.Spec.Parent = fooName + UpdateHierarchy(ctx, barHier) + Eventually(HasChild(ctx, fooName, barName)).Should(Equal(true)) + Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal("val-1")) + + // Verify that the bad label isn't propagated and that a condition is set + Eventually(GetLabel(ctx, fooName, "unpropagated-label")).Should(Equal("")) + Eventually(GetLabel(ctx, barName, "unpropagated-label")).Should(Equal("")) + Eventually(HasCondition(ctx, fooName, api.ConditionBadConfiguration, api.ReasonIllegalManagedLabel)).Should(Equal(true)) + + // Remove the bad config and verify that the condition is removed + fooHier = GetHierarchy(ctx, fooName) + fooHier.Spec.Labels = fooHier.Spec.Labels[0:1] + UpdateHierarchy(ctx, fooHier) + Eventually(HasCondition(ctx, fooName, api.ConditionBadConfiguration, api.ReasonIllegalManagedLabel)).Should(Equal(false)) + + // Change the value of the label and verify that it's propagated + fooHier = GetHierarchy(ctx, fooName) + fooHier.Spec.Labels[0].Value = "second-value" + UpdateHierarchy(ctx, fooHier) + Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal("second-value")) + + // Remove 'bar' as a child and verify that the label is removed + barHier = GetHierarchy(ctx, barName) + barHier.Spec.Parent = "" + UpdateHierarchy(ctx, barHier) + Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal("")) + + // TODO: test external namespace, multiple regexes, etc + }) + + It("should propagate managed annotations and not unmanaged annotations", func() { + config.SetManagedMeta(nil, []string{"legal-.*"}) + + // Set the managed annotation and confirm that it only exists on one namespace + fooHier := NewHierarchy(fooName) + fooHier.Spec.Annotations = []api.MetaKVP{ + {Key: "legal-annotation", Value: "val-1"}, + {Key: "unpropagated-annotation", Value: "not-allowed"}, + } + UpdateHierarchy(ctx, fooHier) + Eventually(GetAnnotation(ctx, fooName, "legal-annotation")).Should(Equal("val-1")) + Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("")) + + // Add 'bar' as a child and verify the right annotations are propagated + barHier := NewHierarchy(barName) + barHier.Spec.Parent = fooName + UpdateHierarchy(ctx, barHier) + Eventually(HasChild(ctx, fooName, barName)).Should(Equal(true)) + Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("val-1")) + + // Verify that the bad annotation isn't propagated and that a condition is set + Eventually(GetAnnotation(ctx, fooName, "unpropagated-annotation")).Should(Equal("")) + Eventually(GetAnnotation(ctx, barName, "unpropagated-annotation")).Should(Equal("")) + Eventually(HasCondition(ctx, fooName, api.ConditionBadConfiguration, api.ReasonIllegalManagedAnnotation)).Should(Equal(true)) + + // Remove the bad config and verify that the condition is removed + fooHier = GetHierarchy(ctx, fooName) + fooHier.Spec.Annotations = fooHier.Spec.Annotations[0:1] + UpdateHierarchy(ctx, fooHier) + Eventually(HasCondition(ctx, fooName, api.ConditionBadConfiguration, api.ReasonIllegalManagedAnnotation)).Should(Equal(false)) + + // Change the value of the annotation and verify that it's propagated + fooHier = GetHierarchy(ctx, fooName) + fooHier.Spec.Annotations[0].Value = "second-value" + UpdateHierarchy(ctx, fooHier) + Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("second-value")) + + // Remove 'bar' as a child and verify that the annotation is removed + barHier = GetHierarchy(ctx, barName) + barHier.Spec.Parent = "" + UpdateHierarchy(ctx, barHier) + Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("")) + + // TODO: test external namespaces, multiple regexes, etc + }) }) diff --git a/internal/integtest/helpers.go b/internal/integtest/helpers.go index e7788f9e3..7c0f9bcf7 100644 --- a/internal/integtest/helpers.go +++ b/internal/integtest/helpers.go @@ -86,6 +86,14 @@ func GetLabel(ctx context.Context, from, label string) func() string { } } +func GetAnnotation(ctx context.Context, from, key string) func() string { + return func() string { + ns := GetNamespace(ctx, from) + val, _ := ns.GetAnnotations()[key] + return val + } +} + func HasChild(ctx context.Context, nm, cnm string) func() bool { return func() bool { children := GetHierarchy(ctx, nm).Status.Children