fix: fix issue with templated helm namespaces when querying non-existent namespace

Author: aaron-prindle

pkg/skaffold/deploy/util/namespaces.go

@@ -22,6 +22,7 @@ import (
 
 	kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
 	latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
 // GetAllPodNamespaces lists the namespaces that should be watched.
@@ -49,7 +50,11 @@ func GetAllPodNamespaces(configNamespace string, pipelines []latestV1.Pipeline)
 	}
 
 	// Set additional namespaces each helm release referenced
-	for _, namespace := range collectHelmReleasesNamespaces(pipelines) {
+	helmReleasesNamespaces, err := collectHelmReleasesNamespaces(pipelines)
+	if err != nil {
+		return nil, fmt.Errorf("collecting helm releases namespaces: %w", err)
+	}
+	for _, namespace := range helmReleasesNamespaces {
 		nsMap[namespace] = true
 	}
 
@@ -63,16 +68,20 @@ func GetAllPodNamespaces(configNamespace string, pipelines []latestV1.Pipeline)
 	return namespaces, nil
 }
 
-func collectHelmReleasesNamespaces(pipelines []latestV1.Pipeline) []string {
+func collectHelmReleasesNamespaces(pipelines []latestV1.Pipeline) ([]string, error) {
 	var namespaces []string
 	for _, cfg := range pipelines {
 		if cfg.Deploy.HelmDeploy != nil {
 			for _, release := range cfg.Deploy.HelmDeploy.Releases {
 				if release.Namespace != "" {
-					namespaces = append(namespaces, release.Namespace)
+					templatedNamespace, err := util.ExpandEnvTemplateOrFail(release.Namespace, nil)
+					if err != nil {
+						return []string{}, fmt.Errorf("cannot parse the release namespace template: %w", err)
+					}
+					namespaces = append(namespaces, templatedNamespace)
 				}
 			}
 		}
 	}
-	return namespaces
+	return namespaces, nil
 }

pkg/skaffold/deploy/util/namespaces_test.go

@@ -0,0 +1,96 @@
+/*
+Copyright 2019 The Skaffold Authors
+
+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 util
+
+import (
+	"testing"
+
+	latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestCollectHelmReleasesNamespaces(t *testing.T) {
+	tests := []struct {
+		description  string
+		helmReleases []latestV1.HelmRelease
+		env          []string
+		expected     []string
+		shouldErr    bool
+	}{
+		{
+			description: "namspaces are collected correctly",
+			helmReleases: []latestV1.HelmRelease{
+				{
+					Namespace: "foo",
+				},
+				{
+					Namespace: "bar",
+				},
+				{
+					Namespace: "baz",
+				},
+			},
+			expected: []string{"foo", "bar", "baz"},
+		},
+		{
+			description: "namespaces are collected correctly with env expansion",
+			helmReleases: []latestV1.HelmRelease{
+				{
+					Namespace: "{{.FOO}}",
+				},
+				{
+					Namespace: "bar",
+				},
+				{
+					Namespace: "baz",
+				},
+			},
+			env:      []string{"FOO=foo"},
+			expected: []string{"foo", "bar", "baz"},
+		},
+		{
+			description: "should error when template expansion fails",
+			helmReleases: []latestV1.HelmRelease{
+				{
+					Namespace: "{{.DOESNT_EXIST_AND_SHOULD_ERROR_AS_SUCH}}",
+				},
+			},
+			shouldErr: true,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.Override(&util.OSEnviron, func() []string { return test.env })
+			ns, err := collectHelmReleasesNamespaces([]latestV1.Pipeline{
+				{
+					Deploy: latestV1.DeployConfig{
+						DeployType: latestV1.DeployType{
+							HelmDeploy: &latestV1.HelmDeploy{
+								Releases: test.helmReleases,
+							},
+						},
+					},
+				},
+			})
+			t.CheckError(test.shouldErr, err)
+			if !test.shouldErr {
+				t.CheckDeepEqual(test.expected, ns)
+			}
+		})
+	}
+}