fix(substrate): use typed config namespace for discovery

Author: vinayaka krishnamurthy

cmd/kapro/adopt.go

@@ -210,7 +210,6 @@ func importSubstrateObjects(opts importSubstrateOptions, substrateKind, manageme
 					ManagementPolicy: managementPolicy,
 					Selector:         &metav1.LabelSelector{MatchLabels: matchLabels},
 				},
-				Parameters: map[string]string{"namespace": opts.Namespace},
 			},
 		},
 		&kaprov1alpha1.SubstrateDiscoveryPolicy{

cmd/kapro/adopt_test.go

@@ -97,6 +97,9 @@ func TestImportSubstrateObjectsUseClassRefAndTypedConfig(t *testing.T) {
 		substrate.Spec.ConfigRef.Name != "checkout" {
 		t.Fatalf("configRef=%#v, want FluxSubstrateConfig checkout", substrate.Spec.ConfigRef)
 	}
+	if _, ok := substrate.Spec.Parameters["namespace"]; ok {
+		t.Fatalf("live import should keep namespace only on typed config, got parameters=%#v", substrate.Spec.Parameters)
+	}
 	if substrate.Spec.Discovery == nil ||
 		substrate.Spec.Discovery.ManagementPolicy != "Adopt" ||
 		substrate.Spec.Discovery.Selector == nil ||

cmd/kapro/doctor.go

@@ -15,6 +15,8 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -473,10 +475,7 @@ func checkGitOpsSubstrates(ctx context.Context, c client.Client) doctorFinding {
 				mode = strings.ToLower(substrate.Spec.Discovery.ManagementPolicy)
 			}
 		}
-		namespace := strings.TrimSpace(substrate.Spec.Parameters["namespace"])
-		if namespace == "" {
-			namespace = defaultSubstrateNamespace(kind)
-		}
+		namespace := doctorSubstrateNamespace(ctx, c, &substrate)
 		details = append(details, fmt.Sprintf("%s substrate=%s execution=%s mode=%s namespace=%s", substrate.Name, kind, executionMode, mode, namespace))
 		if kind == string(kaprov1alpha1.SubstrateKindArgo) || kind == string(kaprov1alpha1.SubstrateKindFlux) {
 			var ns corev1.Namespace
@@ -507,6 +506,37 @@ func checkGitOpsSubstrates(ctx context.Context, c client.Client) doctorFinding {
 	}
 }
 
+func doctorSubstrateNamespace(ctx context.Context, c client.Client, substrate *kaprov1alpha1.Substrate) string {
+	if configNamespace := doctorSubstrateConfigNamespace(ctx, c, substrate); configNamespace != "" {
+		return configNamespace
+	}
+	if parameterNamespace := strings.TrimSpace(substrate.Spec.Parameters["namespace"]); parameterNamespace != "" {
+		return parameterNamespace
+	}
+	return defaultSubstrateNamespace(substrate.Spec.SubstrateKind())
+}
+
+func doctorSubstrateConfigNamespace(ctx context.Context, c client.Client, substrate *kaprov1alpha1.Substrate) string {
+	if c == nil || substrate.Spec.ConfigRef == nil {
+		return ""
+	}
+	ref := substrate.Spec.ConfigRef
+	gv, err := schema.ParseGroupVersion(ref.APIVersion)
+	if err != nil {
+		return ""
+	}
+	config := &unstructured.Unstructured{}
+	config.SetGroupVersionKind(gv.WithKind(ref.Kind))
+	if err := c.Get(ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, config); err != nil {
+		return ""
+	}
+	namespace, _, err := unstructured.NestedString(config.Object, "spec", "namespace")
+	if err != nil {
+		return ""
+	}
+	return strings.TrimSpace(namespace)
+}
+
 type doctorSecretRef struct {
 	types.NamespacedName
 	Source string

cmd/kapro/doctor_test.go

@@ -13,6 +13,7 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -141,6 +142,44 @@ func TestDoctorReportSummarizesGitOpsSubstrates(t *testing.T) {
 	}
 }
 
+func TestDoctorReportUsesTypedConfigNamespace(t *testing.T) {
+	config := &unstructured.Unstructured{}
+	config.SetGroupVersionKind(schema.GroupVersionKind{Group: "flux.substrate.kapro.io", Version: "v1alpha1", Kind: "FluxSubstrateConfig"})
+	config.SetName("checkout")
+	if err := unstructured.SetNestedField(config.Object, "flux-managed", "spec", "namespace"); err != nil {
+		t.Fatal(err)
+	}
+	substrate := &kaprov1alpha1.Substrate{
+		ObjectMeta: metav1.ObjectMeta{Name: "flux"},
+		Spec: kaprov1alpha1.SubstrateSpec{
+			ClassRef: &kaprov1alpha1.SubstrateClassReference{Name: "flux"},
+			ConfigRef: &kaprov1alpha1.SubstrateObjectReference{
+				APIVersion: "flux.substrate.kapro.io/v1alpha1",
+				Kind:       "FluxSubstrateConfig",
+				Name:       "checkout",
+			},
+			Execution:  &kaprov1alpha1.SubstrateExecutionSpec{Mode: kaprov1alpha1.ExecutionModeHubPush},
+			Parameters: map[string]string{"namespace": "wrong-namespace"},
+			Discovery:  &kaprov1alpha1.SubstrateDiscoverySpec{Enabled: true, ManagementPolicy: "Observe"},
+		},
+	}
+	ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "flux-managed"}}
+	c := fakeDoctorClient(t, append(append(healthyDoctorObjects(), validatingWebhookObjects()...), substrate, config, ns)...)
+	report := collectDoctorReport(context.Background(), c, doctorOptions{
+		Namespace:  "kapro-system",
+		Deployment: "kapro-kapro-operator",
+	}, allowAllSAR)
+
+	substrates := findDoctorCheck(report, "gitops-substrates")
+	details := strings.Join(substrates.Details, ",")
+	if substrates.Status != doctorStatusPass || !strings.Contains(details, "namespace=flux-managed") {
+		t.Fatalf("expected typed config namespace in gitops substrate summary, got %#v", substrates)
+	}
+	if strings.Contains(details, "wrong-namespace") {
+		t.Fatalf("doctor should prefer typed config namespace over parameters, got %#v", substrates)
+	}
+}
+
 func TestDoctorReportWarnsOnMissingGitOpsNamespace(t *testing.T) {
 	substrate := &kaprov1alpha1.Substrate{
 		ObjectMeta: metav1.ObjectMeta{Name: "argo"},

cmd/kapro/scaffold.go

@@ -714,15 +714,13 @@ spec:
     name: %s
   execution:
     mode: hub-push
-  parameters:
-    namespace: %s
   discovery:
     enabled: true
     managementPolicy: %s
     maxObjects: 1000
     selector:
       matchLabels:
-%s`, substrate, family, ledger, substrate, apiVersion, configKind, name, namespace, name, substrate, apiVersion, configKind, name, namespace, managementPolicy, renderYAMLMap(labels, 8))
+%s`, substrate, family, ledger, substrate, apiVersion, configKind, name, namespace, name, substrate, apiVersion, configKind, name, managementPolicy, renderYAMLMap(labels, 8))
 }
 
 func substrateConfigKind(substrate string) (string, string) {

cmd/kapro/scaffold_test.go

@@ -6,6 +6,8 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+
+	"sigs.k8s.io/yaml"
 )
 
 func TestRunInitScaffoldArgo(t *testing.T) {
@@ -480,6 +482,15 @@ func TestRunConnectScaffoldFlux(t *testing.T) {
 			t.Fatalf("missing %q in:\n%s", want, content)
 		}
 	}
