From 3cba8f9c8a495beca8cfb069910cba29b0158c9a Mon Sep 17 00:00:00 2001
From: David Gageot <david@gageot.net>
Date: Fri, 19 Jun 2020 10:51:40 +0200
Subject: [PATCH] Fix #4327 (#4336)

Signed-off-by: David Gageot <david@gageot.net>
---
 cmd/skaffold/app/cmd/fix.go      | 12 ++++--
 cmd/skaffold/app/cmd/fix_test.go | 63 +++++++++++++++++++++++++++-----
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/cmd/skaffold/app/cmd/fix.go b/cmd/skaffold/app/cmd/fix.go
index 7db28f865d7..22b6c7f9a9e 100644
--- a/cmd/skaffold/app/cmd/fix.go
+++ b/cmd/skaffold/app/cmd/fix.go
@@ -57,8 +57,8 @@ func fix(out io.Writer, configFile string, toVersion string, overwrite bool) err
 		return err
 	}
 
-	if cfg.GetVersion() == latest.Version {
-		color.Default.Fprintln(out, "config is already latest version")
+	if cfg.GetVersion() == toVersion {
+		color.Default.Fprintln(out, "config is already version", toVersion)
 		return nil
 	}
 
@@ -67,8 +67,12 @@ func fix(out io.Writer, configFile string, toVersion string, overwrite bool) err
 		return err
 	}
 
-	if err := validation.Process(cfg.(*latest.SkaffoldConfig)); err != nil {
-		return fmt.Errorf("validating upgraded config: %w", err)
+	// TODO(dgageot): We should be able run validations on any schema version
+	// but that's not the case. They can only run on the latest version for now.
+	if toVersion == latest.Version {
+		if err := validation.Process(cfg.(*latest.SkaffoldConfig)); err != nil {
+			return fmt.Errorf("validating upgraded config: %w", err)
+		}
 	}
 
 	newCfg, err := yaml.Marshal(cfg)
diff --git a/cmd/skaffold/app/cmd/fix_test.go b/cmd/skaffold/app/cmd/fix_test.go
index 43b81bf3fd3..7a88c3f735c 100644
--- a/cmd/skaffold/app/cmd/fix_test.go
+++ b/cmd/skaffold/app/cmd/fix_test.go
@@ -23,18 +23,21 @@ import (
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	v1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
 func TestFix(t *testing.T) {
 	tests := []struct {
-		description string
-		inputYaml   string
-		output      string
-		shouldErr   bool
+		description   string
+		inputYaml     string
+		targetVersion string
+		output        string
+		shouldErr     bool
 	}{
 		{
-			description: "v1alpha4 to latest",
+			description:   "v1alpha4 to latest",
+			targetVersion: latest.Version,
 			inputYaml: `apiVersion: skaffold/v1alpha4
 kind: Config
 build:
@@ -69,7 +72,8 @@ deploy:
 `, latest.Version),
 		},
 		{
-			description: "v1alpha1 to latest",
+			description:   "v1alpha1 to latest",
+			targetVersion: latest.Version,
 			inputYaml: `apiVersion: skaffold/v1alpha1
 kind: Config
 build:
@@ -96,24 +100,65 @@ deploy:
 `, latest.Version),
 		},
 		{
-			description: "already latest version",
+			description:   "v1alpha1 to v1",
+			targetVersion: v1.Version,
+			inputYaml: `apiVersion: skaffold/v1alpha1
+kind: Config
+build:
+  artifacts:
+  - imageName: docker/image
+    dockerfilePath: dockerfile.test
+deploy:
+  kubectl:
+    manifests:
+    - paths:
+      - k8s/deployment.yaml
+`,
+			output: fmt.Sprintf(`apiVersion: %s
+kind: Config
+build:
+  artifacts:
+  - image: docker/image
+    docker:
+      dockerfile: dockerfile.test
+deploy:
+  kubectl:
+    manifests:
+    - k8s/deployment.yaml
+`, v1.Version),
+		},
+		{
+			description:   "already target version",
+			targetVersion: latest.Version,
 			inputYaml: fmt.Sprintf(`apiVersion: %s
 kind: Config
 `, latest.Version),
-			output: "config is already latest version\n",
+			output: "config is already version " + latest.Version + "\n",
 		},
 		{
 			description: "invalid input",
 			inputYaml:   "invalid",
 			shouldErr:   true,
 		},
+		{
+			description:   "validation fails",
+			targetVersion: latest.Version,
+			inputYaml: `apiVersion: skaffold/v1alpha1
+kind: Config
+build:
+  artifacts:
+  - imageName: 
+    dockerfilePath: dockerfile.test
+`,
+			shouldErr: true,
+		},
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			cfgFile := t.TempFile("config", []byte(test.inputYaml))
 
 			var b bytes.Buffer
-			err := fix(&b, cfgFile, latest.Version, false)
+			err := fix(&b, cfgFile, test.targetVersion, false)
 
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.output, b.String())
 		})