Enhance namespace validation and retrieval for controller scope (#132)

This patch introduces validation for user-provided namespaces, ensuring
RFC 1123 compliance and validiy for defining the controller's scope. The
ACK controllers will now validate the comma-seperated list of namespaces
and expose a method for quering and reusing these validate namespaces.

Signed-off-by: Amine Hilaly <[email protected]>

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

pkg/config/config.go

@@ -27,6 +27,7 @@ import (
 	"github.com/jaypipes/envutil"
 	flag "github.com/spf13/pflag"
 	"go.uber.org/zap/zapcore"
+	apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/klog/v2"
 	ctrlrt "sigs.k8s.io/controller-runtime"
@@ -174,8 +175,8 @@ func (cfg *Config) BindFlags() {
 	flag.StringVar(
 		&cfg.WatchNamespace, flagWatchNamespace,
 		"",
-		"Specific namespace the service controller will watch for object creation from CRD. "+
-			" By default it will listen to all namespaces",
+		"A comma-separated list of valid RFC-1123 namespace names to watch for custom resource events. "+
+			"If unspecified, the controller watches for events in all namespaces.",
 	)
 	flag.Var(
 		&cfg.DeletionPolicy, flagDeletionPolicy,
@@ -366,3 +367,41 @@ func parseReconcileFlagArgument(flagArgument string) (string, int, error) {
 	}
 	return elements[0], resyncSeconds, nil
 }
+
+// GetWatchNamespaces returns a slice of namespaces to watch for custom resource events.
+// If the watchNamespace flag is empty, the function returns nil, which means that the
+// controller will watch for events in all namespaces.
+func (c *Config) GetWatchNamespaces() ([]string, error) {
+	return parseWatchNamespaceString(c.WatchNamespace)
+}
+
+// parseWatchNamespaceString parses the watchNamespace flag and returns a slice of namespaces
+// to watch. The input string is expected to be a comma-separated list of namespaces.
+//
+// When providing multiple namespaces, the watchNamespace string must not contain
+// spaces, empty namespaces, or duplicate namespaces.
+func parseWatchNamespaceString(namespace string) ([]string, error) {
+	if namespace == "" {
+		// This means that the user did not provide a value for the watchNamespace flag.
+		// In this case, we will watch all namespaces.
+		return nil, nil
+	}
+	visited := make(map[string]bool)
+	namespaces := strings.Split(namespace, ",")
+	for _, ns := range namespaces {
+		if ns == "" {
+			return nil, fmt.Errorf("invalid namespace: empty namespace")
+		}
+		if _, ok := visited[ns]; ok {
+			return nil, fmt.Errorf("duplicate namespace '%s'", ns)
+		}
+		// Settling on the same validation rules as k8s.io/apimachinery/pkg/apis/meta/v1/validation.go
+		// for namespace names. prefix=false means that trailing dashes are not allowed. Trailing dashes
+		// are allowed when the namespace name is used as part of generation.
+		if validationErrorStrings := apimachineryvalidation.ValidateNamespaceName(ns, false); len(validationErrorStrings) > 0 {
+			return nil, fmt.Errorf("invalid namespace '%s': %v", ns, validationErrorStrings)
+		}
+		visited[ns] = true
+	}
+	return namespaces, nil
+}

pkg/config/config_test.go

@@ -13,7 +13,10 @@
 
 package config
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestParseReconcileFlagArgument(t *testing.T) {
 	tests := []struct {
@@ -59,3 +62,48 @@ func TestParseReconcileFlagArgument(t *testing.T) {
 		}
 	}
 }
+
+const (
+	dns1123SubdomainErrorMsg string = "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character"
+)
+
+func TestParseNamespace(t *testing.T) {
+	tests := []struct {
+		name                 string
+		watchNamespaceString string
+		expectedNamespaces   []string
+		expectedErr          bool
+		expectedErrMsg       string
+	}{
+		{"empty namespace", "", nil, false, ""},
+		{"default namespace", "default", []string{"default"}, false, ""},
+		{"two valid namespaces", "default,foo", []string{"default", "foo"}, false, ""},
+		{"three valid namespaces", "default,foo,bar", []string{"default", "foo", "bar"}, false, ""},
+		{"multiple empty namespace", ",,,,,", nil, true, "invalid namespace: empty namespace"},
+		{"duplicate namespace", "foo,bar,bar", nil, true, "duplicate namespace 'bar'"},
+		{"valid namespaces and one empty namespace", "default,foo,", nil, true, "invalid namespace: empty namespace"},
+		{"last namespace is invalid", "default,foo,---", nil, true, dns1123SubdomainErrorMsg},
+		{"non RFC 1123 label compliant namespace - dot", "foo.bar", nil, true, "must not contain dots"},
+		{"non RFC 1123 label compliant namespace - underscore", "foo_bar", nil, true, dns1123SubdomainErrorMsg},
+	}
+	for _, test := range tests {
+		namespaces, err := parseWatchNamespaceString(test.watchNamespaceString)
+		if err != nil && !test.expectedErr {
+			t.Errorf("unexpected error for namespace '%s': %v", test.watchNamespaceString, err)
+		}
+		if err == nil && test.expectedErr {
+			t.Errorf("expected error for namespace '%s', got nil", test.watchNamespaceString)
+		}
+		if err != nil && !strings.Contains(err.Error(), test.expectedErrMsg) {
+			t.Errorf("unexpected error message for namespace '%s': expected '%s', got '%v'", test.watchNamespaceString, test.expectedErrMsg, err)
+		}
+		if len(namespaces) != len(test.expectedNamespaces) {
+			t.Errorf("unexpected number of namespaces for namespace '%s': expected %d, got %d", test.watchNamespaceString, len(test.expectedNamespaces), len(namespaces))
+		}
+		for i, ns := range namespaces {
+			if ns != test.expectedNamespaces[i] {
+				t.Errorf("unexpected namespace for namespace '%s': expected '%s', got '%s'", test.watchNamespaceString, test.expectedNamespaces[i], ns)
+			}
+		}
+	}
+}