+	if strings.Contains(content, "parameters:\n    namespace:") {
+		t.Fatalf("connect scaffold should keep namespace only on typed config, got:\n%s", content)
+	}
+	for _, doc := range strings.Split(content, "\n---\n") {
+		var object map[string]any
+		if err := yaml.Unmarshal([]byte(doc), &object); err != nil {
+			t.Fatalf("connect scaffold emitted invalid YAML document:\n%s\nerror: %v", doc, err)
+		}
+	}
 	for _, forbidden := range []string{
 		"actuator:",
 		"\n  substrate:\n    kind:",

internal/controller/substrate_controller.go

@@ -173,13 +173,7 @@ func (r *SubstrateReconciler) observeDiscovery(ctx context.Context, profile *kap
 	if profile.Spec.Discovery == nil || !profile.Spec.Discovery.Enabled {
 		return counts, "DiscoveryDisabled", "substrate discovery is disabled"
 	}
-	namespace := "argocd"
-	if profile.Spec.SubstrateKind() == string(kaprov1alpha1.SubstrateKindFlux) {
-		namespace = "flux-system"
-	}
-	if profile.Spec.Parameters["namespace"] != "" {
-		namespace = profile.Spec.Parameters["namespace"]
-	}
+	namespace := substrateDiscoveryNamespace(ctx, r.Client, profile)
 
 	switch profile.Spec.SubstrateKind() {
 	case string(kaprov1alpha1.SubstrateKindArgo):
@@ -659,6 +653,49 @@ func (r *SubstrateReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return b.Complete(r)
 }
 
+func substrateDiscoveryNamespace(ctx context.Context, c client.Client, profile *kaprov1alpha1.Substrate) string {
+	namespace := defaultSubstrateDiscoveryNamespace(profile)
+	if configNamespace := substrateConfigNamespace(ctx, c, profile); configNamespace != "" {
+		return configNamespace
+	}
+	if parameterNamespace := strings.TrimSpace(profile.Spec.Parameters["namespace"]); parameterNamespace != "" {
+		return parameterNamespace
+	}
+	return namespace
+}
+
+func defaultSubstrateDiscoveryNamespace(profile *kaprov1alpha1.Substrate) string {
+	switch profile.Spec.SubstrateKind() {
+	case string(kaprov1alpha1.SubstrateKindArgo):
+		return "argocd"
+	case string(kaprov1alpha1.SubstrateKindFlux):
+		return "flux-system"
+	default:
+		return ""
+	}
+}
+
+func substrateConfigNamespace(ctx context.Context, c client.Client, profile *kaprov1alpha1.Substrate) string {
+	if c == nil || profile.Spec.ConfigRef == nil {
+		return ""
+	}
+	ref := profile.Spec.ConfigRef
+	gv, err := schema.ParseGroupVersion(ref.APIVersion)
+	if err != nil {
+		return ""
+	}
+	config := &unstructured.Unstructured{}
+	config.SetGroupVersionKind(gv.WithKind(ref.Kind))
+	if err := c.Get(ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, config); err != nil {
+		return ""
+	}
+	namespace, _, err := unstructured.NestedString(config.Object, "spec", "namespace")
+	if err != nil {
+		return ""
+	}
+	return strings.TrimSpace(namespace)
+}
+
 func typedSubstrateConfigWatchKinds() []schema.GroupVersionKind {
 	return []schema.GroupVersionKind{
 		{Group: "argocd.substrate.kapro.io", Version: "v1alpha1", Kind: "ArgoCDSubstrateConfig"},
@@ -694,7 +731,7 @@ func (r *SubstrateReconciler) substrateProfilesForSubstrateObject(ctx context.Co
 	requests := make([]reconcile.Request, 0, len(profiles.Items))
 	for i := range profiles.Items {
 		profile := &profiles.Items[i]
-		if substrateProfileMatchesObject(profile, obj) {
+		if r.substrateProfileMatchesObject(ctx, profile, obj) {
 			requests = append(requests, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(profile)})
 		}
 	}
@@ -737,7 +774,7 @@ func (r *SubstrateReconciler) substrateProfilesForTypedConfig(ctx context.Contex
 	return requests
 }
 
-func substrateProfileMatchesObject(profile *kaprov1alpha1.Substrate, obj client.Object) bool {
+func (r *SubstrateReconciler) substrateProfileMatchesObject(ctx context.Context, profile *kaprov1alpha1.Substrate, obj client.Object) bool {
 	if profile.Spec.Discovery == nil || !profile.Spec.Discovery.Enabled {
 		return false
 	}
@@ -759,13 +796,7 @@ func substrateProfileMatchesObject(profile *kaprov1alpha1.Substrate, obj client.
 	if profile.Spec.SubstrateKind() != string(objectDriver) {
 		return false
 	}
-	namespace := "argocd"
-	if profile.Spec.SubstrateKind() == string(kaprov1alpha1.SubstrateKindFlux) {
-		namespace = "flux-system"
-	}
-	if profile.Spec.Parameters["namespace"] != "" {
-		namespace = profile.Spec.Parameters["namespace"]
-	}
+	namespace := substrateDiscoveryNamespace(ctx, r.Client, profile)
 	if obj.GetNamespace() != namespace {
 		return false
 	}

internal/controller/substrate_controller_test.go

@@ -326,6 +326,77 @@ func TestSubstrateProfileArgoDiscoveryCountsExistingResources(t *testing.T) {
 	}
 }
 
+func TestSubstrateProfileDiscoveryUsesTypedConfigNamespace(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := kaprov1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	config := typedSubstrateConfigWithSpecNamespace("flux.substrate.kapro.io/v1alpha1", "FluxSubstrateConfig", "checkout", "flux-managed")
+	profile := &kaprov1alpha1.Substrate{
+		ObjectMeta: metav1.ObjectMeta{Name: "flux"},
+		Spec: kaprov1alpha1.SubstrateSpec{
+			ClassRef: &kaprov1alpha1.SubstrateClassReference{Name: "flux"},
+			ConfigRef: &kaprov1alpha1.SubstrateObjectReference{
+				APIVersion: "flux.substrate.kapro.io/v1alpha1",
+				Kind:       "FluxSubstrateConfig",
+				Name:       "checkout",
+			},
+			Discovery:  &kaprov1alpha1.SubstrateDiscoverySpec{Enabled: true},
+			Parameters: map[string]string{"namespace": "wrong-namespace"},
+		},
+	}
+	gitRepository := &unstructured.Unstructured{}
+	gitRepository.SetGroupVersionKind(schema.GroupVersionKind{Group: "source.toolkit.fluxcd.io", Version: "v1", Kind: "GitRepository"})
+	gitRepository.SetNamespace("flux-managed")
+	gitRepository.SetName("checkout-git")
+
+	r := &SubstrateReconciler{
+		Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(profile, config, gitRepository).Build(),
+	}
+
+	counts, reason, _ := r.observeDiscovery(context.Background(), profile)
+	if reason != "DiscoverySucceeded" {
+		t.Fatalf("reason=%s", reason)
+	}
+	if counts.applications != 1 {
+		t.Fatalf("applications=%d, want typed config namespace object to be discovered", counts.applications)
+	}
+}
+
+func TestSubstrateProfileObjectWatchUsesTypedConfigNamespace(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := kaprov1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	config := typedSubstrateConfigWithSpecNamespace("flux.substrate.kapro.io/v1alpha1", "FluxSubstrateConfig", "checkout", "flux-managed")
+	profile := &kaprov1alpha1.Substrate{
+		ObjectMeta: metav1.ObjectMeta{Name: "flux"},
+		Spec: kaprov1alpha1.SubstrateSpec{
+			ClassRef: &kaprov1alpha1.SubstrateClassReference{Name: "flux"},
+			ConfigRef: &kaprov1alpha1.SubstrateObjectReference{
+				APIVersion: "flux.substrate.kapro.io/v1alpha1",
+				Kind:       "FluxSubstrateConfig",
+				Name:       "checkout",
+			},
+			Discovery:  &kaprov1alpha1.SubstrateDiscoverySpec{Enabled: true},
+			Parameters: map[string]string{"namespace": "wrong-namespace"},
+		},
+	}
+	gitRepository := &unstructured.Unstructured{}
+	gitRepository.SetGroupVersionKind(schema.GroupVersionKind{Group: "source.toolkit.fluxcd.io", Version: "v1", Kind: "GitRepository"})
+	gitRepository.SetNamespace("flux-managed")
+	gitRepository.SetName("checkout-git")
+
+	r := &SubstrateReconciler{
+		Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(profile, config).Build(),
+	}
+
+	requests := r.substrateProfilesForSubstrateObject(context.Background(), gitRepository)
+	if len(requests) != 1 || requests[0].Name != "flux" {
+		t.Fatalf("requests=%#v, want flux profile from typed config namespace", requests)
+	}
+}
+
 func TestSubstrateProfileArgoDiscoveryClassifiesExistingGitOpsPatterns(t *testing.T) {
 	scheme := runtime.NewScheme()
 	if err := clientgoscheme.AddToScheme(scheme); err != nil {
@@ -587,6 +658,14 @@ func typedSubstrateConfig(apiVersion, kind, namespace, name string) *unstructure
 	return config
 }
 
+func typedSubstrateConfigWithSpecNamespace(apiVersion, kind, name, namespace string) *unstructured.Unstructured {
+	config := typedSubstrateConfig(apiVersion, kind, "", name)
+	if err := unstructured.SetNestedField(config.Object, namespace, "spec", "namespace"); err != nil {
+		panic(err)
+	}
+	return config
+}
+
 func hasDiscoveryPattern(objects []kaprov1alpha1.DiscoveredSubstrateObject, pattern string) bool {
 	for _, obj := range objects {
 		if obj.Pattern == pattern {

internal/controller/substratediscoverypolicy_controller.go

@@ -136,7 +136,7 @@ func (r *SubstrateDiscoveryPolicyReconciler) discover(ctx context.Context, polic
 	if !a.Capabilities().SupportsDiscover {
 		return adapterPolicyDiscoveryOutcome{reason: "DiscoveryUnsupported", message: fmt.Sprintf("substrate kind %s does not support discovery", substrate.Spec.SubstrateKind())}, nil
 	}
-	req, err := adapterPolicyDiscoveryRequest(&substrate, policy)
+	req, err := r.adapterPolicyDiscoveryRequest(ctx, &substrate, policy)
 	if err != nil {
 		return adapterPolicyDiscoveryOutcome{reason: "InvalidSelector", message: err.Error()}, nil
 	}
@@ -247,12 +247,12 @@ func adapterPolicySubstrateStatusObjects(substrate *kaprov1alpha1.Substrate) int
 	return substrate.Status.DiscoveredClusters + substrate.Status.DiscoveredApplications + substrate.Status.DiscoveredApplicationSets
 }
 
-func adapterPolicyDiscoveryRequest(substrate *kaprov1alpha1.Substrate, policy *kaprov1alpha1.SubstrateDiscoveryPolicy) (kaproadapter.DiscoveryRequest, error) {
+func (r *SubstrateDiscoveryPolicyReconciler) adapterPolicyDiscoveryRequest(ctx context.Context, substrate *kaprov1alpha1.Substrate, policy *kaprov1alpha1.SubstrateDiscoveryPolicy) (kaproadapter.DiscoveryRequest, error) {
 	req := kaproadapter.DiscoveryRequest{
 		Substrate:      substrate,
 		SubstrateKind:  kaprov1alpha1.SubstrateKind(substrate.Spec.SubstrateKind()),
 		ExecutionScope: substrateRuntimeForDiscovery(substrate.Spec.ExecutionMode()),
-		Namespace:      substrate.Spec.Parameters["namespace"],
+		Namespace:      substrateDiscoveryNamespace(ctx, r.Client, substrate),
 		Parameters:     substrate.Spec.Parameters,
 	}
 	if substrate.Spec.Discovery != nil {