Fix namespace handling regression (#403)

Fixed a couple different bugs related to namespace handling that
were introduced by the recent client-go refactoring. Added a test
that covers the three related cases:

1. Changing a non-namespaceable resource (Namespace)
2. Changing the namespace on a Pod from "" -> "default"
3. Changing the namespace on a Pod from "default" -> ""

pkg/provider/provider.go

@@ -251,9 +251,6 @@ func (k *kubeProvider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (
 		assignNameIfAutonamable(newInputs, urn.Name())
 	}
 
-	// Ensure that a namespace is set. Use "default" if none specified.
-	newInputs.SetNamespace(canonicalNamespace(newInputs.GetNamespace()))
-
 	gvk, err := k.gvkFromURN(urn)
 	if err != nil {
 		return nil, err
@@ -351,6 +348,10 @@ func (k *kubeProvider) Diff(
 		return nil, err
 	}
 
+	// Explicitly set the "default" namespace if unset so that the diff ignores it.
+	oldInputs.SetNamespace(canonicalNamespace(oldInputs.GetNamespace()))
+	newInputs.SetNamespace(canonicalNamespace(newInputs.GetNamespace()))
+
 	// Decide whether to replace the resource.
 	replaces, err := forceNewProperties(oldInputs.Object, newInputs.Object, gvk)
 	if err != nil {
@@ -376,7 +377,7 @@ func (k *kubeProvider) Diff(
 			newInputs.GetName() == oldInputs.GetName() &&
 			// 4. The resource is being deployed to the same namespace (i.e., we aren't creating the
 			// object in a new namespace and then deleting the old one).
-			canonicalNamespace(newInputs.GetNamespace()) == canonicalNamespace(oldInputs.GetNamespace())
+			newInputs.GetNamespace() == oldInputs.GetNamespace()
 
 	return &pulumirpc.DiffResponse{
 		Changes:             hasChanges,
@@ -416,7 +417,6 @@ func (k *kubeProvider) Create(
 		return nil, err
 	}
 	newInputs := propMapToUnstructured(newResInputs)
-	newInputs.SetNamespace(canonicalNamespace(newInputs.GetNamespace()))
 
 	config := await.CreateConfig{
 		ProviderConfig: await.ProviderConfig{
@@ -503,11 +503,7 @@ func (k *kubeProvider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*p
 		oldInputs.SetGroupVersionKind(newInputs.GroupVersionKind())
 	}
 
-	ns, name := ParseFqName(req.GetId())
-	ns = canonicalNamespace(ns)
-	if oldInputs.GetNamespace() == "" {
-		oldInputs.SetNamespace(ns)
-	}
+	_, name := ParseFqName(req.GetId())
 	if oldInputs.GetName() == "" {
 		oldInputs.SetName(name)
 	}
@@ -659,7 +655,6 @@ func (k *kubeProvider) Update(
 		return nil, err
 	}
 	newInputs := propMapToUnstructured(newResInputs)
-	newInputs.SetNamespace(canonicalNamespace(newInputs.GetNamespace()))
 
 	config := await.UpdateConfig{
 		ProviderConfig: await.ProviderConfig{

tests/integration/namespace/namespace_test.go

@@ -0,0 +1,142 @@
+// Copyright 2016-2019, Pulumi Corporation.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package ints
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/pulumi/pulumi-kubernetes/pkg/openapi"
+
+	"github.com/pulumi/pulumi/pkg/tokens"
+
+	"github.com/pulumi/pulumi-kubernetes/tests"
+	"github.com/pulumi/pulumi/pkg/resource"
+	"github.com/pulumi/pulumi/pkg/resource/deploy/providers"
+	"github.com/pulumi/pulumi/pkg/testing/integration"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNamespace(t *testing.T) {
+	kubectx := os.Getenv("KUBERNETES_CONTEXT")
+
+	if kubectx == "" {
+		t.Skipf("Skipping test due to missing KUBERNETES_CONTEXT variable")
+	}
+
+	var nmPodName, defaultPodName string
+	integration.ProgramTest(t, &integration.ProgramTestOptions{
+		Dir:          "step1",
+		Dependencies: []string{"@pulumi/kubernetes"},
+		Quick:        true,
+		ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
+			assert.NotNil(t, stackInfo.Deployment)
+			assert.Equal(t, 5, len(stackInfo.Deployment.Resources))
+
+			tests.SortResourcesByURN(stackInfo)
+
+			stackRes := stackInfo.Deployment.Resources[4]
+			assert.Equal(t, resource.RootStackType, stackRes.URN.Type())
+
+			provRes := stackInfo.Deployment.Resources[3]
+			assert.True(t, providers.IsProviderType(provRes.URN.Type()))
+
+			// Assert the Namespace was created
+			namespace := stackInfo.Deployment.Resources[0]
+			assert.Equal(t, tokens.Type("kubernetes:core/v1:Namespace"), namespace.URN.Type())
+
+			// Assert the "no metadata" Pod was created in the "default" namespace.
+			nmPod := stackInfo.Deployment.Resources[2]
+			assert.Equal(t, tokens.Type("kubernetes:core/v1:Pod"), nmPod.URN.Type())
+			nmPodNamespace, _ := openapi.Pluck(nmPod.Outputs, "metadata", "namespace")
+			assert.Equal(t, nmPodNamespace.(string), "default")
+			nmPodNameRaw, _ := openapi.Pluck(nmPod.Outputs, "metadata", "name")
+			nmPodName = nmPodNameRaw.(string)
+			assert.True(t, strings.HasPrefix(nmPodName, "no-metadata-pod"))
+
+			// Assert the "explicit default namespace" Pod was created in the "default" namespace.
+			defaultPod := stackInfo.Deployment.Resources[1]
+			assert.Equal(t, tokens.Type("kubernetes:core/v1:Pod"), defaultPod.URN.Type())
+			defaultPodNamespace, _ := openapi.Pluck(defaultPod.Outputs, "metadata", "namespace")
+			assert.Equal(t, defaultPodNamespace.(string), "default")
+			defaultPodNameRaw, _ := openapi.Pluck(defaultPod.Outputs, "metadata", "name")
+			defaultPodName = defaultPodNameRaw.(string)
+			assert.True(t, strings.HasPrefix(defaultPodName, "default-ns-pod"))
+		},
+		EditDirs: []integration.EditDir{
+			{
+				Dir:      "step2",
+				Additive: true,
+				ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
+					assert.NotNil(t, stackInfo.Deployment)
+					assert.Equal(t, 5, len(stackInfo.Deployment.Resources))
+
+					tests.SortResourcesByURN(stackInfo)
+
+					stackRes := stackInfo.Deployment.Resources[4]
+					assert.Equal(t, resource.RootStackType, stackRes.URN.Type())
+
+					provRes := stackInfo.Deployment.Resources[3]
+					assert.True(t, providers.IsProviderType(provRes.URN.Type()))
+
+					// Assert that the Namespace was updated with the expected label.
+					namespace := stackInfo.Deployment.Resources[0]
+					assert.Equal(t, tokens.Type("kubernetes:core/v1:Namespace"), namespace.URN.Type())
+					namespaceLabels, _ := openapi.Pluck(namespace.Outputs, "metadata", "labels")
+					assert.Equal(t, map[string]interface{}{"hello": "world"},
+						namespaceLabels.(map[string]interface{}))
+				},
+			},
+			{
+				Dir:      "step3",
+				Additive: true,
+				ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
+					assert.NotNil(t, stackInfo.Deployment)
+					assert.Equal(t, 5, len(stackInfo.Deployment.Resources))
+
+					tests.SortResourcesByURN(stackInfo)
+
+					stackRes := stackInfo.Deployment.Resources[4]
+					assert.Equal(t, resource.RootStackType, stackRes.URN.Type())
+
+					provRes := stackInfo.Deployment.Resources[3]
+					assert.True(t, providers.IsProviderType(provRes.URN.Type()))
+
+					namespace := stackInfo.Deployment.Resources[0]
+					assert.Equal(t, tokens.Type("kubernetes:core/v1:Namespace"), namespace.URN.Type())
+
+					// Assert the "no metadata" Pod has not changed.
+					nmPod := stackInfo.Deployment.Resources[2]
+					assert.Equal(t, tokens.Type("kubernetes:core/v1:Pod"), nmPod.URN.Type())
+					nmPodNamespace, _ := openapi.Pluck(nmPod.Outputs, "metadata", "namespace")
+					assert.Equal(t, nmPodNamespace.(string), "default")
+					nmPodNameRaw, _ := openapi.Pluck(nmPod.Outputs, "metadata", "name")
+					assert.True(t, strings.HasPrefix(nmPodNameRaw.(string), "no-metadata-pod"))
+					assert.Equal(t, nmPodNameRaw.(string), nmPodName)
+
+					// Assert the "explicit default namespace" has not changed.
+					defaultPod := stackInfo.Deployment.Resources[1]
+					assert.Equal(t, tokens.Type("kubernetes:core/v1:Pod"), defaultPod.URN.Type())
+					defaultPodNamespace, _ := openapi.Pluck(defaultPod.Outputs, "metadata", "namespace")
+					assert.Equal(t, defaultPodNamespace.(string), "default")
+					defaultPodNameRaw, _ := openapi.Pluck(defaultPod.Outputs, "metadata", "name")
+					assert.True(t, strings.HasPrefix(defaultPodNameRaw.(string), "default-ns-pod"))
+					assert.Equal(t, defaultPodNameRaw.(string), defaultPodName)
+				},
+			},
+		},
+	})
+}

tests/integration/namespace/step1/Pulumi.yaml

@@ -0,0 +1,3 @@
+name: namespace-tests
+description: Tests whether Pulumi successfully updates namespace resources.
+runtime: nodejs

tests/integration/namespace/step1/index.ts

@@ -0,0 +1,57 @@
+// Copyright 2016-2019, Pulumi Corporation.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import * as k8s from "@pulumi/kubernetes";
+
+///
+/// Create a test namespace with no specified metadata.
+///
+
+new k8s.core.v1.Namespace("test");
+
+///
+/// Create a test Pod with no metadata specified. This will be created in the "default" namespace,
+/// but the object registered with Pulumi to create will not have the .metadata.namespace field set.
+///
+
+new k8s.core.v1.Pod("no-metadata-pod", {
+    spec: {
+        containers: [
+            {
+                name: "nginx",
+                image: "nginx:1.7.9",
+                ports: [{containerPort: 80}]
+            }
+        ]
+    }
+});
+
+///
+/// Create a test Pod with the "default" namespace explicitly set.
+///
+
+new k8s.core.v1.Pod("default-ns-pod", {
+    metadata: {
+        namespace: "default"
+    },
+    spec: {
+        containers: [
+            {
+                name: "nginx",
+                image: "nginx:1.7.9",
+                ports: [{containerPort: 80}]
+            }
+        ]
+    }
+});

tests/integration/namespace/step1/package.json

@@ -0,0 +1,14 @@
+{
+    "name": "steps",
+    "version": "0.1.0",
+    "dependencies": {
+        "@pulumi/pulumi": "dev",
+        "@pulumi/random": "dev"
+    },
+    "devDependencies": {
+        "typescript": "^2.5.3"
+    },
+    "peerDependencies": {
+        "@pulumi/kubernetes": "latest"
+    }
+}

tests/integration/namespace/step1/tsconfig.json

@@ -0,0 +1,22 @@
+{
+    "compilerOptions": {
+        "outDir": "bin",
+        "target": "es6",
+        "module": "commonjs",
+        "moduleResolution": "node",
+        "declaration": true,
+        "sourceMap": true,
+        "stripInternal": true,
+        "experimentalDecorators": true,
+        "pretty": true,
+        "noFallthroughCasesInSwitch": true,
+        "noImplicitAny": true,
+        "noImplicitReturns": true,
+        "forceConsistentCasingInFileNames": true,
+        "strictNullChecks": true
+    },
+    "files": [
+        "index.ts"
+    ]
+}
+

tests/integration/namespace/step2/Pulumi.yaml

@@ -0,0 +1,3 @@
+name: namespace-tests
+description: Tests whether Pulumi successfully updates namespace resources.
+runtime: nodejs

tests/integration/namespace/step2/index.ts

@@ -0,0 +1,62 @@
+// Copyright 2016-2019, Pulumi Corporation.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import * as k8s from "@pulumi/kubernetes";
+
+///
+/// Update the test namespace by adding a label.
+///
+
+new k8s.core.v1.Namespace("test", {
+    metadata: {
+        labels: {
+            hello: "world"
+        }
+    }
+});
+
+///
+/// No change to this Pod.
+///
+
+new k8s.core.v1.Pod("no-metadata-pod", {
+    spec: {
+        containers: [
+            {
+                name: "nginx",
+                image: "nginx:1.7.9",
+                ports: [{containerPort: 80}]
+            }
+        ]
+    }
+});
+
+///
+/// No change to this Pod.
+///
+
+new k8s.core.v1.Pod("default-ns-pod", {
+    metadata: {
+        namespace: "default"
+    },
+    spec: {
+        containers: [
+            {
+                name: "nginx",
+                image: "nginx:1.7.9",
+                ports: [{containerPort: 80}]
+            }
+        ]
+    }
+});

tests/integration/namespace/step2/package.json

@@ -0,0 +1,14 @@
+{
+    "name": "steps",
+    "version": "0.1.0",
+    "dependencies": {
+        "@pulumi/pulumi": "dev",
+        "@pulumi/random": "dev"
+    },
+    "devDependencies": {
+        "typescript": "^2.5.3"
+    },
+    "peerDependencies": {
+        "@pulumi/kubernetes": "latest"
+    }
+}