feat: add better support and messaging around using helm with skaffold apply

Author: aaron-prindle

pkg/skaffold/runner/deployer.go

@@ -66,11 +66,45 @@ func (d *deployerCtx) JSONParseConfig() v1.JSONParseConfig {
 
 // GetDeployer creates a deployer from a given RunContext and deploy pipeline definitions.
 func GetDeployer(ctx context.Context, runCtx *runcontext.RunContext, labeller *label.DefaultLabeller) (deploy.Deployer, error) {
+	deployerCfg := runCtx.Deployers()
+
 	if runCtx.Opts.Apply {
+		// helmNamespaceValue := ""
+		helmNamespaces := make(map[string]bool)
+		// helmReleasesWithDiffNamespacesFound := false
+		nonHelmDeployFound := false
+
+		for _, d := range deployerCfg {
+			if d.DockerDeploy != nil || d.KptDeploy != nil || d.KubectlDeploy != nil || d.KustomizeDeploy != nil {
+				nonHelmDeployFound = true
+			}
+
+			if d.HelmDeploy != nil {
+				for _, release := range d.HelmDeploy.Releases {
+					if release.Namespace != "" {
+						helmNamespaces[release.Namespace] = true
+					}
+				}
+			}
+		}
+
+		if len(helmNamespaces) > 1 || (nonHelmDeployFound && len(helmNamespaces) == 1) {
+			return nil, errors.New("skaffold apply called with conflicting namespaces set via skaffold.yaml. This is likely due to the use of the 'deploy.helm.releases.*.namespace' field which is not supported in apply.  Remove the 'deploy.helm.releases.*.namespace' field(s) and run skaffold apply again")
+		}
+
+		if len(helmNamespaces) == 1 && !nonHelmDeployFound {
+			if runCtx.Opts.Namespace == "" {
+				// if skaffold --namespace flag not set, use the helm namespace value
+				for k := range helmNamespaces {
+					// map only has 1 (k,v) from length check in if condition
+					runCtx.Opts.Namespace = k
+				}
+			}
+		}
+
 		return getDefaultDeployer(runCtx, labeller)
 	}
 
-	deployerCfg := runCtx.Deployers()
 	localDeploy := false
 	remoteDeploy := false
 

pkg/skaffold/runner/deployer_test.go

@@ -133,6 +133,59 @@ func TestGetDeployer(tOuter *testing.T) {
 					&kpt.Deployer{},
 				}, false),
 			},
+			{
+				description: "apply does not allow multiple deployers when a helm namespace is set",
+				apply:       true,
+				cfg: latestV1.DeployType{
+					HelmDeploy: &latestV1.HelmDeploy{
+						Releases: []latestV1.HelmRelease{
+							{
+								Namespace: "foo",
+							},
+						},
+					},
+					KubectlDeploy: &latestV1.KubectlDeploy{},
+				},
+				shouldErr: true,
+			},
+			{
+				description: "apply does not allow multiple helm releases with different namespaces set",
+				apply:       true,
+				cfg: latestV1.DeployType{
+					HelmDeploy: &latestV1.HelmDeploy{
+						Releases: []latestV1.HelmRelease{
+							{
+								Namespace: "foo",
+							},
+							{
+								Namespace: "bar",
+							},
+						},
+					},
+				},
+				shouldErr: true,
+			},
+			{
+				description: "apply does allow multiple helm releases with the same namespace set",
+				apply:       true,
+				cfg: latestV1.DeployType{
+					HelmDeploy: &latestV1.HelmDeploy{
+						Releases: []latestV1.HelmRelease{
+							{
+								Namespace: "foo",
+							},
+							{
+								Namespace: "foo",
+							},
+						},
+					},
+				},
+				expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{
+					Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}),
+				}, &label.DefaultLabeller{}, &latestV1.KubectlDeploy{
+					Flags: latestV1.KubectlFlags{},
+				})).(deploy.Deployer),
+			},
 		}
 		for _, test := range tests {
 			testutil.Run(tOuter, test.description, func(t *testutil.T) {