Merge pull request #125 from nobbs/dev/per-object-filtering

Add per-object-level filtering based on labels (excluding Rancher-generated resources)

Author: k8s-ci-robot

docs/user-guide/concepts.md

@@ -361,6 +361,14 @@ adding annotations is not possible and HNC will by default exclude them.
 Similarly, Kubernetes ServiceAccount Secrets will also by default be excluded
 from propagation.
 
+In addition, propagation exclusions are also used for Rancher-managed Kubernetes
+clusters. Rancher uses a "project" concept that bundles namespaces and thus sets
+roles, rolebindings, etc. for all namespaces of a project. This leads to
+conflicts with HNC, so all resources created by Rancher (which are automatically
+labeled with `"cattle.io/creator": "norman"` by Rancher, cf. [their
+docs](https://rancher.com/docs/rancher/v2.6/en/system-tools/#remove)) are
+excluded from propagation.
+
 <a name="admin"/>
 
 ## Administration

internal/integtest/helpers.go

@@ -300,9 +300,9 @@ func MakeObject(ctx context.Context, resource string, nsName, name string) {
 	createdObjects = append(createdObjects, inst)
 }
 
-// MakeObjectWithAnnotation creates an empty object with annotation given kind in a specific
+// MakeObjectWithAnnotations creates an empty object with annotation given kind in a specific
 // namespace. The kind and its corresponding GVK should be included in the GVKs map.
-func MakeObjectWithAnnotation(ctx context.Context, resource string, nsName,
+func MakeObjectWithAnnotations(ctx context.Context, resource string, nsName,
 	name string, a map[string]string) {
 	inst := &unstructured.Unstructured{}
 	inst.SetGroupVersionKind(GVKs[resource])
@@ -313,9 +313,22 @@ func MakeObjectWithAnnotation(ctx context.Context, resource string, nsName,
 	createdObjects = append(createdObjects, inst)
 }
 
-// UpdateObjectWithAnnotation gets an object given it's kind, nsName and name, adds the annotation
+// MakeObjectWithLabels creates an empty object with label given kind in a specific
+// namespace. The kind and its corresponding GVK should be included in the GVKs map.
+func MakeObjectWithLabels(ctx context.Context, resource string, nsName,
+	name string, l map[string]string) {
+	inst := &unstructured.Unstructured{}
+	inst.SetGroupVersionKind(GVKs[resource])
+	inst.SetNamespace(nsName)
+	inst.SetName(name)
+	inst.SetLabels(l)
+	ExpectWithOffset(1, K8sClient.Create(ctx, inst)).Should(Succeed())
+	createdObjects = append(createdObjects, inst)
+}
+
+// UpdateObjectWithAnnotations gets an object given it's kind, nsName and name, adds the annotation
 // and updates this object
-func UpdateObjectWithAnnotation(ctx context.Context, resource string, nsName,
+func UpdateObjectWithAnnotations(ctx context.Context, resource string, nsName,
 	name string, a map[string]string) error {
 	nnm := types.NamespacedName{Namespace: nsName, Name: name}
 	inst := &unstructured.Unstructured{}

internal/objects/reconciler_test.go

@@ -142,7 +142,7 @@ var _ = Describe("Exceptions", func() {
 				tc.treeSelector = ReplaceStrings(tc.treeSelector, names)
 
 				// Create a Role with the selector and treeSelector annotation
-				MakeObjectWithAnnotation(ctx, api.RoleResource, names[p], "testrole", map[string]string{
+				MakeObjectWithAnnotations(ctx, api.RoleResource, names[p], "testrole", map[string]string{
 					api.AnnotationSelector:     tc.selector,
 					api.AnnotationTreeSelector: tc.treeSelector,
 					api.AnnotationNoneSelector: tc.noneSelector,
@@ -215,7 +215,7 @@ var _ = Describe("Exceptions", func() {
 				}
 
 				// update the role with the selector and treeSelector annotation
-				UpdateObjectWithAnnotation(ctx, api.RoleResource, names[p], "testrole", map[string]string{
+				UpdateObjectWithAnnotations(ctx, api.RoleResource, names[p], "testrole", map[string]string{
 					api.AnnotationSelector:     tc.selector,
 					api.AnnotationTreeSelector: tc.treeSelector,
 					api.AnnotationNoneSelector: tc.noneSelector,
@@ -232,7 +232,7 @@ var _ = Describe("Exceptions", func() {
 				}
 
 				// remove the annotation and verify that the object is back for every namespace
-				UpdateObjectWithAnnotation(ctx, api.RoleResource, names[p], "testrole", map[string]string{})
+				UpdateObjectWithAnnotations(ctx, api.RoleResource, names[p], "testrole", map[string]string{})
 				for _, ns := range names {
 					Eventually(HasObject(ctx, api.RoleResource, ns, "testrole")).Should(BeTrue(), "When propagating testrole to %s", ns)
 				}
@@ -304,7 +304,7 @@ var _ = Describe("Exceptions", func() {
 				Eventually(HasObject(ctx, api.RoleResource, names[nolabelchild], "testrole")).Should(BeTrue(), "When propagating testrole to %s", names[nolabelchild])
 				// Add `select` exception annotation with propagate label and verify the
 				// object is only propagated to children with the label.
-				UpdateObjectWithAnnotation(ctx, api.RoleResource, names[p], "testrole", map[string]string{
+				UpdateObjectWithAnnotations(ctx, api.RoleResource, names[p], "testrole", map[string]string{
 					api.AnnotationSelector: label,
 				})
 				Eventually(HasObject(ctx, api.RoleResource, names[nolabelchild], "testrole")).Should(BeFalse(), "When propagating testrole to %s", names[nolabelchild])
@@ -398,7 +398,7 @@ var _ = Describe("Basic propagation", func() {
 		Expect(ObjectInheritedFrom(ctx, "configmaps", barName, "foo-config")).Should(Equal(fooName))
 	})
 
-	It("should not propagate builtin exclusions", func() {
+	It("should not propagate builtin exclusions by name", func() {
 		SetParent(ctx, barName, fooName)
 		MakeObject(ctx, "configmaps", fooName, "istio-ca-root-cert")
 		MakeObject(ctx, "configmaps", fooName, "kube-root-ca.crt")
@@ -411,6 +411,27 @@ var _ = Describe("Basic propagation", func() {
 		Eventually(HasObject(ctx, "configmaps", barName, "kube-root-ca.crt")).Should(BeFalse())
 	})
 
+	It("should not propagate builtin exclusions by labels", func() {
+		SetParent(ctx, barName, fooName)
+		MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-blocked", map[string]string{"cattle.io/creator": "norman"})
+		MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-wrong-value", map[string]string{"cattle.io/creator": "testme"})
+		MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-something", map[string]string{"app": "hello-world"})
+		AddToHNCConfig(ctx, api.RBACGroup, api.RoleKind, api.Propagate)
+
+		// the first one should not propagate, everything else should
+		Consistently(HasObject(ctx, "roles", barName, "role-with-labels-blocked")).Should(BeFalse())
+		Eventually(HasObject(ctx, "roles", barName, "role-with-labels-wrong-value")).Should(BeTrue())
+		Eventually(HasObject(ctx, "roles", barName, "role-with-labels-something")).Should(BeTrue())
+
+		// lets try the same with config maps, they are also filtered and should not propagate
+		MakeObjectWithLabels(ctx, "configmaps", fooName, "cm-with-label-1", map[string]string{"cattle.io/creator": "norman"})
+		MakeObjectWithLabels(ctx, "configmaps", fooName, "cm-with-label-2", map[string]string{"app": "hello-world"})
+		AddToHNCConfig(ctx, "", "configmaps", api.Propagate)
+
+		Consistently(HasObject(ctx, "configmaps", barName, "cm-with-label-1")).Should(BeFalse())
+		Eventually(HasObject(ctx, "configmaps", barName, "cm-with-label-2")).Should(BeTrue())
+	})
+
 	It("should be removed if the hierarchy changes", func() {
 		SetParent(ctx, barName, fooName)
 		SetParent(ctx, bazName, barName)
@@ -658,7 +679,7 @@ var _ = Describe("Basic propagation", func() {
 
 	It("should avoid propagating banned annotations", func() {
 		SetParent(ctx, barName, fooName)
-		MakeObjectWithAnnotation(ctx, "roles", fooName, "foo-annot-role", map[string]string{
+		MakeObjectWithAnnotations(ctx, "roles", fooName, "foo-annot-role", map[string]string{
 			"annot-a": "value-a",
 			"annot-b": "value-b",
 		})
@@ -682,7 +703,7 @@ var _ = Describe("Basic propagation", func() {
 
 		// Tell the HNC config not to propagate annot-a and verify that this time, it's not annotated
 		config.UnpropagatedAnnotations = []string{"annot-a"}
-		MakeObjectWithAnnotation(ctx, "roles", fooName, "foo-annot-role", map[string]string{
+		MakeObjectWithAnnotations(ctx, "roles", fooName, "foo-annot-role", map[string]string{
 			"annot-a": "value-a",
 			"annot-b": "value-b",
 		})

internal/selectors/selectors.go

@@ -158,19 +158,46 @@ func GetNoneSelector(inst *unstructured.Unstructured) (bool, error) {
 	return noneSelector, nil
 }
 
-// cmExclusions are known (istio and kube-root) CA configmap which are excluded from propagation
-var cmExclusions = []string{"istio-ca-root-cert", "kube-root-ca.crt"}
+// cmExclusionsByName are known (istio and kube-root) CA configmap which are excluded from propagation
+var cmExclusionsByName = []string{"istio-ca-root-cert", "kube-root-ca.crt"}
+
+// A label as a key, value pair, used to exclude resources matching this label (both key and value).
+type ExclusionByLabelsSpec struct {
+	Key   string
+	Value string
+}
+
+// ExclusionByLabelsSpec are known label key-value pairs which are excluded from propagation. Right
+// now only used to exclude resources created by Rancher, see "System Tools > Remove"
+// (https://rancher.com/docs/rancher/v2.6/en/system-tools/#remove)
+var exclusionByLabels = []ExclusionByLabelsSpec{
+	{Key: "cattle.io/creator", Value: "norman"},
+}
 
 // isExcluded returns true to indicate that this object is excluded from being propagated
 func isExcluded(inst *unstructured.Unstructured) (bool, error) {
 	name := inst.GetName()
 	kind := inst.GetKind()
 	group := inst.GroupVersionKind().Group
+	labels := inst.GetLabels()
 
-	for _, excludedResourceName := range cmExclusions {
+	// exclusion by name
+	for _, excludedResourceName := range cmExclusionsByName {
 		if group == "" && kind == "ConfigMap" && name == excludedResourceName {
 			return true, nil
 		}
 	}
+
+	// exclusion by labels
+	for _, res := range exclusionByLabels {
+		gotLabelValue, ok := labels[res.Key]
+		// check for presence has to be explicit, as empty label values are allowed and a
+		// nonexisting key in the `labels` map will also return an empty string ("") - potentially
+		// causing false matches if `ok` is not checked
+		if ok && gotLabelValue == res.Value {
+			return true, nil
+		}
+	}
+
 	return false, nil
 }