no error in skaffold init if pre-existing skaffold.yaml is different from target file

pkg/skaffold/initializer/analyze.go

@@ -47,7 +47,9 @@ func newAnalysis(c Config) *analysis {
 		kubectlAnalyzer: &kubectlAnalyzer{},
 		builderAnalyzer: builders,
 		skaffoldAnalyzer: &skaffoldConfigAnalyzer{
-			force: c.Force,
+			force:        c.Force,
+			analyzeMode:  c.Analyze,
+			targetConfig: c.Opts.ConfigurationFile,
 		},
 	}
 }

pkg/skaffold/initializer/analyze_test.go

@@ -20,6 +20,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/jib"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/testutil"
@@ -219,6 +221,9 @@ deploy:
 				Force:               false,
 				EnableBuildpackInit: false,
 				EnableJibInit:       true,
+				Opts: config.SkaffoldOptions{
+					ConfigurationFile: "skaffold.yaml",
+				},
 			},
 			expectedConfigs: nil,
 			expectedPaths:   nil,
@@ -240,6 +245,9 @@ deploy:
 				Force:               false,
 				EnableBuildpackInit: false,
 				EnableJibInit:       true,
+				Opts: config.SkaffoldOptions{
+					ConfigurationFile: "skaffold.yaml",
+				},
 			},
 			expectedConfigs: nil,
 			expectedPaths:   nil,

pkg/skaffold/initializer/config.go

