Validate all Config Sync managed Namespaces (#961)

GoogleContainerTools · Oct 31, 2023 · c93c2f2 · c93c2f2
1 parent e79862c
commit c93c2f2
Show file tree

Hide file tree

Showing 14 changed files with 73 additions and 64 deletions.
diff --git a/e2e/nomostest/config_sync.go b/e2e/nomostest/config_sync.go
@@ -109,7 +109,7 @@ func IsReconcilerManagerConfigMap(obj client.Object) bool {
 // otel-collector Deployment in the config-management-monitoring namespace.
 func isOtelCollectorDeployment(obj client.Object) bool {
 	return obj.GetName() == ocmetrics.OtelCollectorName &&
-		obj.GetNamespace() == ocmetrics.MonitoringNamespace &&
+		obj.GetNamespace() == configmanagement.MonitoringNamespace &&
 		obj.GetObjectKind().GroupVersionKind() == kinds.Deployment()
 }
 
@@ -345,7 +345,7 @@ func ValidateMultiRepoDeployments(nt *NT) error {
 			predicates = append(predicates, testpredicates.HasGenerationAtLeast(2))
 		}
 		return nt.Watcher.WatchObject(kinds.Deployment(),
-			ocmetrics.OtelCollectorName, ocmetrics.MonitoringNamespace, predicates)
+			ocmetrics.OtelCollectorName, configmanagement.MonitoringNamespace, predicates)
 	})
 	tg.Go(func() error {
 		// The root-reconciler is created after the reconciler-manager is ready.

diff --git a/e2e/nomostest/new.go b/e2e/nomostest/new.go
@@ -35,7 +35,6 @@ import (
 	"kpt.dev/configsync/pkg/api/configsync"
 	"kpt.dev/configsync/pkg/importer/filesystem"
 	"kpt.dev/configsync/pkg/kinds"
-	"kpt.dev/configsync/pkg/metrics"
 	"kpt.dev/configsync/pkg/testing/fake"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/kind/pkg/errors"
@@ -342,7 +341,7 @@ func FreshTestEnv(t nomostesting.NTB, opts *ntopts.New) *NT {
 	if err := nt.KubeClient.Create(fake.NamespaceObject(configmanagement.ControllerNamespace)); err != nil {
 		nt.T.Fatal(err)
 	}
-	if err := nt.KubeClient.Create(fake.NamespaceObject(metrics.MonitoringNamespace)); err != nil {
+	if err := nt.KubeClient.Create(fake.NamespaceObject(configmanagement.MonitoringNamespace)); err != nil {
 		nt.T.Fatal(err)
 	}
 	if *e2e.GitProvider == e2e.Local {

diff --git a/e2e/nomostest/nt.go b/e2e/nomostest/nt.go
@@ -206,7 +206,7 @@ func (nt *NT) Must(args ...interface{}) {
 // CSNamespaces is the namespaces of the Config Sync components.
 var CSNamespaces = []string{
 	configmanagement.ControllerNamespace,
-	ocmetrics.MonitoringNamespace,
+	configmanagement.MonitoringNamespace,
 	configmanagement.RGControllerNamespace,
 }
 
@@ -675,12 +675,12 @@ func (nt *NT) portForwardOtelCollector() {
 		nt.T.Fatal("otel collector port forward already initialized")
 	}
 	nt.otelCollectorPortForwarder = nt.newPortForwarder(
-		ocmetrics.MonitoringNamespace,
+		configmanagement.MonitoringNamespace,
 		ocmetrics.OtelCollectorName,
 		fmt.Sprintf(":%d", testmetrics.OtelCollectorMetricsPort),
 	)
 	nt.startPortForwarder(
-		ocmetrics.MonitoringNamespace,
+		configmanagement.MonitoringNamespace,
 		ocmetrics.OtelCollectorName,
 		nt.otelCollectorPortForwarder,
 	)

diff --git a/e2e/nomostest/reset.go b/e2e/nomostest/reset.go
@@ -36,7 +36,6 @@ import (
 	"kpt.dev/configsync/pkg/importer/filesystem"
 	"kpt.dev/configsync/pkg/kinds"
 	"kpt.dev/configsync/pkg/metadata"
-	"kpt.dev/configsync/pkg/metrics"
 	"kpt.dev/configsync/pkg/reconcilermanager"
 	"kpt.dev/configsync/pkg/syncer/differ"
 	"kpt.dev/configsync/pkg/util/log"
@@ -50,7 +49,7 @@ import (
 var sharedTestNamespaces = []string{
 	configsync.ControllerNamespace,
 	configmanagement.RGControllerNamespace,
-	metrics.MonitoringNamespace,
+	configmanagement.MonitoringNamespace,
 	testGitNamespace,
 	prometheusNamespace,
 }

diff --git a/e2e/testcases/otel_collector_test.go b/e2e/testcases/otel_collector_test.go
@@ -37,6 +37,7 @@ import (
 	"kpt.dev/configsync/e2e/nomostest/retry"
 	nomostesting "kpt.dev/configsync/e2e/nomostest/testing"
 	"kpt.dev/configsync/e2e/nomostest/workloadidentity"
+	"kpt.dev/configsync/pkg/api/configmanagement"
 	"kpt.dev/configsync/pkg/api/configsync"
 	"kpt.dev/configsync/pkg/core"
 	"kpt.dev/configsync/pkg/kinds"
@@ -82,14 +83,14 @@ func TestOtelCollectorDeployment(t *testing.T) {
 		nt.MustKubectl("delete", "-f", "../testdata/otel-collector/otel-cm-monarch-rejected-labels.yaml", "--ignore-not-found")
 		nt.T.Log("Restart otel-collector pod to reset the ConfigMap and log")
 		nomostest.DeletePodByLabel(nt, "app", ocmetrics.OpenTelemetry, false)
-		if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, ocmetrics.MonitoringNamespace); err != nil {
+		if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, configmanagement.MonitoringNamespace); err != nil {
 			nt.T.Errorf("otel-collector pod failed to come up after a restart: %v", err)
 		}
 	})
 
 	nt.T.Log("Restart otel-collector pod to refresh the ConfigMap, log and IAM")
 	nomostest.DeletePodByLabel(nt, "app", ocmetrics.OpenTelemetry, false)
-	if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, ocmetrics.MonitoringNamespace); err != nil {
+	if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, configmanagement.MonitoringNamespace); err != nil {
 		nt.T.Fatal(err)
 	}
 
@@ -123,7 +124,7 @@ func TestOtelCollectorDeployment(t *testing.T) {
 	}
 
 	nt.T.Log("Checking the otel-collector log contains no failure...")
-	err = validateDeploymentLogHasNoFailure(nt, ocmetrics.OtelCollectorName, ocmetrics.MonitoringNamespace, GCMExportErrorCaption)
+	err = validateDeploymentLogHasNoFailure(nt, ocmetrics.OtelCollectorName, configmanagement.MonitoringNamespace, GCMExportErrorCaption)
 	if err != nil {
 		nt.T.Fatal(err)
 	}
@@ -136,13 +137,13 @@ func TestOtelCollectorDeployment(t *testing.T) {
 	nt.MustKubectl("apply", "-f", "../testdata/otel-collector/otel-cm-monarch-rejected-labels.yaml")
 	nt.T.Log("Restart otel-collector pod to refresh the ConfigMap and log")
 	nomostest.DeletePodByLabel(nt, "app", ocmetrics.OpenTelemetry, false)
-	if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, ocmetrics.MonitoringNamespace); err != nil {
+	if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, configmanagement.MonitoringNamespace); err != nil {
 		nt.T.Fatal(err)
 	}
 
 	nt.T.Log("Checking the otel-collector log contains failure...")
 	_, err = retry.Retry(60*time.Second, func() error {
-		return validateDeploymentLogHasFailure(nt, ocmetrics.OtelCollectorName, ocmetrics.MonitoringNamespace, GCMExportErrorCaption)
+		return validateDeploymentLogHasFailure(nt, ocmetrics.OtelCollectorName, configmanagement.MonitoringNamespace, GCMExportErrorCaption)
 	})
 	if err != nil {
 		nt.T.Fatal(err)
@@ -166,7 +167,7 @@ func TestOtelCollectorGCMLabelAggregation(t *testing.T) {
 
 	nt.T.Log("Restarting the otel-collector pod to refresh the service account")
 	nomostest.DeletePodByLabel(nt, "app", ocmetrics.OpenTelemetry, false)
-	if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, ocmetrics.MonitoringNamespace); err != nil {
+	if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, configmanagement.MonitoringNamespace); err != nil {
 		nt.T.Fatal(err)
 	}
 
@@ -224,7 +225,7 @@ func setupMetricsServiceAccount(nt *nomostest.NT) {
 
 		nt.T.Cleanup(func() {
 			ksa := &corev1.ServiceAccount{}
-			if err := nt.KubeClient.Get(DefaultMonitorKSA, ocmetrics.MonitoringNamespace, ksa); err != nil {
+			if err := nt.KubeClient.Get(DefaultMonitorKSA, configmanagement.MonitoringNamespace, ksa); err != nil {
 				if apierrors.IsNotFound(err) {
 					return // no need to remove annotation
 				}
@@ -238,7 +239,7 @@ func setupMetricsServiceAccount(nt *nomostest.NT) {
 
 		nt.T.Log(fmt.Sprintf("Workload identity enabled, adding KSA annotation to use %s service account", MonitorGSA))
 		ksa := &corev1.ServiceAccount{}
-		if err := nt.KubeClient.Get(DefaultMonitorKSA, ocmetrics.MonitoringNamespace, ksa); err != nil {
+		if err := nt.KubeClient.Get(DefaultMonitorKSA, configmanagement.MonitoringNamespace, ksa); err != nil {
 			nt.T.Fatalf("failed to get service account: %v", err)
 		}
 		core.SetAnnotation(ksa, "iam.gke.io/gcp-service-account", gsaEmail)

diff --git a/pkg/api/configmanagement/register.go b/pkg/api/configmanagement/register.go
@@ -57,12 +57,16 @@ const (
 	// RGControllerNamespace is the namespace used for the resource-group controller
 	RGControllerNamespace = "resource-group-system"
 
+	// MonitoringNamespace is the namespace used for Config Sync monitoring
+	MonitoringNamespace = "config-management-monitoring"
+
 	// RGControllerName is the name used for the resource-group controller
 	RGControllerName = "resource-group-controller-manager"
 )
 
-// IsControllerNamespace returns true if the namespace is the ACM Controller Namespace.
+// IsControllerNamespace returns true if the namespace is one of the Config Sync controller Namespace.
 func IsControllerNamespace(name string) bool {
-	// For now we only forbid syncing the Namespace containing the ACM controllers.
-	return name == ControllerNamespace
+	return name == ControllerNamespace ||
+		name == RGControllerNamespace ||
+		name == MonitoringNamespace
 }
diff --git a/pkg/bugreport/bugreport.go b/pkg/bugreport/bugreport.go
@@ -33,7 +33,6 @@ import (
 	"k8s.io/klog/v2"
 	v1 "kpt.dev/configsync/pkg/api/configmanagement/v1"
 	"kpt.dev/configsync/pkg/api/configsync/v1beta1"
-	"kpt.dev/configsync/pkg/metrics"
 	"kpt.dev/configsync/pkg/policycontroller"
 
 	corev1 "k8s.io/api/core/v1"
@@ -439,7 +438,7 @@ func (b *BugReporter) FetchCMSystemPods(ctx context.Context) (rd []Readable) {
 		configmanagement.ControllerNamespace,
 		metav1.NamespaceSystem,
 		configmanagement.RGControllerNamespace,
-		metrics.MonitoringNamespace,
+		configmanagement.MonitoringNamespace,
 		policycontroller.NamespaceSystem,
 	}
 

diff --git a/pkg/bugreport/constants.go b/pkg/bugreport/constants.go
@@ -16,7 +16,6 @@ package bugreport
 
 import (
 	"kpt.dev/configsync/pkg/api/configmanagement"
-	"kpt.dev/configsync/pkg/metrics"
 	"kpt.dev/configsync/pkg/policycontroller"
 )
 
@@ -39,6 +38,6 @@ var (
 		PolicyController:     policycontroller.NamespaceSystem,
 		ConfigSync:           configmanagement.ControllerNamespace,
 		ResourceGroup:        configmanagement.RGControllerNamespace,
-		ConfigSyncMonitoring: metrics.MonitoringNamespace,
+		ConfigSyncMonitoring: configmanagement.MonitoringNamespace,
 	}
 )
diff --git a/pkg/importer/analyzer/validation/nonhierarchical/illegal_namespace_validator.go b/pkg/importer/analyzer/validation/nonhierarchical/illegal_namespace_validator.go
@@ -15,7 +15,6 @@
 package nonhierarchical
 
 import (
-	"kpt.dev/configsync/pkg/api/configmanagement"
 	"kpt.dev/configsync/pkg/status"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -25,9 +24,9 @@ const IllegalNamespaceErrorCode = "1034"
 
 var illegalNamespaceError = status.NewErrorBuilder(IllegalNamespaceErrorCode)
 
-// IllegalNamespace reports that the config-management-system Namespace MUST NOT be declared.
+// IllegalNamespace reports that the controller Namespaces MUST NOT be declared.
 func IllegalNamespace(resource client.Object) status.Error {
 	return illegalNamespaceError.
-		Sprintf("The %q Namespace must not be declared", configmanagement.ControllerNamespace).
+		Sprintf("The %q Namespace must not be declared", resource.GetName()).
 		BuildWithResources(resource)
 }
diff --git a/pkg/metrics/otel.go b/pkg/metrics/otel.go
@@ -30,9 +30,6 @@ const (
 	// OtelCollectorCustomCM is the name of the custom OpenTelemetry Collector ConfigMap.
 	OtelCollectorCustomCM = "otel-collector-custom"
 
-	// MonitoringNamespace is the Namespace used for OpenTelemetry Collector deployment.
-	MonitoringNamespace = "config-management-monitoring"
-
 	// CollectorConfigGooglecloud is the OpenTelemetry Collector configuration with
 	// the googlecloud exporter.
 	CollectorConfigGooglecloud = `receivers:

diff --git a/pkg/reconcilermanager/controllers/otel_controller.go b/pkg/reconcilermanager/controllers/otel_controller.go
@@ -26,6 +26,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"kpt.dev/configsync/pkg/api/configmanagement"
 	"kpt.dev/configsync/pkg/core"
 	"kpt.dev/configsync/pkg/metadata"
 	"kpt.dev/configsync/pkg/metrics"
@@ -98,7 +99,7 @@ func (r *OtelReconciler) Reconcile(ctx context.Context, req reconcile.Request) (
 func otelCollectorDeploymentRef() client.ObjectKey {
 	return client.ObjectKey{
 		Name:      metrics.OtelCollectorName,
-		Namespace: metrics.MonitoringNamespace,
+		Namespace: configmanagement.MonitoringNamespace,
 	}
 }
 
@@ -143,7 +144,7 @@ func (r *OtelReconciler) configureGooglecloudConfigMap(ctx context.Context) ([]b
 
 	cm := &corev1.ConfigMap{}
 	cm.Name = metrics.OtelCollectorGooglecloud
-	cm.Namespace = metrics.MonitoringNamespace
+	cm.Namespace = configmanagement.MonitoringNamespace
 	op, err := CreateOrUpdate(ctx, r.client, cm, func() error {
 		cm.Labels = map[string]string{
 			"app":                metrics.OpenTelemetry,
@@ -198,13 +199,13 @@ func (r *OtelReconciler) SetupWithManager(mgr controllerruntime.Manager) error {
 	// Process create / update events for resources in the `config-management-monitoring` namespace.
 	p := predicate.Funcs{
 		CreateFunc: func(e event.CreateEvent) bool {
-			return e.Object.GetNamespace() == metrics.MonitoringNamespace
+			return e.Object.GetNamespace() == configmanagement.MonitoringNamespace
 		},
 		UpdateFunc: func(e event.UpdateEvent) bool {
-			return e.ObjectNew.GetNamespace() == metrics.MonitoringNamespace
+			return e.ObjectNew.GetNamespace() == configmanagement.MonitoringNamespace
 		},
 		DeleteFunc: func(e event.DeleteEvent) bool {
-			return e.Object.GetNamespace() == metrics.MonitoringNamespace
+			return e.Object.GetNamespace() == configmanagement.MonitoringNamespace
 		},
 	}
 	return controllerruntime.NewControllerManagedBy(mgr).