adding new --exclude-resources-with-label manager CLI argument

Changed default exclusion by label mechanism to use this CLI argument instead of a hardcoded list of labels. Moved the default Rancher label exclusion to be a default CLI argument on the manager. Fixed a bug that caused excluded resources to be propagated with the propagate.hnc.x-k8s.io/all annotation.

Tested: made sure the current test for default exclusion fails after my
CLI argument change, changed the test to reflect the changes, test now passes as
expected. Wrote a test that verifies the propagate.hnc.x-k8s.io/all annotation bug
doesn't occur, which failed before the bugfix. Executed e2e tests and they pass.
Manually tested the manager by adding the new CLI argument and with that a labeled
resource was skipped, then removed the argument and it has propagated. Did the same
with two different annotations specified together as two CLI arguments, and resources that
had any of those labels were skipped as expected. Manually tried to cause an
excluded resource to propagate by adding the propagate.hnc.x-k8s.io/all
annotation, and it didn't propagate.

Author: dedri-github

cmd/manager/main.go

@@ -18,6 +18,7 @@ package main
 import (
 	"errors"
 	"flag"
+	"fmt"
 	"net/http"
 	"os"
 	"strings"
@@ -72,6 +73,7 @@ var (
 	excludedNamespaces      arrayArg
 	managedNamespaceLabels  arrayArg
 	managedNamespaceAnnots  arrayArg
+	nopropagationLabel      arrayArg
 	includedNamespacesRegex string
 	webhooksOnly            bool
 	enableHRQ               bool
@@ -156,10 +158,31 @@ func parseFlags() {
 	flag.BoolVar(&enableHRQ, "enable-hrq", false, "Enables hierarchical resource quotas")
 	flag.StringVar(&hncNamespace, "namespace", "hnc-system", "Namespace where hnc-manager and hnc resources deployed")
 	flag.DurationVar(&hrqSyncInterval, "hrq-sync-interval", 1*time.Minute, "Frequency to double-check that all HRQ usages are up-to-date (shouldn't be needed)")
+	flag.Var(&nopropagationLabel, "nopropagation-label", "A label specified as key=val that, if present, will cause HNC to skip objects that match this label. May be specified multiple times, with each key=value pair specifying one label. 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
+
+	// Assign the exclusion label args to the configuration
+	for _, label := range nopropagationLabel {
+		keyVal := strings.Split(label, "=")
+		if len(keyVal) != 2 {
+			setupLog.Info(fmt.Sprintf("--nopropagation-label must contain exactly one equal sign (=): %s", label))
+			os.Exit(1)
+		}
+		labelKey := keyVal[0]
+		labelValue := keyVal[1]
+
+		if len(labelKey) == 0 {
+			setupLog.Info(fmt.Sprintf("--nopropagation-label must not have an empty key for the label: %s", label))
+			os.Exit(1)
+		}
+
+		setupLog.Info(fmt.Sprintf("Will exclude objects that match label %s", label))
+		config.NoPropagationLabels = append(config.NoPropagationLabels, config.NoPropagationLabel{Key: labelKey, Value: labelValue})
+	}
+
 	config.SetNamespaces(includedNamespacesRegex, excludedNamespaces...)
 	if err := config.SetManagedMeta(managedNamespaceLabels, managedNamespaceAnnots); err != nil {
 		setupLog.Error(err, "Illegal flag values")

config/manager/manager.yaml

@@ -49,6 +49,9 @@ spec:
         - "--excluded-namespace=kube-public"
         - "--excluded-namespace=hnc-system"
         - "--excluded-namespace=kube-node-lease"
+        # Preserves the existing behavior of skipping Rancher objects.
+        # Please don't add any more nopropagation labels to the list of args.
+        - "--nopropagation-label=cattle.io/creator=norman"
         ports:
         - containerPort: 9443
           name: webhook-server

docs/user-guide/concepts.md

@@ -445,7 +445,10 @@ objects from being propagated by HNC.
   auto-created in new namespaces by Istio and Kubernetes respectively
 * Any objects with the label
   `cattle.io/creator:norman`, which are [inserted by Rancher to support
-  Projects](https://rancher.com/docs/rancher/v2.6/en/system-tools/#remove))
+  Projects](https://rancher.com/docs/rancher/v2.6/en/system-tools/#remove)).
+  Can be disabled by removing the default command line argument on the manager
+  `--nopropagation-label=cattle.io/creator=norman`. Refer to [How-to:
+  Modify command-line arguments](how-to.md#modify-command-line-arguments) for more details.
 * *HNC v1.1+:* Secrets with type `helm.sh/release.v1`, which is auto-created in
   the namespaces where their respective Helm releases are deployed to.
 

docs/user-guide/how-to.md

@@ -972,3 +972,12 @@ Interesting parameters include:
   load on your metrics database (through increased metric cardinality) and also
   by increasing how carefully you need to guard your metrics against
   unauthorized viewers.
+* `--nopropagation-label`: has the same effect as the `propagate.hnc.x-k8s.io/none`
+  annotation as specified in [limiting propagation](how-to.md#limit-the-propagation-of-an-object-to-descendant-namespaces),
+  but is useful when there's no control over what annotations the object has in order
+  to disable the propagation of that object. This argument may be specified multiple
+  times, with each parameter representing one `key=val` pair of a label that should
+  exclude an object from propagation.
+    * Rancher objects that have the label `cattle.io/creator=norman` are not propagated
+    by the default manifests (refer to [Concepts: built in exceptions](concepts.md#built-in-exceptions)
+    for more information).

internal/config/default_config.go

@@ -8,3 +8,14 @@ package config
 // This value is controlled by the --unpropagated-annotation command line, which may be set multiple
 // times.
 var UnpropagatedAnnotations []string
+
+// NoPropagationLabel specifies a label Key and Value which will cause an object to be excluded
+// from propagation if the object defines that label with this specific value.
+type NoPropagationLabel struct {
+	Key   string
+	Value string
+}
+
+// NoPropagationLabels is a configuration slice that contains all NoPropagationLabel labels that should
+// cause objects to be ignored from propagation.
+var NoPropagationLabels []NoPropagationLabel

internal/objects/reconciler_test.go

@@ -446,15 +446,20 @@ var _ = Describe("Basic propagation", func() {
 		Eventually(HasObject(ctx, "secrets", barName, "helm-secret")).Should(BeFalse())
 	})
 
-	It("should not propagate builtin exclusions by labels", func() {
+	It("should not propagate objects excluded by labels", func() {
 		SetParent(ctx, barName, fooName)
+		config.NoPropagationLabels = append(config.NoPropagationLabels, config.NoPropagationLabel{Key: "cattle.io/creator", Value: "norman"})
+		config.NoPropagationLabels = append(config.NoPropagationLabels, config.NoPropagationLabel{Key: "ignore-label", Value: "label"})
+
 		MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-blocked", map[string]string{"cattle.io/creator": "norman"})
+		MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-blocked-2", map[string]string{"ignore-label": "label"})
 		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())
+		Consistently(HasObject(ctx, "roles", barName, "role-with-labels-blocked-2")).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())
 
@@ -467,6 +472,18 @@ var _ = Describe("Basic propagation", func() {
 		Eventually(HasObject(ctx, "configmaps", barName, "cm-with-label-2")).Should(BeTrue())
 	})
 
+	It("should not propagate objects excluded by labels when using all annotation", func() {
+		SetParent(ctx, barName, fooName)
+		config.NoPropagationLabels = append(config.NoPropagationLabels, config.NoPropagationLabel{Key: "cattle.io/creator", Value: "norman"})
+
+		MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-blocked", map[string]string{"cattle.io/creator": "norman"})
+		UpdateObjectWithAnnotations(ctx, "roles", fooName, "role-with-labels-blocked", map[string]string{"propagate.hnc.x-k8s.io/all": "true"})
+		AddToHNCConfig(ctx, api.RBACGroup, api.RoleKind, api.Propagate)
+
+		// The object should not be propagated even though it uses the `all` annotation
+		Consistently(HasObject(ctx, "roles", barName, "role-with-labels-blocked")).Should(BeFalse())
+	})
+
 	It("should be removed if the hierarchy changes", func() {
 		SetParent(ctx, barName, fooName)
 		SetParent(ctx, bazName, barName)

internal/selectors/selectors.go

@@ -13,6 +13,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation"
 
 	api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
+	"sigs.k8s.io/hierarchical-namespaces/internal/config"
 )
 
 // ShouldPropagate returns true if the given object should be propagated
@@ -43,8 +44,10 @@ func ShouldPropagate(inst *unstructured.Unstructured, nsLabels labels.Set, mode
 	if none, err := GetNoneSelector(inst); err != nil || none {
 		return false, err
 	}
-	if all, err := GetAllSelector(inst); err != nil || all {
-		return true, err
+	if all, err := GetAllSelector(inst); err != nil {
+		return false, err
+	} else if all {
+		propIfNotExcluded = true
 	}
 	if excluded, err := isExcluded(inst); excluded {
 		return false, err
@@ -197,19 +200,6 @@ func GetAllSelector(inst *unstructured.Unstructured) (bool, error) {
 // 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"},
-}
-
 // A annotation as a key, value pair, used to exclude resources matching this annotation
 type ExclusionByAnnotationsSpec struct {
 	Key   string
@@ -246,7 +236,7 @@ func isExcluded(inst *unstructured.Unstructured) (bool, error) {
 	}
 
 	// exclusion by labels
-	for _, res := range exclusionByLabels {
+	for _, res := range config.NoPropagationLabels {
 		gotLabelValue, ok := inst.GetLabels()[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