Repository: fluxcd/flux
Pull request #2799 (2020-01-29 15:38:13)

Fix syncing with --k8s-default-namespace
Fixes https://github.com/fluxcd/flux/issues/2797

The problem was that the namespace was set on the `KubeManifest` next to the raw `Resource` but then when the `KubeManifest` is synced as a `Resource`, the raw bytes are taken.
Since we're reserializing in `applyMetadata` anyway, I just update the namespace there.

<!--
# General contribution criteria

Please have a look at our contribution guidelines: https://github.com/fluxcd/flux/blob/master/CONTRIBUTING.md
Particularly the sections about the:

 - DCO;
 - contribution workflow; and
 - how to get your fix accepted

To help the maintainers out when they're writing release notes, please
try to include a sentence or two here describing your change for end
users. See the CHANGELOG.md file in the top-level directory for examples.

Particularly for ground-breaking changes and new features, it's important to
make users and developers aware of what's changing and where those changes
were documented or discussed.

Even for smaller changes it's useful to see things documented as well, as it
gives everybody a chance to see at a glance what's coming up in the next
release. It makes the life of the project maintainer a lot easier as well.

The following short checklist can be used to make sure your PR is of good
quality, and can be merged easily:

- [ ] if it resolves an issue;
      is a reference (i.e. #1) to this issue included?
- [ ] if it introduces a new functionality or configuration flag;
      did you document this in the references or guides?
- [ ] optional but much appreciated;
      do you think many users would profit from a dedicated setting
      for this functionality in the Helm chart?
-->


2opremio: Can you squash the commits?
2opremio: And rebase on top of the current master?

Patch:
diff --git a/pkg/cluster/kubernetes/sync.go b/pkg/cluster/kubernetes/sync.go
index 00f1183f..5883e228 100644
--- a/pkg/cluster/kubernetes/sync.go
+++ b/pkg/cluster/kubernetes/sync.go
@@ -377,6 +377,15 @@ func applyMetadata(res resource.Resource, syncSetName, checksum string) ([]byte,
 		mixin["labels"] = mixinLabels
 	}
 
+	// After loading the manifest the namespace of the resource can change
+	// (e.g. a default namespace is applied)
+	// The `ResourceID` should give us the up-to-date value
+	// (see `KubeManifest.SetNamespace`)
+	namespace, _, _ := res.ResourceID().Components()
+	if namespace != kresource.ClusterScope {
+		mixin["namespace"] = namespace
+	}
+
 	if checksum != "" {
 		mixinAnnotations := map[string]string{}
 		mixinAnnotations[checksumAnnotation] = checksum


Test patch:
diff --git a/pkg/cluster/kubernetes/sync_test.go b/pkg/cluster/kubernetes/sync_test.go
index 9f89fd9a..e276cfe3 100644
--- a/pkg/cluster/kubernetes/sync_test.go
+++ b/pkg/cluster/kubernetes/sync_test.go
@@ -368,11 +368,11 @@ metadata:
 		}
 	}
 
-	test := func(t *testing.T, kube *Cluster, defs, expectedAfterSync string, expectErrors bool) {
+	testDefaultNs := func(t *testing.T, kube *Cluster, defs, expectedAfterSync string, expectErrors bool, defaultNamespace string) {
 		saved := getKubeconfigDefaultNamespace
 		getKubeconfigDefaultNamespace = func() (string, error) { return defaultTestNamespace, nil }
 		defer func() { getKubeconfigDefaultNamespace = saved }()
-		namespacer, err := NewNamespacer(kube.client.coreClient.Discovery(), "")
+		namespacer, err := NewNamespacer(kube.client.coreClient.Discovery(), defaultNamespace)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -425,6 +425,9 @@ metadata:
 			// the intersection of actual and expected above.
 		}
 	}
+	test := func(t *testing.T, kube *Cluster, defs, expectedAfterSync string, expectErrors bool) {
+		testDefaultNs(t, kube, defs, expectedAfterSync, expectErrors, "")
+	}
 
 	t.Run("sync adds and GCs resources", func(t *testing.T) {
 		kube, _, cancel := setup(t)
@@ -475,6 +478,43 @@ metadata:
 		test(t, kube, nsDef, nsDef, false)
 	})
 
+	t.Run("sync applies default namespace", func(t *testing.T) {
+		kube, _, cancel := setup(t)
+		defer cancel()
+		kube.GC = true
+
+		const depDef = `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: bar
+`
+		const depDefNamespaced = `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: bar
+  namespace: system
+`
+		const depDefAlreadyNamespaced = `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: bar
+  namespace: other
+`
+		const ns1 = `---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: foobar
+`
+		defaultNs := "system"
+		testDefaultNs(t, kube, depDef, depDefNamespaced, false, defaultNs)
+		testDefaultNs(t, kube, depDefAlreadyNamespaced, depDefAlreadyNamespaced, false, defaultNs)
+		testDefaultNs(t, kube, ns1, ns1, false, defaultNs)
+	})
+
 	t.Run("sync won't delete resources that got the fallback namespace when created", func(t *testing.T) {
 		// NB: this tests the fake client implementation to some
 		// extent as well. It relies on it to reflect the kubectl