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"
@@ -55,28 +56,29 @@ var (
 )
 
 var (
-	probeAddr               string
-	metricsAddr             string
-	enableStackdriver       bool
-	maxReconciles           int
-	enableLeaderElection    bool
-	leaderElectionId        string
-	noWebhooks              bool
-	debugLogs               bool
-	testLog                 bool
-	internalCert            bool
-	qps                     int
-	webhookServerPort       int
-	restartOnSecretRefresh  bool
-	unpropagatedAnnotations arrayArg
-	excludedNamespaces      arrayArg
-	managedNamespaceLabels  arrayArg
-	managedNamespaceAnnots  arrayArg
-	includedNamespacesRegex string
-	webhooksOnly            bool
-	enableHRQ               bool
-	hncNamespace            string
-	hrqSyncInterval         time.Duration
+	probeAddr                 string
+	metricsAddr               string
+	enableStackdriver         bool
+	maxReconciles             int
+	enableLeaderElection      bool
+	leaderElectionId          string
+	noWebhooks                bool
+	debugLogs                 bool
+	testLog                   bool
+	internalCert              bool
+	qps                       int
+	webhookServerPort         int
+	restartOnSecretRefresh    bool
+	unpropagatedAnnotations   arrayArg
+	excludedNamespaces        arrayArg
+	managedNamespaceLabels    arrayArg
+	managedNamespaceAnnots    arrayArg
+	excludeResourcesWithLabel arrayArg
+	includedNamespacesRegex   string
+	webhooksOnly              bool
+	enableHRQ                 bool
+	hncNamespace              string
+	hrqSyncInterval           time.Duration
 )
 
 // init preloads some global vars before main() starts. Since this is the top-level module, I'm not
@@ -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(&excludeResourcesWithLabel, "exclude-objects-with-label", "An annotation specified as key=val that, if present, will cause HNC to skip objects that match this label. May be specified multiple times, with each instance 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 excludeResourcesWithLabel {
+		keyVal := strings.Split(label, "=")
+		if len(keyVal) != 2 {
+			setupLog.Info(fmt.Sprintf("--exclude-objects-with-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("--exclude-objects-with-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.ExclusionByLabels = append(config.ExclusionByLabels, config.ExclusionByLabelsSpec{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,7 @@ spec:
         - "--excluded-namespace=kube-public"
         - "--excluded-namespace=hnc-system"
         - "--excluded-namespace=kube-node-lease"
+        - "--exclude-objects-with-label=cattle.io/creator=norman"
         ports:
         - containerPort: 9443
           name: webhook-server

docs/user-guide/concepts.md

@@ -378,7 +378,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
+  `--exclude-objects-with-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

@@ -956,3 +956,10 @@ 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.
+* `--exclude-objects-with-label`: present by default with label
+  `cattle.io/creator=norman` in order to exclude objects created by Rancher
+  (refer to [Concepts: built in exceptions](concepts.md#built-in-exceptions)
+  for more information).
+  This argument may be specified multiple times, with each parameter representing
+  a pair of key and value that represent a label that should exclude an object from
+  propagation. Must have the `key=val` format.

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
+
+// ExclusionByLabelsSpec 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 ExclusionByLabelsSpec struct {
+	Key   string
+	Value string
+}
+
+// ExclusionByLabels is a configuration slice that contains all ExclusionByLabelSpec labels that should
+// cause objects to be ignored from propagation.
+var ExclusionByLabels []ExclusionByLabelsSpec

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.ExclusionByLabels = append(config.ExclusionByLabels, config.ExclusionByLabelsSpec{Key: "cattle.io/creator", Value: "norman"})
+		config.ExclusionByLabels = append(config.ExclusionByLabels, config.ExclusionByLabelsSpec{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.ExclusionByLabels = append(config.ExclusionByLabels, config.ExclusionByLabelsSpec{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 first one should not propagate, everything else should
+		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.ExclusionByLabels {
 		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