@@ -37,18 +37,35 @@ var (
 
 type skaffoldConfigAnalyzer struct {
 	directoryAnalyzer
-	force bool
+	force        bool
+	analyzeMode  bool
+	targetConfig string
 }
 
 func (a *skaffoldConfigAnalyzer) analyzeFile(filePath string) error {
-	if !isSkaffoldConfig(filePath) {
+	if !isSkaffoldConfig(filePath) || a.force || a.analyzeMode {
 		return nil
 	}
-	if !a.force {
-		return fmt.Errorf("pre-existing %s found (you may continue with --force)", filePath)
+	sameFiles, err := sameFiles(filePath, a.targetConfig)
+	if err != nil {
+		return fmt.Errorf("failed to analyze file %s: %s", filePath, err)
+	}
+	if !sameFiles {
+		return nil
+	}
+	return fmt.Errorf("pre-existing %s found (you may continue with --force)", filePath)
+}
+
+func sameFiles(a, b string) (bool, error) {
+	absA, err := filepath.Abs(a)
+	if err != nil {
+		return false, err
+	}
+	absB, err := filepath.Abs(b)
+	if err != nil {
+		return false, err
 	}
-	logrus.Debugf("%s is a valid skaffold configuration: continuing since --force=true", filePath)
-	return nil
+	return absA == absB, nil
 }
 
 func generateSkaffoldConfig(k deploymentInitializer, buildConfigPairs []builderImagePair) *latest.SkaffoldConfig {

pkg/skaffold/initializer/config_test.go

@@ -39,6 +39,64 @@ func (s stubDeploymentInitializer) GetImages() []string {
 	panic("implement me")
 }
 
+func TestConfigAnalyzer(t *testing.T) {
+	tests := []struct {
+		name      string
+		inputFile string
+		analyzer  skaffoldConfigAnalyzer
+		shouldErr bool
+	}{
+		{
+			name:      "not skaffold config",
+			inputFile: "testdata/init/hello/main.go",
+			analyzer:  skaffoldConfigAnalyzer{},
+			shouldErr: false,
+		},
+		{
+			name:      "skaffold config equals target config",
+			inputFile: "testdata/init/hello/skaffold.yaml",
+			analyzer: skaffoldConfigAnalyzer{
+				targetConfig: "testdata/init/hello/skaffold.yaml",
+			},
+			shouldErr: true,
+		},
+		{
+			name:      "skaffold config does not equal target config",
+			inputFile: "testdata/init/hello/skaffold.yaml",
+			analyzer: skaffoldConfigAnalyzer{
+				targetConfig: "testdata/init/hello/skaffold.yaml.out",
+			},
+			shouldErr: false,
+		},
+		{
+			name:      "force overrides",
+			inputFile: "testdata/init/hello/skaffold.yaml",
+			analyzer: skaffoldConfigAnalyzer{
+				force:        true,
+				targetConfig: "testdata/init/hello/skaffold.yaml",
+			},
+			shouldErr: false,
+		},
+		{
+			name:      "analyze mode can skip writing, no error",
+			inputFile: "testdata/init/hello/skaffold.yaml",
+			analyzer: skaffoldConfigAnalyzer{
+				force:        false,
+				analyzeMode:  true,
+				targetConfig: testutil.Abs(t, "testdata/init/hello/skaffold.yaml"),
+			},
+			shouldErr: false,
+		},
+	}
+
+	for _, test := range tests {
+		testutil.Run(t, test.name, func(t *testutil.T) {
+			err := test.analyzer.analyzeFile(test.inputFile)
+			t.CheckError(test.shouldErr, err)
+		})
+	}
+}
+
 func TestGenerateSkaffoldConfig(t *testing.T) {
 	tests := []struct {
 		name                   string

pkg/skaffold/initializer/init_test.go

@@ -42,7 +42,6 @@ func TestDoInit(t *testing.T) {
 			name: "getting-started",
 			dir:  "testdata/init/hello",
 			config: Config{
-				Force: true,
 				Opts: config.SkaffoldOptions{
 					ConfigurationFile: "skaffold.yaml.out",
 				},
@@ -52,7 +51,6 @@ func TestDoInit(t *testing.T) {
 			name: "ignore existing tags",
 			dir:  "testdata/init/ignore-tags",
 			config: Config{
-				Force: true,
 				Opts: config.SkaffoldOptions{
 					ConfigurationFile: "skaffold.yaml.out",
 				},
@@ -62,7 +60,6 @@ func TestDoInit(t *testing.T) {
 			name: "microservices (backwards compatibility)",
 			dir:  "testdata/init/microservices",
 			config: Config{
-				Force: true,
 				CliArtifacts: []string{
 					"leeroy-app/Dockerfile=gcr.io/k8s-skaffold/leeroy-app",
 					"leeroy-web/Dockerfile=gcr.io/k8s-skaffold/leeroy-web",
@@ -76,7 +73,6 @@ func TestDoInit(t *testing.T) {
 			name: "microservices",
 			dir:  "testdata/init/microservices",
 			config: Config{
-				Force: true,
 				CliArtifacts: []string{
 					`{"builder":"Docker","payload":{"path":"leeroy-app/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-app"}`,
 					`{"builder":"Docker","payload":{"path":"leeroy-web/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-web"}`,
@@ -91,7 +87,6 @@ func TestDoInit(t *testing.T) {
 			dir:  "testdata/init/microservices",
 
 			config: Config{
-				Force: true,
 				CliArtifacts: []string{
 					`{"builder":"Docker","payload":{"path":"leeroy-app/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-app"}`,
 					`{"builder":"Docker","payload":{"path":"leeroy-web/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-web"}`,
@@ -107,6 +102,8 @@ func TestDoInit(t *testing.T) {
 	for _, test := range tests {
 		testutil.Run(t, test.name, func(t *testutil.T) {
 			t.Chdir(test.dir)
+			// we still need as a "no-prompt" mode
+			test.config.Force = true
 			err := DoInit(context.TODO(), os.Stdout, test.config)
 			t.CheckError(test.shouldErr, err)
 			checkGeneratedConfig(t, ".")
@@ -126,7 +123,6 @@ func TestDoInitAnalyze(t *testing.T) {
 			name: "analyze microservices",
 			dir:  "testdata/init/microservices",
 			config: Config{
-				Force:   true,
 				Analyze: true,
 			},
 			expectedOut: strip(`{
@@ -138,7 +134,6 @@ func TestDoInitAnalyze(t *testing.T) {
 			name: "analyze microservices new format",
 			dir:  "testdata/init/microservices",
 			config: Config{
-				Force:         true,
 				Analyze:       true,
 				EnableJibInit: true,
 			},

testutil/util.go

@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"path/filepath"
 	"reflect"
 	"regexp"
 	"strings"
@@ -201,6 +202,14 @@ func (t *T) Chdir(dir string) {
 	})
 }
 
+func Abs(t *testing.T, path string) string {
+	absPath, err := filepath.Abs(path)
+	if err != nil {
+		t.Fatalf("Failed to get absolute path for file %s: %s", path, absPath)
+	}
+	return absPath
+}
+
 func Run(t *testing.T, name string, f func(t *T)) {
 	if name == "" {
 		name = t.Name()