From 138506064a1dadb24dc891afd4fd6b7fc338eb65 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 09:44:22 +0200 Subject: [PATCH 01/14] Add missing test Signed-off-by: David Gageot --- pkg/skaffold/yamltags/tags_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/skaffold/yamltags/tags_test.go b/pkg/skaffold/yamltags/tags_test.go index fc905101043..ae2ffefb199 100644 --- a/pkg/skaffold/yamltags/tags_test.go +++ b/pkg/skaffold/yamltags/tags_test.go @@ -171,6 +171,16 @@ func TestValidateStructInvalid(t *testing.T) { _ = ValidateStruct(invalidYamlTags) } +func TestValidateStructInvalidSyntax(t *testing.T) { + invalidTags := struct { + A string `yamltags:"oneOf=set1=set2"` + }{} + + err := ValidateStruct(invalidTags) + + testutil.CheckError(t, true, err) +} + func TestIsZeroValue(t *testing.T) { testutil.CheckDeepEqual(t, true, isZeroValue(reflect.ValueOf(0))) testutil.CheckDeepEqual(t, true, isZeroValue(reflect.ValueOf(nil))) From 01cf6a31444381ca9c1103c33a44fa0960fa8370 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 09:47:51 +0200 Subject: [PATCH 02/14] Add missing test Signed-off-by: David Gageot --- pkg/skaffold/config/options_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/skaffold/config/options_test.go b/pkg/skaffold/config/options_test.go index c7d5e94d33c..84e919348b9 100644 --- a/pkg/skaffold/config/options_test.go +++ b/pkg/skaffold/config/options_test.go @@ -63,6 +63,20 @@ func TestLabels(t *testing.T) { "skaffold.dev/profiles": "profile1__profile2", }, }, + { + description: "tail", + options: SkaffoldOptions{Tail: true}, + expectedLabels: map[string]string{ + "skaffold.dev/tail": "true", + }, + }, + { + description: "tail dev", + options: SkaffoldOptions{TailDev: true}, + expectedLabels: map[string]string{ + "skaffold.dev/tail": "true", + }, + }, { description: "all labels", options: SkaffoldOptions{ From 7a1d9b037a21834a6d2d0726ce718fe2fc00c567 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 09:50:03 +0200 Subject: [PATCH 03/14] Add missing test Signed-off-by: David Gageot --- pkg/skaffold/version/version_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/skaffold/version/version_test.go b/pkg/skaffold/version/version_test.go index 35918166557..aa5bbcf1baf 100644 --- a/pkg/skaffold/version/version_test.go +++ b/pkg/skaffold/version/version_test.go @@ -54,3 +54,12 @@ func TestParseVersion(t *testing.T) { }) } } + +func TestUserAgent(t *testing.T) { + testutil.Override(t, &platform, "osx") + testutil.Override(t, &version, "1.0") + + userAgent := UserAgent() + + testutil.CheckDeepEqual(t, "skaffold/osx/1.0", userAgent) +} From e1c41abab41bb7ffa2cfbbba9a1c873757fdec08 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 09:56:44 +0200 Subject: [PATCH 04/14] Add missing test Signed-off-by: David Gageot --- pkg/skaffold/gcp/projectid_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/skaffold/gcp/projectid_test.go b/pkg/skaffold/gcp/projectid_test.go index 68842a13a18..9ae65285296 100644 --- a/pkg/skaffold/gcp/projectid_test.go +++ b/pkg/skaffold/gcp/projectid_test.go @@ -49,6 +49,11 @@ func TestExtractProjectID(t *testing.T) { imageName: "gcr.io", shouldErr: true, }, + { + description: "invalid reference", + imageName: "", + shouldErr: true, + }, } for _, test := range tests { From 0aa7e4b3ee26704c6a60cd8041c9fdab788d2dc6 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 09:56:59 +0200 Subject: [PATCH 05/14] Rewrite with go-containerregistry Signed-off-by: David Gageot --- pkg/skaffold/gcp/projectid.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/skaffold/gcp/projectid.go b/pkg/skaffold/gcp/projectid.go index ec4ea9f784d..142a13eeb09 100644 --- a/pkg/skaffold/gcp/projectid.go +++ b/pkg/skaffold/gcp/projectid.go @@ -20,27 +20,21 @@ import ( "fmt" "strings" - "github.com/docker/distribution/reference" - "github.com/docker/docker/registry" + "github.com/google/go-containerregistry/pkg/name" "github.com/pkg/errors" ) // ExtractProjectID extracts the GCP projectID from a docker image name // This only works if the imageName is pushed to gcr.io. func ExtractProjectID(imageName string) (string, error) { - ref, err := reference.ParseNormalizedNamed(imageName) + ref, err := name.ParseReference(imageName, name.WeakValidation) if err != nil { - return "", errors.Wrap(err, "parsing image name for registry") + return "", errors.Wrap(err, "parsing image name") } - repoInfo, err := registry.ParseRepositoryInfo(ref) - if err != nil { - return "", err - } - - index := repoInfo.Index - if index.Name == "gcr.io" || strings.HasSuffix(index.Name, ".gcr.io") { - parts := strings.Split(repoInfo.Name.String(), "/") + registry := ref.Context().Registry.Name() + if registry == "gcr.io" || strings.HasSuffix(registry, ".gcr.io") { + parts := strings.Split(imageName, "/") if len(parts) >= 2 { return parts[1], nil } From b524649128c6b6bcea8de6333364a31a66160cc7 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 09:59:51 +0200 Subject: [PATCH 06/14] Add missing test Signed-off-by: David Gageot --- pkg/skaffold/jib/jib_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/skaffold/jib/jib_test.go b/pkg/skaffold/jib/jib_test.go index cd8faf8981c..471e61b652e 100644 --- a/pkg/skaffold/jib/jib_test.go +++ b/pkg/skaffold/jib/jib_test.go @@ -42,7 +42,13 @@ func TestGetDependencies(t *testing.T) { var tests = []struct { stdout string expectedDeps []string + shouldErr bool }{ + { + stdout: "", + expectedDeps: nil, + shouldErr: true, + }, { stdout: "BEGIN JIB JSON\n{\"build\":[],\"inputs\":[],\"ignore\":[]}", expectedDeps: nil, @@ -90,7 +96,7 @@ func TestGetDependencies(t *testing.T) { results, err := getDependencies(tmpDir.Root(), &exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}, "test") - testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedDeps, results) + testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedDeps, results) }) } } From 8858db35a6c694185776e025a1c738a26e8a1dbf Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 10:06:17 +0200 Subject: [PATCH 07/14] Shorter syntax for single fake commands Signed-off-by: David Gageot --- pkg/skaffold/build/bazel/dependencies_test.go | 4 ++-- .../build/custom/dependencies_test.go | 2 +- pkg/skaffold/build/local/bazel_test.go | 2 +- pkg/skaffold/build/local/jib_maven_test.go | 2 +- pkg/skaffold/deploy/kubectl/version_test.go | 10 +++++----- pkg/skaffold/deploy/kubectl_test.go | 6 +++--- pkg/skaffold/deploy/kustomize_test.go | 2 +- pkg/skaffold/docker/client_test.go | 2 +- pkg/skaffold/jib/jib_gradle_test.go | 2 +- pkg/skaffold/jib/jib_maven_test.go | 2 +- pkg/skaffold/jib/jib_test.go | 2 +- testutil/cmd_helper.go | 20 +++++++++++++++++++ 12 files changed, 38 insertions(+), 18 deletions(-) diff --git a/pkg/skaffold/build/bazel/dependencies_test.go b/pkg/skaffold/build/bazel/dependencies_test.go index 4b048246c22..0229b7090ce 100644 --- a/pkg/skaffold/build/bazel/dependencies_test.go +++ b/pkg/skaffold/build/bazel/dependencies_test.go @@ -26,7 +26,7 @@ import ( ) func TestGetDependenciesWithWorkspace(t *testing.T) { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOut( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOut(t, "bazel query kind('source file', deps('target')) union buildfiles('target') --noimplicit_deps --order_output=no", "@ignored\n//external/ignored\n\n//:dep1\n//:dep2\n", )) @@ -44,7 +44,7 @@ func TestGetDependenciesWithWorkspace(t *testing.T) { } func TestGetDependenciesWithoutWorkspace(t *testing.T) { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOut( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOut(t, "bazel query kind('source file', deps('target2')) union buildfiles('target2') --noimplicit_deps --order_output=no", "@ignored\n//external/ignored\n\n//:dep3\n", )) diff --git a/pkg/skaffold/build/custom/dependencies_test.go b/pkg/skaffold/build/custom/dependencies_test.go index 712b2dcc7ac..dc8f4fd0816 100644 --- a/pkg/skaffold/build/custom/dependencies_test.go +++ b/pkg/skaffold/build/custom/dependencies_test.go @@ -60,7 +60,7 @@ func TestGetDependenciesDockerfile(t *testing.T) { } func TestGetDependenciesCommand(t *testing.T) { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOut( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOut(t, "echo [\"file1\",\"file2\",\"file3\"]", "[\"file1\",\"file2\",\"file3\"]", )) diff --git a/pkg/skaffold/build/local/bazel_test.go b/pkg/skaffold/build/local/bazel_test.go index 1d5b4445cec..14ff21eddbc 100644 --- a/pkg/skaffold/build/local/bazel_test.go +++ b/pkg/skaffold/build/local/bazel_test.go @@ -26,7 +26,7 @@ import ( ) func TestBazelBin(t *testing.T) { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOut( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOut(t, "bazel info bazel-bin --arg1 --arg2", "/absolute/path/bin\n", )) diff --git a/pkg/skaffold/build/local/jib_maven_test.go b/pkg/skaffold/build/local/jib_maven_test.go index 86955857e4e..ee334c1c9b1 100644 --- a/pkg/skaffold/build/local/jib_maven_test.go +++ b/pkg/skaffold/build/local/jib_maven_test.go @@ -43,7 +43,7 @@ func TestMavenVerifyJibPackageGoal(t *testing.T) { defer reset() for _, tt := range testCases { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOut( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOut(t, "mvn --quiet --projects module jib:_skaffold-package-goals", tt.mavenOutput, )) diff --git a/pkg/skaffold/deploy/kubectl/version_test.go b/pkg/skaffold/deploy/kubectl/version_test.go index f0e05a7e89f..cf19fac8db7 100644 --- a/pkg/skaffold/deploy/kubectl/version_test.go +++ b/pkg/skaffold/deploy/kubectl/version_test.go @@ -35,26 +35,26 @@ func TestCheckVersion(t *testing.T) { }{ { description: "1.12 is valid", - command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", `{"clientVersion":{"major":"1","minor":"12"}}`), + command: testutil.FakeRunOut(t, "kubectl version --client -ojson", `{"clientVersion":{"major":"1","minor":"12"}}`), }, { description: "1.12+ is valid", - command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", `{"clientVersion":{"major":"1","minor":"12+"}}`), + command: testutil.FakeRunOut(t, "kubectl version --client -ojson", `{"clientVersion":{"major":"1","minor":"12+"}}`), }, { description: "1.11 is too old", - command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", `{"clientVersion":{"major":"1","minor":"11"}}`), + command: testutil.FakeRunOut(t, "kubectl version --client -ojson", `{"clientVersion":{"major":"1","minor":"11"}}`), shouldErr: true, }, { description: "invalid version", - command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", `not json`), + command: testutil.FakeRunOut(t, "kubectl version --client -ojson", `not json`), shouldErr: true, warnings: []string{"unable to parse client version: invalid character 'o' in literal null (expecting 'u')"}, }, { description: "cli not found", - command: testutil.NewFakeCmd(t).WithRunOutErr("kubectl version --client -ojson", ``, errors.New("not found")), + command: testutil.FakeRunOutErr(t, "kubectl version --client -ojson", ``, errors.New("not found")), shouldErr: true, warnings: []string{"unable to get kubectl client version: not found"}, }, diff --git a/pkg/skaffold/deploy/kubectl_test.go b/pkg/skaffold/deploy/kubectl_test.go index 97b0c74bfda..a25a6aeb857 100644 --- a/pkg/skaffold/deploy/kubectl_test.go +++ b/pkg/skaffold/deploy/kubectl_test.go @@ -72,21 +72,21 @@ func TestKubectlDeploy(t *testing.T) { { description: "no manifest", cfg: &latest.KubectlDeploy{}, - command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", kubectlVersion), + command: testutil.FakeRunOut(t, "kubectl version --client -ojson", kubectlVersion), }, { description: "missing manifest file", cfg: &latest.KubectlDeploy{ Manifests: []string{"missing.yaml"}, }, - command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", kubectlVersion), + command: testutil.FakeRunOut(t, "kubectl version --client -ojson", kubectlVersion), }, { description: "ignore non-manifest", cfg: &latest.KubectlDeploy{ Manifests: []string{"*.ignored"}, }, - command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", kubectlVersion), + command: testutil.FakeRunOut(t, "kubectl version --client -ojson", kubectlVersion), }, { description: "deploy success (forced)", diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 0c212b754aa..ac27de9c979 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -129,7 +129,7 @@ func TestKustomizeCleanup(t *testing.T) { cfg: &latest.KustomizeDeploy{ KustomizePath: tmpDir.Root(), }, - command: testutil.NewFakeCmd(t).WithRunOutErr("kustomize build "+tmpDir.Root(), ``, errors.New("BUG")), + command: testutil.FakeRunOutErr(t, "kustomize build "+tmpDir.Root(), ``, errors.New("BUG")), shouldErr: true, }, } diff --git a/pkg/skaffold/docker/client_test.go b/pkg/skaffold/docker/client_test.go index 733a9a25752..752d9f397c3 100644 --- a/pkg/skaffold/docker/client_test.go +++ b/pkg/skaffold/docker/client_test.go @@ -115,7 +115,7 @@ DOCKER_API_VERSION=1.23`, for _, test := range tests { t.Run(test.description, func(t *testing.T) { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOut( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOut(t, "minikube docker-env --shell none", test.env, )) diff --git a/pkg/skaffold/jib/jib_gradle_test.go b/pkg/skaffold/jib/jib_gradle_test.go index 5da89dd851d..335de20ce57 100644 --- a/pkg/skaffold/jib/jib_gradle_test.go +++ b/pkg/skaffold/jib/jib_gradle_test.go @@ -86,7 +86,7 @@ func TestGetDependenciesGradle(t *testing.T) { for _, test := range tests { t.Run(test.description, func(t *testing.T) { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOutErr( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOutErr(t, strings.Join(getCommandGradle(ctx, tmpDir.Root(), &latest.JibGradleArtifact{Project: "gradle-test"}).Args, " "), test.stdout, test.err, diff --git a/pkg/skaffold/jib/jib_maven_test.go b/pkg/skaffold/jib/jib_maven_test.go index 56c0364c200..38cbe2199c8 100644 --- a/pkg/skaffold/jib/jib_maven_test.go +++ b/pkg/skaffold/jib/jib_maven_test.go @@ -86,7 +86,7 @@ func TestGetDependenciesMaven(t *testing.T) { for _, test := range tests { t.Run(test.description, func(t *testing.T) { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOutErr( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOutErr(t, strings.Join(getCommandMaven(ctx, tmpDir.Root(), &latest.JibMavenArtifact{Module: "maven-test"}).Args, " "), test.stdout, test.err, diff --git a/pkg/skaffold/jib/jib_test.go b/pkg/skaffold/jib/jib_test.go index 471e61b652e..23bdc1ec9c7 100644 --- a/pkg/skaffold/jib/jib_test.go +++ b/pkg/skaffold/jib/jib_test.go @@ -88,7 +88,7 @@ func TestGetDependencies(t *testing.T) { watchedFiles = map[string]filesLists{} t.Run("getDependencies", func(t *testing.T) { - reset := testutil.Override(t, &util.DefaultExecCommand, testutil.NewFakeCmd(t).WithRunOut( + reset := testutil.Override(t, &util.DefaultExecCommand, testutil.FakeRunOut(t, "ignored", test.stdout, )) diff --git a/testutil/cmd_helper.go b/testutil/cmd_helper.go index a7c15c028e1..93f15dd7cd3 100644 --- a/testutil/cmd_helper.go +++ b/testutil/cmd_helper.go @@ -58,6 +58,26 @@ func (c *FakeCmd) popRun() (*run, error) { return &run, nil } +func FakeRun(t *testing.T, command string) *FakeCmd { + return NewFakeCmd(t).WithRun(command) +} + +func FakeRunInput(t *testing.T, command, input string) *FakeCmd { + return NewFakeCmd(t).WithRunInput(command, input) +} + +func FakeRunErr(t *testing.T, command string, err error) *FakeCmd { + return NewFakeCmd(t).WithRunErr(command, err) +} + +func FakeRunOut(t *testing.T, command string, output string) *FakeCmd { + return NewFakeCmd(t).WithRunOut(command, output) +} + +func FakeRunOutErr(t *testing.T, command string, output string, err error) *FakeCmd { + return NewFakeCmd(t).WithRunOutErr(command, output, err) +} + func (c *FakeCmd) WithRun(command string) *FakeCmd { return c.addRun(run{ command: command, From a92cc532807ff5aa495195d99723b5e4e7a717b3 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 10:13:58 +0200 Subject: [PATCH 08/14] Test ForceColors Signed-off-by: David Gageot --- pkg/skaffold/color/formatter.go | 5 ++++- pkg/skaffold/color/formatter_test.go | 7 +++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/skaffold/color/formatter.go b/pkg/skaffold/color/formatter.go index 63d4f9d9d58..260a7b8d3a4 100644 --- a/pkg/skaffold/color/formatter.go +++ b/pkg/skaffold/color/formatter.go @@ -121,8 +121,11 @@ func isTerminal(w io.Writer) bool { } } -func ForceColors() { +func ForceColors() func() { IsTerminal = func(_ io.Writer) bool { return true } + return func() { + IsTerminal = isTerminal + } } diff --git a/pkg/skaffold/color/formatter_test.go b/pkg/skaffold/color/formatter_test.go index 80e312c11a5..5050eb9f0f1 100644 --- a/pkg/skaffold/color/formatter_test.go +++ b/pkg/skaffold/color/formatter_test.go @@ -18,7 +18,6 @@ package color import ( "bytes" - "io" "testing" "github.com/GoogleContainerTools/skaffold/testutil" @@ -38,7 +37,7 @@ func compareText(t *testing.T, expected, actual string, expectedN int, actualN i } func TestFprint(t *testing.T) { - reset := testutil.Override(t, &IsTerminal, func(io.Writer) bool { return true }) + reset := ForceColors() defer reset() var b bytes.Buffer @@ -48,7 +47,7 @@ func TestFprint(t *testing.T) { } func TestFprintln(t *testing.T) { - reset := testutil.Override(t, &IsTerminal, func(io.Writer) bool { return true }) + reset := ForceColors() defer reset() var b bytes.Buffer @@ -58,7 +57,7 @@ func TestFprintln(t *testing.T) { } func TestFprintf(t *testing.T) { - reset := testutil.Override(t, &IsTerminal, func(io.Writer) bool { return true }) + reset := ForceColors() defer reset() var b bytes.Buffer From 53382280476750f4eef681efb9179a5d112da84e Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 10:20:40 +0200 Subject: [PATCH 09/14] Add missing test Signed-off-by: David Gageot --- pkg/skaffold/color/formatter_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pkg/skaffold/color/formatter_test.go b/pkg/skaffold/color/formatter_test.go index 5050eb9f0f1..020d5e4255c 100644 --- a/pkg/skaffold/color/formatter_test.go +++ b/pkg/skaffold/color/formatter_test.go @@ -18,6 +18,7 @@ package color import ( "bytes" + "io" "testing" "github.com/GoogleContainerTools/skaffold/testutil" @@ -66,6 +67,22 @@ func TestFprintf(t *testing.T) { compareText(t, "\033[32mIt's been 1 week\033[0m", b.String(), 25, n, err) } +type nopCloser struct{ io.Writer } + +func (n *nopCloser) Close() error { return nil } + +func TestFprintOnColoredWriter(t *testing.T) { + var b bytes.Buffer + + coloredWriter := ColoredWriteCloser{ + WriteCloser: &nopCloser{Writer: &b}, + } + + n, err := Green.Fprint(coloredWriter, "It's not easy being") + + compareText(t, "\033[32mIt's not easy being\033[0m", b.String(), 28, n, err) +} + func TestFprintNoTTY(t *testing.T) { var b bytes.Buffer expected := "It's not easy being" From 9b1db78abe8057c76a2ff3c518b9925ade13ebdd Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 10:28:55 +0200 Subject: [PATCH 10/14] Add missing test Signed-off-by: David Gageot --- pkg/skaffold/schema/v1alpha1/upgrade_test.go | 34 ++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/pkg/skaffold/schema/v1alpha1/upgrade_test.go b/pkg/skaffold/schema/v1alpha1/upgrade_test.go index cd48533484e..e3b2c531154 100644 --- a/pkg/skaffold/schema/v1alpha1/upgrade_test.go +++ b/pkg/skaffold/schema/v1alpha1/upgrade_test.go @@ -80,6 +80,40 @@ deploy: verifyUpgrade(t, yaml, expected) } +func TestUpgrade_helm(t *testing.T) { + yaml := `apiVersion: skaffold/v1alpha1 +kind: Config +build: + artifacts: + - imageName: gcr.io/k8s-skaffold/skaffold-example +deploy: + helm: + releases: + - name: release + chartPath: path + valuesFilePath: valuesFile + values: {key:value} + namespace: ns + version: 1.0 +` + expected := `apiVersion: skaffold/v1alpha2 +kind: Config +build: + artifacts: + - imageName: gcr.io/k8s-skaffold/skaffold-example +deploy: + helm: + releases: + - name: release + chartPath: path + valuesFilePath: valuesFile + values: {key:value} + namespace: ns + version: 1.0 +` + verifyUpgrade(t, yaml, expected) +} + func verifyUpgrade(t *testing.T, input, output string) { config := NewSkaffoldConfig() err := yaml.UnmarshalStrict([]byte(input), config) From cf46039e39893fd7d2ce258dc8bbe8aec7cdae99 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 10:34:51 +0200 Subject: [PATCH 11/14] Add missing tests Signed-off-by: David Gageot --- pkg/skaffold/schema/v1alpha1/upgrade_test.go | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/pkg/skaffold/schema/v1alpha1/upgrade_test.go b/pkg/skaffold/schema/v1alpha1/upgrade_test.go index e3b2c531154..58e85e7c8ef 100644 --- a/pkg/skaffold/schema/v1alpha1/upgrade_test.go +++ b/pkg/skaffold/schema/v1alpha1/upgrade_test.go @@ -114,6 +114,44 @@ deploy: verifyUpgrade(t, yaml, expected) } +func TestUpgrade_dockerfile(t *testing.T) { + yaml := `apiVersion: skaffold/v1alpha1 +kind: Config +build: + artifacts: + - imageName: gcr.io/k8s-skaffold/skaffold-example + dockerfilePath: Dockerfile +` + expected := `apiVersion: skaffold/v1alpha2 +kind: Config +build: + artifacts: + - imageName: gcr.io/k8s-skaffold/skaffold-example + docker: + dockerfilePath: Dockerfile +` + verifyUpgrade(t, yaml, expected) +} + +func TestUpgrade_buildargs(t *testing.T) { + yaml := `apiVersion: skaffold/v1alpha1 +kind: Config +build: + artifacts: + - imageName: gcr.io/k8s-skaffold/skaffold-example + buildArgs: {key:value} +` + expected := `apiVersion: skaffold/v1alpha2 +kind: Config +build: + artifacts: + - imageName: gcr.io/k8s-skaffold/skaffold-example + docker: + buildArgs: {key:value} +` + verifyUpgrade(t, yaml, expected) +} + func verifyUpgrade(t *testing.T, input, output string) { config := NewSkaffoldConfig() err := yaml.UnmarshalStrict([]byte(input), config) From 8cc2de717251776c2cfbbf6eab531f40d9517697 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 10:41:04 +0200 Subject: [PATCH 12/14] Add missing tests Signed-off-by: David Gageot --- pkg/skaffold/schema/v1alpha1/upgrade_test.go | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/pkg/skaffold/schema/v1alpha1/upgrade_test.go b/pkg/skaffold/schema/v1alpha1/upgrade_test.go index 58e85e7c8ef..d961e2581ac 100644 --- a/pkg/skaffold/schema/v1alpha1/upgrade_test.go +++ b/pkg/skaffold/schema/v1alpha1/upgrade_test.go @@ -152,6 +152,38 @@ build: verifyUpgrade(t, yaml, expected) } +func TestUpgrade_gcb(t *testing.T) { + yaml := `apiVersion: skaffold/v1alpha1 +kind: Config +build: + googleCloudBuild: + projectId: PROJECT +` + expected := `apiVersion: skaffold/v1alpha2 +kind: Config +build: + googleCloudBuild: + projectId: PROJECT +` + verifyUpgrade(t, yaml, expected) +} + +func TestUpgrade_local(t *testing.T) { + yaml := `apiVersion: skaffold/v1alpha1 +kind: Config +build: + local: + skipPush: true +` + expected := `apiVersion: skaffold/v1alpha2 +kind: Config +build: + local: + skipPush: true +` + verifyUpgrade(t, yaml, expected) +} + func verifyUpgrade(t *testing.T, input, output string) { config := NewSkaffoldConfig() err := yaml.UnmarshalStrict([]byte(input), config) From f2c1b24213cb559e0870a964e0b2fdab1e2afc1c Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 10:52:29 +0200 Subject: [PATCH 13/14] Add missing test Signed-off-by: David Gageot --- pkg/skaffold/schema/v1alpha2/upgrade_test.go | 25 ++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/pkg/skaffold/schema/v1alpha2/upgrade_test.go b/pkg/skaffold/schema/v1alpha2/upgrade_test.go index 592bcf1d89a..8d34a83a7e5 100644 --- a/pkg/skaffold/schema/v1alpha2/upgrade_test.go +++ b/pkg/skaffold/schema/v1alpha2/upgrade_test.go @@ -46,6 +46,31 @@ deploy: verifyUpgrade(t, yaml, expected) } +func TestUpgrade_helmReleaseValuesFileWithProfile(t *testing.T) { + yaml := `apiVersion: skaffold/v1alpha2 +kind: Config +profiles: +- name: test + deploy: + helm: + releases: + - name: test + valuesFilePath: values.yaml +` + expected := `apiVersion: skaffold/v1alpha3 +kind: Config +profiles: +- name: test + deploy: + helm: + releases: + - name: test + valuesFiles: + - values.yaml +` + verifyUpgrade(t, yaml, expected) +} + func TestUpgrade_kanikoWithProfile(t *testing.T) { yaml := `apiVersion: skaffold/v1alpha2 kind: Config From a5049dcfbe0e270b59f4de11c352875aa514f810 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 29 May 2019 10:54:06 +0200 Subject: [PATCH 14/14] Remove duplicated code Signed-off-by: David Gageot --- pkg/skaffold/schema/v1alpha2/upgrade.go | 20 ++++---------------- pkg/skaffold/schema/v1alpha3/upgrade.go | 20 ++++---------------- pkg/skaffold/schema/v1alpha4/upgrade.go | 22 +++++----------------- pkg/skaffold/schema/v1alpha5/upgrade.go | 22 +++++----------------- 4 files changed, 18 insertions(+), 66 deletions(-) diff --git a/pkg/skaffold/schema/v1alpha2/upgrade.go b/pkg/skaffold/schema/v1alpha2/upgrade.go index d3f064605a5..abb4783af84 100644 --- a/pkg/skaffold/schema/v1alpha2/upgrade.go +++ b/pkg/skaffold/schema/v1alpha2/upgrade.go @@ -17,10 +17,9 @@ limitations under the License. package v1alpha2 import ( - "encoding/json" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" next "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha3" + pkgutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/pkg/errors" ) @@ -34,7 +33,7 @@ import ( func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { // convert Deploy (should be the same) var newDeploy next.DeployConfig - if err := convert(config.Deploy, &newDeploy); err != nil { + if err := pkgutil.CloneThroughJSON(config.Deploy, &newDeploy); err != nil { return nil, errors.Wrap(err, "converting deploy config") } // if the helm deploy config was set, then convert ValueFilePath to ValuesFiles @@ -49,7 +48,7 @@ func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { // convert Profiles (should be the same) var newProfiles []next.Profile if config.Profiles != nil { - if err := convert(config.Profiles, &newProfiles); err != nil { + if err := pkgutil.CloneThroughJSON(config.Profiles, &newProfiles); err != nil { return nil, errors.Wrap(err, "converting new profile") } } @@ -71,7 +70,7 @@ func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { // copy over old build config to new build config var newBuild next.BuildConfig - if err := convert(config.Build, &newBuild); err != nil { + if err := pkgutil.CloneThroughJSON(config.Build, &newBuild); err != nil { return nil, errors.Wrap(err, "converting new build") } // if the kaniko build was set, then convert it @@ -95,14 +94,3 @@ func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { Profiles: newProfiles, }, nil } - -func convert(old interface{}, new interface{}) error { - o, err := json.Marshal(old) - if err != nil { - return errors.Wrap(err, "marshalling old") - } - if err := json.Unmarshal(o, &new); err != nil { - return errors.Wrap(err, "unmarshalling new") - } - return nil -} diff --git a/pkg/skaffold/schema/v1alpha3/upgrade.go b/pkg/skaffold/schema/v1alpha3/upgrade.go index 2e43fdf37fb..278e8e8a4c0 100644 --- a/pkg/skaffold/schema/v1alpha3/upgrade.go +++ b/pkg/skaffold/schema/v1alpha3/upgrade.go @@ -17,10 +17,9 @@ limitations under the License. package v1alpha3 import ( - "encoding/json" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" next "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha4" + pkgutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/pkg/errors" ) @@ -44,14 +43,14 @@ import ( func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { // convert Deploy (should be the same) var newDeploy next.DeployConfig - if err := convert(config.Deploy, &newDeploy); err != nil { + if err := pkgutil.CloneThroughJSON(config.Deploy, &newDeploy); err != nil { return nil, errors.Wrap(err, "converting deploy config") } // convert Profiles (should be the same) var newProfiles []next.Profile if config.Profiles != nil { - if err := convert(config.Profiles, &newProfiles); err != nil { + if err := pkgutil.CloneThroughJSON(config.Profiles, &newProfiles); err != nil { return nil, errors.Wrap(err, "converting new profile") } for i, oldProfile := range config.Profiles { @@ -62,7 +61,7 @@ func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { // convert Build (should be the same) var newBuild next.BuildConfig oldBuild := config.Build - if err := convert(oldBuild, &newBuild); err != nil { + if err := pkgutil.CloneThroughJSON(oldBuild, &newBuild); err != nil { return nil, errors.Wrap(err, "converting new build") } convertBuild(oldBuild, newBuild) @@ -82,14 +81,3 @@ func convertBuild(oldBuild BuildConfig, newBuild next.BuildConfig) { newBuild.LocalBuild.Push = &push } } - -func convert(old interface{}, new interface{}) error { - o, err := json.Marshal(old) - if err != nil { - return errors.Wrap(err, "marshalling old") - } - if err := json.Unmarshal(o, &new); err != nil { - return errors.Wrap(err, "unmarshalling new") - } - return nil -} diff --git a/pkg/skaffold/schema/v1alpha4/upgrade.go b/pkg/skaffold/schema/v1alpha4/upgrade.go index 6d14d218287..2a9d9e2ca52 100644 --- a/pkg/skaffold/schema/v1alpha4/upgrade.go +++ b/pkg/skaffold/schema/v1alpha4/upgrade.go @@ -17,10 +17,9 @@ limitations under the License. package v1alpha4 import ( - "encoding/json" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" next "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha5" + pkgutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/pkg/errors" ) @@ -34,27 +33,27 @@ import ( func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { // convert Deploy (should be the same) var newDeploy next.DeployConfig - if err := convert(config.Deploy, &newDeploy); err != nil { + if err := pkgutil.CloneThroughJSON(config.Deploy, &newDeploy); err != nil { return nil, errors.Wrap(err, "converting deploy config") } // convert Profiles (should be the same) var newProfiles []next.Profile if config.Profiles != nil { - if err := convert(config.Profiles, &newProfiles); err != nil { + if err := pkgutil.CloneThroughJSON(config.Profiles, &newProfiles); err != nil { return nil, errors.Wrap(err, "converting new profile") } } // convert Build (should be the same) var newBuild next.BuildConfig - if err := convert(config.Build, &newBuild); err != nil { + if err := pkgutil.CloneThroughJSON(config.Build, &newBuild); err != nil { return nil, errors.Wrap(err, "converting new build") } // convert Test (should be the same) var newTest next.TestConfig - if err := convert(config.Test, &newTest); err != nil { + if err := pkgutil.CloneThroughJSON(config.Test, &newTest); err != nil { return nil, errors.Wrap(err, "converting new test") } @@ -67,14 +66,3 @@ func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { Profiles: newProfiles, }, nil } - -func convert(old interface{}, new interface{}) error { - o, err := json.Marshal(old) - if err != nil { - return errors.Wrap(err, "marshalling old") - } - if err := json.Unmarshal(o, &new); err != nil { - return errors.Wrap(err, "unmarshalling new") - } - return nil -} diff --git a/pkg/skaffold/schema/v1alpha5/upgrade.go b/pkg/skaffold/schema/v1alpha5/upgrade.go index 57e1e344efa..d41ea6e0a16 100644 --- a/pkg/skaffold/schema/v1alpha5/upgrade.go +++ b/pkg/skaffold/schema/v1alpha5/upgrade.go @@ -17,10 +17,9 @@ limitations under the License. package v1alpha5 import ( - "encoding/json" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" next "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1beta1" + pkgutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/pkg/errors" ) @@ -40,7 +39,7 @@ func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { // convert Deploy (should be the same) var newDeploy next.DeployConfig - if err := convert(config.Deploy, &newDeploy); err != nil { + if err := pkgutil.CloneThroughJSON(config.Deploy, &newDeploy); err != nil { return nil, errors.Wrap(err, "converting deploy config") } @@ -52,19 +51,19 @@ func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { return nil, errors.Errorf("can't upgrade to %s, profiles.build.acr is not supported anymore, please remove it from the %s profile manually", next.Version, profile.Name) } } - if err := convert(config.Profiles, &newProfiles); err != nil { + if err := pkgutil.CloneThroughJSON(config.Profiles, &newProfiles); err != nil { return nil, errors.Wrap(err, "converting new profile") } } // convert Build (should be the same) var newBuild next.BuildConfig - if err := convert(config.Build, &newBuild); err != nil { + if err := pkgutil.CloneThroughJSON(config.Build, &newBuild); err != nil { return nil, errors.Wrap(err, "converting new build") } // convert Test (should be the same) var newTest next.TestConfig - if err := convert(config.Test, &newTest); err != nil { + if err := pkgutil.CloneThroughJSON(config.Test, &newTest); err != nil { return nil, errors.Wrap(err, "converting new test") } @@ -77,14 +76,3 @@ func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { Profiles: newProfiles, }, nil } - -func convert(old interface{}, new interface{}) error { - o, err := json.Marshal(old) - if err != nil { - return errors.Wrap(err, "marshalling old") - } - if err := json.Unmarshal(o, &new); err != nil { - return errors.Wrap(err, "unmarshalling new") - } - return nil -}