Support Cluster config with a path (#2342)

* Support Cluster config with path to docker config

Fix #2330

Signed-off-by: David Gageot <david@gageot.net>

* Test skaffold.yaml files for regressions

Add a test for #2330


Signed-off-by: David Gageot <david@gageot.net>

integration/testdata/regressions/docker-config-path/skaffold.yaml

@@ -0,0 +1,10 @@
+# Test for #2330
+apiVersion: skaffold/v1beta11
+kind: Config
+build:
+  artifacts:
+  - image: image
+    kaniko: {}
+  cluster:
+    dockerConfig:
+      path: C:\Users\prashant\.docker\config.json

pkg/skaffold/schema/defaults/defaults.go

@@ -29,7 +29,6 @@ import (
 
 // Set makes sure default values are set on a SkaffoldConfig.
 func Set(c *latest.SkaffoldConfig) error {
-
 	defaultToLocalBuild(c)
 	defaultToKubectlDeploy(c)
 	setDefaultTagger(c)
@@ -207,8 +206,6 @@ func setDefaultClusterDockerConfigSecret(cluster *latest.ClusterDetails) error {
 		return nil
 	}
 
-	cluster.DockerConfig.SecretName = valueOrDefault(cluster.DockerConfig.SecretName, constants.DefaultKanikoDockerConfigSecretName)
-
 	if cluster.DockerConfig.Path != "" {
 		absPath, err := homedir.Expand(cluster.DockerConfig.Path)
 		if err != nil {
@@ -219,6 +216,8 @@ func setDefaultClusterDockerConfigSecret(cluster *latest.ClusterDetails) error {
 		return nil
 	}
 
+	cluster.DockerConfig.SecretName = valueOrDefault(cluster.DockerConfig.SecretName, constants.DefaultKanikoDockerConfigSecretName)
+
 	return nil
 }
 

pkg/skaffold/schema/defaults/defaults_test.go

@@ -61,33 +61,57 @@ func TestSetDefaults(t *testing.T) {
 }
 
 func TestSetDefaultsOnCluster(t *testing.T) {
-	testutil.Run(t, "", func(t *testutil.T) {
+	testutil.Run(t, "no docker config", func(t *testutil.T) {
 		t.SetupFakeKubernetesContext(api.Config{
 			CurrentContext: "cluster1",
 			Contexts: map[string]*api.Context{
 				"cluster1": {Namespace: "ns"},
 			},
 		})
 
+		// no docker config
 		cfg := &latest.SkaffoldConfig{
 			Pipeline: latest.Pipeline{
 				Build: latest.BuildConfig{
-					Artifacts: []*latest.Artifact{
-						{ImageName: "image"},
-					},
 					BuildType: latest.BuildType{
 						Cluster: &latest.ClusterDetails{},
 					},
 				},
 			},
 		}
-
 		err := Set(cfg)
 
 		t.CheckNoError(err)
 		t.CheckDeepEqual("ns", cfg.Build.Cluster.Namespace)
 		t.CheckDeepEqual(constants.DefaultKanikoTimeout, cfg.Build.Cluster.Timeout)
 		t.CheckDeepEqual(constants.DefaultKanikoSecretName, cfg.Build.Cluster.PullSecretName)
+
+		// default docker config
+		cfg.Pipeline.Build.BuildType.Cluster.DockerConfig = &latest.DockerConfig{}
+		err = Set(cfg)
+
+		t.CheckNoError(err)
+		t.CheckDeepEqual(constants.DefaultKanikoDockerConfigSecretName, cfg.Build.Cluster.DockerConfig.SecretName)
+
+		// docker config with path
+		cfg.Pipeline.Build.BuildType.Cluster.DockerConfig = &latest.DockerConfig{
+			Path: "/path",
+		}
+		err = Set(cfg)
+
+		t.CheckNoError(err)
+		t.CheckDeepEqual("", cfg.Build.Cluster.DockerConfig.SecretName)
+		t.CheckDeepEqual("/path", cfg.Build.Cluster.DockerConfig.Path)
+
+		// docker config with secret name
+		cfg.Pipeline.Build.BuildType.Cluster.DockerConfig = &latest.DockerConfig{
+			SecretName: "secret",
+		}
+		err = Set(cfg)
+
+		t.CheckNoError(err)
+		t.CheckDeepEqual("secret", cfg.Build.Cluster.DockerConfig.SecretName)
+		t.CheckDeepEqual("", cfg.Build.Cluster.DockerConfig.Path)
 	})
 }
 

pkg/skaffold/schema/samples_test.go

@@ -24,20 +24,28 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/validation"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
 const (
-	samplesRoot  = "../../../docs/content/en/samples"
-	examplesRoot = "../../../examples"
+	samplesRoot = "../../../docs/content/en/samples"
 )
 
 var (
 	ignoredSamples = []string{"structureTest.yaml", "build.sh"}
 )
 
+// Test that every example can be parsed and produces a valid
+// Skaffold configuration.
+func TestParseExamples(t *testing.T) {
+	parseConfigFiles(t, "../../../examples")
+	parseConfigFiles(t, "../../../integration/testdata/regressions")
+}
+
 // Samples are skaffold.yaml fragments that are used
 // in the documentation.
 func TestParseSamples(t *testing.T) {
@@ -52,37 +60,39 @@ func TestParseSamples(t *testing.T) {
 
 	for _, path := range paths {
 		name := filepath.Base(path)
+		if util.StrSliceContains(ignoredSamples, name) {
+			continue
+		}
 
 		testutil.Run(t, name, func(t *testutil.T) {
-			for _, is := range ignoredSamples {
-				if name == is {
-					t.Skip()
-				}
-			}
-
 			buf, err := ioutil.ReadFile(path)
 			t.CheckNoError(err)
 
-			configFile := t.TempFile("skaffold.yaml", addHeader(buf))
-			cfg, err := ParseConfig(configFile, true)
-			t.CheckNoError(err)
-
-			err = validation.Process(cfg.(*latest.SkaffoldConfig))
-			t.CheckNoError(err)
+			checkSkaffoldConfig(t, addHeader(buf))
 		})
 	}
 }
 
-// TestParseExamples complete skaffold.yaml that user can use
-// with the latest release of Skaffold.
-func TestParseExamples(t *testing.T) {
-	paths, err := findExamples(examplesRoot)
+func checkSkaffoldConfig(t *testutil.T, yaml []byte) {
+	configFile := t.TempFile("skaffold.yaml", yaml)
+	cfg, err := ParseConfig(configFile, true)
+	t.CheckNoError(err)
+
+	err = defaults.Set(cfg.(*latest.SkaffoldConfig))
+	t.CheckNoError(err)
+
+	err = validation.Process(cfg.(*latest.SkaffoldConfig))
+	t.CheckNoError(err)
+}
+
+func parseConfigFiles(t *testing.T, root string) {
+	paths, err := findExamples(root)
 	if err != nil {
-		t.Fatalf("unable to list examples in %q", examplesRoot)
+		t.Fatalf("unable to list skaffold configuration files in %q", root)
 	}
 
 	if len(paths) == 0 {
-		t.Fatalf("did not find examples in %q", examplesRoot)
+		t.Fatalf("did not find skaffold configuration files in %q", root)
 	}
 
 	for _, path := range paths {
@@ -92,9 +102,7 @@ func TestParseExamples(t *testing.T) {
 			buf, err := ioutil.ReadFile(path)
 			t.CheckNoError(err)
 
-			configFile := t.TempFile("skaffold.yaml", buf)
-			_, err = ParseConfig(configFile, true)
-			t.CheckNoError(err)
+			checkSkaffoldConfig(t, buf)
 		})
 	}
 }