diff --git a/pkg/skaffold/initializer/analyze.go b/pkg/skaffold/initializer/analyze.go index 62408c19945..8709214f738 100644 --- a/pkg/skaffold/initializer/analyze.go +++ b/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, }, } } diff --git a/pkg/skaffold/initializer/analyze_test.go b/pkg/skaffold/initializer/analyze_test.go index ec9c76a3e9d..a272e4f5b2f 100644 --- a/pkg/skaffold/initializer/analyze_test.go +++ b/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, diff --git a/pkg/skaffold/initializer/config.go b/pkg/skaffold/initializer/config.go index a90bcf1b7cb..b0d993a2ab6 100644 --- a/pkg/skaffold/initializer/config.go +++ b/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 { diff --git a/pkg/skaffold/initializer/config_test.go b/pkg/skaffold/initializer/config_test.go index a2ae4f38941..954f8ac79bd 100644 --- a/pkg/skaffold/initializer/config_test.go +++ b/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 diff --git a/pkg/skaffold/initializer/init_test.go b/pkg/skaffold/initializer/init_test.go index 2b1db807623..f428bc1e478 100644 --- a/pkg/skaffold/initializer/init_test.go +++ b/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, }, diff --git a/testutil/util.go b/testutil/util.go index 3e4fcee844c..ac06527999e 100644 --- a/testutil/util.go +++ b/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()