From 4408cbca30abb0ceefe85148cc3737965595d585 Mon Sep 17 00:00:00 2001 From: balopat Date: Fri, 24 Jan 2020 04:54:15 -0800 Subject: [PATCH 1/4] move e2e unit tests to initializer package from cmd + add cmd/init tests --- cmd/skaffold/app/cmd/init.go | 5 +- cmd/skaffold/app/cmd/init_test.go | 165 ++++++----------- pkg/skaffold/initializer/init_test.go | 169 ++++++++++++++++++ .../initializer}/testdata/init/.gitignore | 0 .../testdata/init/hello/Dockerfile | 0 .../testdata/init/hello/k8s-pod.yaml | 0 .../initializer}/testdata/init/hello/main.go | 0 .../testdata/init/hello/skaffold.yaml | 0 .../testdata/init/ignore-tags/Dockerfile | 0 .../testdata/init/ignore-tags/k8s-pod.yaml | 0 .../testdata/init/ignore-tags/main.go | 0 .../testdata/init/ignore-tags/skaffold.yaml | 0 .../init/microservices/leeroy-app/Dockerfile | 0 .../init/microservices/leeroy-app/app.go | 0 .../leeroy-app/kubernetes/deployment.yaml | 0 .../init/microservices/leeroy-web/Dockerfile | 0 .../leeroy-web/kubernetes/deployment.yaml | 0 .../init/microservices/leeroy-web/web.go | 0 .../testdata/init/microservices/skaffold.yaml | 0 19 files changed, 224 insertions(+), 115 deletions(-) create mode 100644 pkg/skaffold/initializer/init_test.go rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/.gitignore (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/hello/Dockerfile (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/hello/k8s-pod.yaml (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/hello/main.go (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/hello/skaffold.yaml (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/ignore-tags/Dockerfile (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/ignore-tags/k8s-pod.yaml (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/ignore-tags/main.go (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/ignore-tags/skaffold.yaml (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/microservices/leeroy-app/Dockerfile (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/microservices/leeroy-app/app.go (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/microservices/leeroy-app/kubernetes/deployment.yaml (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/microservices/leeroy-web/Dockerfile (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/microservices/leeroy-web/kubernetes/deployment.yaml (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/microservices/leeroy-web/web.go (100%) rename {cmd/skaffold/app/cmd => pkg/skaffold/initializer}/testdata/init/microservices/skaffold.yaml (100%) diff --git a/cmd/skaffold/app/cmd/init.go b/cmd/skaffold/app/cmd/init.go index 8345e7f1733..9b0a20aac63 100644 --- a/cmd/skaffold/app/cmd/init.go +++ b/cmd/skaffold/app/cmd/init.go @@ -36,6 +36,9 @@ var ( enableBuildpackInit bool ) +// for testing +var initEntrypoint = initializer.DoInit + // NewCmdInit describes the CLI command to generate a Skaffold configuration. func NewCmdInit() *cobra.Command { return NewCmd("init"). @@ -56,7 +59,7 @@ func NewCmdInit() *cobra.Command { } func doInit(ctx context.Context, out io.Writer) error { - return initializer.DoInit(ctx, out, initializer.Config{ + return initEntrypoint(ctx, out, initializer.Config{ ComposeFile: composeFile, CliArtifacts: cliArtifacts, SkipBuild: skipBuild, diff --git a/cmd/skaffold/app/cmd/init_test.go b/cmd/skaffold/app/cmd/init_test.go index 4bae645dba4..cc89ee68687 100644 --- a/cmd/skaffold/app/cmd/init_test.go +++ b/cmd/skaffold/app/cmd/init_test.go @@ -17,142 +17,79 @@ limitations under the License. package cmd import ( - "bytes" - "fmt" + "context" + "errors" + "io" "os" - "path/filepath" - "strings" "testing" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/initializer" + "github.com/GoogleContainerTools/skaffold/testutil" ) -func TestInit(t *testing.T) { +func TestFlagsToConfigVersion(t *testing.T) { tests := []struct { - name string - dir string - args []string - config string - shouldErr bool + name string + args []string + expectedConfig initializer.Config + initResult error + shouldErr bool }{ - //TODO: mocked kompose test - { - name: "getting-started", - dir: "testdata/init/hello", - config: "skaffold.yaml.out", - }, - { - name: "ignore existing tags", - dir: "testdata/init/ignore-tags", - config: "skaffold.yaml.out", - }, { - name: "microservices (backwards compatibility)", - dir: "testdata/init/microservices", - config: "skaffold.yaml.out", - args: []string{ - "-a", `leeroy-app/Dockerfile=gcr.io/k8s-skaffold/leeroy-app`, - "-a", `leeroy-web/Dockerfile=gcr.io/k8s-skaffold/leeroy-web`, + name: "error + default values of flags mapped to config values", + initResult: errors.New("test error"), + shouldErr: true, + expectedConfig: initializer.Config{ + ComposeFile: "", + CliArtifacts: nil, + SkipBuild: false, + Force: false, + Analyze: false, + EnableJibInit: false, + EnableBuildpackInit: false, + Opts: opts, }, }, { - name: "microservices", - dir: "testdata/init/microservices", - config: "skaffold.yaml.out", + name: "no error + non-default values for flags mapped to config values", args: []string{ - "-a", `{"builder":"Docker","payload":{"path":"leeroy-app/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-app"}`, - "-a", `{"builder":"Docker","payload":{"path":"leeroy-web/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-web"}`, + "--compose-file=a-compose-file", + "--artifact", "a1=b1", + "-a", "a2=b2", + "--skip-build", + "--force", + "--analyze", + "--XXenableJibInit", + "--XXenableBuildpackInit", }, - }, - { - name: "error writing config file", - dir: "testdata/init/microservices", - // erroneous config file as . is a directory - config: ".", - args: []string{ - "-a", `{"builder":"Docker","payload":{"path":"leeroy-app/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-app"}`, - "-a", `{"builder":"Docker","payload":{"path":"leeroy-web/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-web"}`, + expectedConfig: initializer.Config{ + ComposeFile: "a-compose-file", + CliArtifacts: []string{"a1=b1", "a2=b2"}, + SkipBuild: true, + Force: true, + Analyze: true, + EnableJibInit: true, + EnableBuildpackInit: true, + Opts: opts, }, - shouldErr: true, - }, - } - for _, test := range tests { - testutil.Run(t, test.name, func(t *testutil.T) { - initArgs := append([]string{"init", "--force", "-f", test.config}, test.args...) - os.Args = initArgs - t.Chdir(test.dir) - init := NewCmdInit() - err := init.Execute() - t.CheckError(test.shouldErr, err) - checkGeneratedConfig(t, ".") - }) - } -} - -func TestAnalyze(t *testing.T) { - tests := []struct { - name string - dir string - args []string - expectedOut string - shouldErr bool - }{ - { - name: "analyze microservices", - dir: "testdata/init/microservices", - args: []string{"--analyze"}, - expectedOut: strip(`{ - "dockerfiles":["leeroy-app/Dockerfile","leeroy-web/Dockerfile"], - "images":["gcr.io/k8s-skaffold/leeroy-app","gcr.io/k8s-skaffold/leeroy-web"] - }`), - }, - { - name: "analyze microservices new format", - dir: "testdata/init/microservices", - args: []string{"--analyze", "--XXenableJibInit"}, - expectedOut: strip(`{ - "builders":[ - {"name":"Docker","payload":{"path":"leeroy-app/Dockerfile"}}, - {"name":"Docker","payload":{"path":"leeroy-web/Dockerfile"}} - ], - "images":[ - {"name":"gcr.io/k8s-skaffold/leeroy-app","foundMatch":false}, - {"name":"gcr.io/k8s-skaffold/leeroy-web","foundMatch":false}]}`), }, } - for _, test := range tests { testutil.Run(t, test.name, func(t *testutil.T) { - var out bytes.Buffer - initArgs := append([]string{"init", "--force"}, test.args...) + var capturedConfig initializer.Config + mockFunc := func(ctx context.Context, out io.Writer, c initializer.Config) error { + capturedConfig = c + return test.initResult + } + t.Override(&initEntrypoint, mockFunc) + initArgs := append([]string{"init"}, test.args...) os.Args = initArgs - t.Chdir(test.dir) init := NewCmdInit() - init.SetOut(&out) err := init.Execute() - t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedOut, out.String()) + // we ignore Skaffold options + test.expectedConfig.Opts = capturedConfig.Opts + t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedConfig, capturedConfig) }) } } - -func strip(s string) string { - cutString := "\n\t\r" - stripped := "" - for _, r := range s { - if strings.ContainsRune(cutString, r) { - continue - } - stripped = fmt.Sprintf("%s%c", stripped, r) - } - return stripped -} - -func checkGeneratedConfig(t *testutil.T, dir string) { - expectedOutput, err := schema.ParseConfig(filepath.Join(dir, "skaffold.yaml"), false) - t.CheckNoError(err) - - output, err := schema.ParseConfig(filepath.Join(dir, "skaffold.yaml.out"), false) - t.CheckNoError(err) - t.CheckDeepEqual(expectedOutput, output) -} diff --git a/pkg/skaffold/initializer/init_test.go b/pkg/skaffold/initializer/init_test.go new file mode 100644 index 00000000000..fd524742f0b --- /dev/null +++ b/pkg/skaffold/initializer/init_test.go @@ -0,0 +1,169 @@ +package initializer + +import ( + "bytes" + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema" + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestDoInit(t *testing.T) { + tests := []struct { + name string + dir string + config Config + shouldErr bool + }{ + //TODO: mocked kompose test + { + name: "getting-started", + dir: "testdata/init/hello", + config: Config{ + Force: true, + Opts: config.SkaffoldOptions{ + ConfigurationFile: "skaffold.yaml.out", + }, + }, + }, + { + name: "ignore existing tags", + dir: "testdata/init/ignore-tags", + config: Config{ + Force: true, + Opts: config.SkaffoldOptions{ + ConfigurationFile: "skaffold.yaml.out", + }, + }, + }, + { + 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", + }, + Opts: config.SkaffoldOptions{ + ConfigurationFile: "skaffold.yaml.out", + }, + }, + }, + { + 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"}`, + }, + Opts: config.SkaffoldOptions{ + ConfigurationFile: "skaffold.yaml.out", + }, + }, + }, + { + name: "error writing config file", + 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"}`, + }, + Opts: config.SkaffoldOptions{ + // erroneous config file as . is a directory + ConfigurationFile: ".", + }, + }, + shouldErr: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.name, func(t *testutil.T) { + t.Chdir(test.dir) + err := DoInit(context.TODO(), os.Stdout, test.config) + t.CheckError(test.shouldErr, err) + checkGeneratedConfig(t, ".") + }) + } +} + +func TestDoInitAnalyze(t *testing.T) { + tests := []struct { + name string + dir string + config Config + expectedOut string + shouldErr bool + }{ + { + name: "analyze microservices", + dir: "testdata/init/microservices", + config: Config{ + Force: true, + Analyze: true, + }, + expectedOut: strip(`{ + "dockerfiles":["leeroy-app/Dockerfile","leeroy-web/Dockerfile"], + "images":["gcr.io/k8s-skaffold/leeroy-app","gcr.io/k8s-skaffold/leeroy-web"] + }`), + }, + { + name: "analyze microservices new format", + dir: "testdata/init/microservices", + config: Config{ + Force: true, + Analyze: true, + EnableJibInit: true, + }, + expectedOut: strip(`{ + "builders":[ + {"name":"Docker","payload":{"path":"leeroy-app/Dockerfile"}}, + {"name":"Docker","payload":{"path":"leeroy-web/Dockerfile"}} + ], + "images":[ + {"name":"gcr.io/k8s-skaffold/leeroy-app","foundMatch":false}, + {"name":"gcr.io/k8s-skaffold/leeroy-web","foundMatch":false}]}`), + }, + } + + for _, test := range tests { + testutil.Run(t, test.name, func(t *testutil.T) { + var out bytes.Buffer + t.Chdir(test.dir) + err := DoInit(context.TODO(), &out, test.config) + t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedOut, out.String()) + }) + } +} + +func strip(s string) string { + cutString := "\n\t\r" + stripped := "" + for _, r := range s { + if strings.ContainsRune(cutString, r) { + continue + } + stripped = fmt.Sprintf("%s%c", stripped, r) + } + return stripped +} + +func checkGeneratedConfig(t *testutil.T, dir string) { + expectedOutput, err := schema.ParseConfig(filepath.Join(dir, "skaffold.yaml"), false) + t.CheckNoError(err) + + output, err := schema.ParseConfig(filepath.Join(dir, "skaffold.yaml.out"), false) + t.CheckNoError(err) + t.CheckDeepEqual(expectedOutput, output) +} diff --git a/cmd/skaffold/app/cmd/testdata/init/.gitignore b/pkg/skaffold/initializer/testdata/init/.gitignore similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/.gitignore rename to pkg/skaffold/initializer/testdata/init/.gitignore diff --git a/cmd/skaffold/app/cmd/testdata/init/hello/Dockerfile b/pkg/skaffold/initializer/testdata/init/hello/Dockerfile similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/hello/Dockerfile rename to pkg/skaffold/initializer/testdata/init/hello/Dockerfile diff --git a/cmd/skaffold/app/cmd/testdata/init/hello/k8s-pod.yaml b/pkg/skaffold/initializer/testdata/init/hello/k8s-pod.yaml similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/hello/k8s-pod.yaml rename to pkg/skaffold/initializer/testdata/init/hello/k8s-pod.yaml diff --git a/cmd/skaffold/app/cmd/testdata/init/hello/main.go b/pkg/skaffold/initializer/testdata/init/hello/main.go similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/hello/main.go rename to pkg/skaffold/initializer/testdata/init/hello/main.go diff --git a/cmd/skaffold/app/cmd/testdata/init/hello/skaffold.yaml b/pkg/skaffold/initializer/testdata/init/hello/skaffold.yaml similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/hello/skaffold.yaml rename to pkg/skaffold/initializer/testdata/init/hello/skaffold.yaml diff --git a/cmd/skaffold/app/cmd/testdata/init/ignore-tags/Dockerfile b/pkg/skaffold/initializer/testdata/init/ignore-tags/Dockerfile similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/ignore-tags/Dockerfile rename to pkg/skaffold/initializer/testdata/init/ignore-tags/Dockerfile diff --git a/cmd/skaffold/app/cmd/testdata/init/ignore-tags/k8s-pod.yaml b/pkg/skaffold/initializer/testdata/init/ignore-tags/k8s-pod.yaml similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/ignore-tags/k8s-pod.yaml rename to pkg/skaffold/initializer/testdata/init/ignore-tags/k8s-pod.yaml diff --git a/cmd/skaffold/app/cmd/testdata/init/ignore-tags/main.go b/pkg/skaffold/initializer/testdata/init/ignore-tags/main.go similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/ignore-tags/main.go rename to pkg/skaffold/initializer/testdata/init/ignore-tags/main.go diff --git a/cmd/skaffold/app/cmd/testdata/init/ignore-tags/skaffold.yaml b/pkg/skaffold/initializer/testdata/init/ignore-tags/skaffold.yaml similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/ignore-tags/skaffold.yaml rename to pkg/skaffold/initializer/testdata/init/ignore-tags/skaffold.yaml diff --git a/cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-app/Dockerfile b/pkg/skaffold/initializer/testdata/init/microservices/leeroy-app/Dockerfile similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-app/Dockerfile rename to pkg/skaffold/initializer/testdata/init/microservices/leeroy-app/Dockerfile diff --git a/cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-app/app.go b/pkg/skaffold/initializer/testdata/init/microservices/leeroy-app/app.go similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-app/app.go rename to pkg/skaffold/initializer/testdata/init/microservices/leeroy-app/app.go diff --git a/cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-app/kubernetes/deployment.yaml b/pkg/skaffold/initializer/testdata/init/microservices/leeroy-app/kubernetes/deployment.yaml similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-app/kubernetes/deployment.yaml rename to pkg/skaffold/initializer/testdata/init/microservices/leeroy-app/kubernetes/deployment.yaml diff --git a/cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-web/Dockerfile b/pkg/skaffold/initializer/testdata/init/microservices/leeroy-web/Dockerfile similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-web/Dockerfile rename to pkg/skaffold/initializer/testdata/init/microservices/leeroy-web/Dockerfile diff --git a/cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-web/kubernetes/deployment.yaml b/pkg/skaffold/initializer/testdata/init/microservices/leeroy-web/kubernetes/deployment.yaml similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-web/kubernetes/deployment.yaml rename to pkg/skaffold/initializer/testdata/init/microservices/leeroy-web/kubernetes/deployment.yaml diff --git a/cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-web/web.go b/pkg/skaffold/initializer/testdata/init/microservices/leeroy-web/web.go similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/microservices/leeroy-web/web.go rename to pkg/skaffold/initializer/testdata/init/microservices/leeroy-web/web.go diff --git a/cmd/skaffold/app/cmd/testdata/init/microservices/skaffold.yaml b/pkg/skaffold/initializer/testdata/init/microservices/skaffold.yaml similarity index 100% rename from cmd/skaffold/app/cmd/testdata/init/microservices/skaffold.yaml rename to pkg/skaffold/initializer/testdata/init/microservices/skaffold.yaml From b3be7b4b7a57dc01da6701c4f9f6d1a408a42943 Mon Sep 17 00:00:00 2001 From: balopat Date: Fri, 24 Jan 2020 05:55:33 -0800 Subject: [PATCH 2/4] boilerplate --- cmd/skaffold/app/cmd/init_test.go | 2 +- pkg/skaffold/initializer/init_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cmd/skaffold/app/cmd/init_test.go b/cmd/skaffold/app/cmd/init_test.go index cc89ee68687..06529708538 100644 --- a/cmd/skaffold/app/cmd/init_test.go +++ b/cmd/skaffold/app/cmd/init_test.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Skaffold Authors +Copyright 2020 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. diff --git a/pkg/skaffold/initializer/init_test.go b/pkg/skaffold/initializer/init_test.go index fd524742f0b..2b1db807623 100644 --- a/pkg/skaffold/initializer/init_test.go +++ b/pkg/skaffold/initializer/init_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2020 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 initializer import ( From bdb90f2a6de33fdd82e1042d16e1b13e1bb71cc1 Mon Sep 17 00:00:00 2001 From: balopat Date: Fri, 24 Jan 2020 05:54:22 -0800 Subject: [PATCH 3/4] no need for --force for certain cases --- pkg/skaffold/initializer/analyze.go | 4 +- pkg/skaffold/initializer/analyze_test.go | 8 ++++ pkg/skaffold/initializer/config.go | 29 +++++++++--- pkg/skaffold/initializer/config_test.go | 58 ++++++++++++++++++++++++ pkg/skaffold/initializer/init_test.go | 9 +--- testutil/util.go | 9 ++++ 6 files changed, 103 insertions(+), 14 deletions(-) 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..54777cd5c04 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 + targetConfig string + analyzeMode bool } 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() From b0c54fefb42d8ca15ee0d6cccc25ff908fdb94c0 Mon Sep 17 00:00:00 2001 From: balopat Date: Fri, 24 Jan 2020 06:49:16 -0800 Subject: [PATCH 4/4] lint --- pkg/skaffold/initializer/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/skaffold/initializer/config.go b/pkg/skaffold/initializer/config.go index 54777cd5c04..b0d993a2ab6 100644 --- a/pkg/skaffold/initializer/config.go +++ b/pkg/skaffold/initializer/config.go @@ -38,8 +38,8 @@ var ( type skaffoldConfigAnalyzer struct { directoryAnalyzer force bool - targetConfig string analyzeMode bool + targetConfig string } func (a *skaffoldConfigAnalyzer) analyzeFile(filePath string) error {