Merge pull request #2212 from dgageot/simplify-schema-upgrades

Simplify schema upgrades: remove duplication
GoogleContainerTools · Jun 3, 2019 · c2ba29e · c2ba29e
2 parents b139240 + 8929dd5
commit c2ba29e
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 164 deletions.
diff --git a/pkg/skaffold/schema/v1alpha4/upgrade.go b/pkg/skaffold/schema/v1alpha4/upgrade.go
@@ -20,7 +20,6 @@ import (
 	"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"
 )
 
 // Upgrade upgrades a configuration to the next version.
@@ -31,38 +30,10 @@ import (
 // 3. Updates
 //    - minor - []TestCase type aliased to TestConfig
 func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) {
-	// convert Deploy (should be the same)
-	var newDeploy next.DeployConfig
-	if err := pkgutil.CloneThroughJSON(config.Deploy, &newDeploy); err != nil {
-		return nil, errors.Wrap(err, "converting deploy config")
-	}
+	var newConfig next.SkaffoldConfig
 
-	// convert Profiles (should be the same)
-	var newProfiles []next.Profile
-	if config.Profiles != nil {
-		if err := pkgutil.CloneThroughJSON(config.Profiles, &newProfiles); err != nil {
-			return nil, errors.Wrap(err, "converting new profile")
-		}
-	}
+	err := pkgutil.CloneThroughJSON(config, &newConfig)
+	newConfig.APIVersion = next.Version
 
-	// convert Build (should be the same)
-	var newBuild next.BuildConfig
-	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 := pkgutil.CloneThroughJSON(config.Test, &newTest); err != nil {
-		return nil, errors.Wrap(err, "converting new test")
-	}
-
-	return &next.SkaffoldConfig{
-		APIVersion: next.Version,
-		Kind:       config.Kind,
-		Build:      newBuild,
-		Test:       newTest,
-		Deploy:     newDeploy,
-		Profiles:   newProfiles,
-	}, nil
+	return &newConfig, err
 }
diff --git a/pkg/skaffold/schema/v1alpha5/upgrade.go b/pkg/skaffold/schema/v1alpha5/upgrade.go
@@ -32,47 +32,20 @@ import (
 //   - AzureContainerBuilder
 // 3. No updates
 func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) {
-
 	if config.Build.AzureContainerBuild != nil {
 		return nil, errors.Errorf("can't upgrade to %s, build.acr is not supported anymore, please remove it manually", next.Version)
 	}
 
-	// convert Deploy (should be the same)
-	var newDeploy next.DeployConfig
-	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 {
-		for _, profile := range config.Profiles {
-			if profile.Build.AzureContainerBuild != nil {
-				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 := pkgutil.CloneThroughJSON(config.Profiles, &newProfiles); err != nil {
-			return nil, errors.Wrap(err, "converting new profile")
+	for _, profile := range config.Profiles {
+		if profile.Build.AzureContainerBuild != nil {
+			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)
 		}
 	}
-	// convert Build (should be the same)
-	var newBuild next.BuildConfig
-	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 := pkgutil.CloneThroughJSON(config.Test, &newTest); err != nil {
-		return nil, errors.Wrap(err, "converting new test")
-	}
+	var newConfig next.SkaffoldConfig
+
+	err := pkgutil.CloneThroughJSON(config, &newConfig)
+	newConfig.APIVersion = next.Version
 
-	return &next.SkaffoldConfig{
-		APIVersion: next.Version,
-		Kind:       config.Kind,
-		Build:      newBuild,
-		Test:       newTest,
-		Deploy:     newDeploy,
-		Profiles:   newProfiles,
-	}, nil
+	return &newConfig, err
 }
diff --git a/pkg/skaffold/schema/v1beta10/upgrade.go b/pkg/skaffold/schema/v1beta10/upgrade.go
@@ -20,7 +20,6 @@ import (
 	next "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
 	pkgutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
-	"github.com/pkg/errors"
 )
 
 // Upgrade upgrades a configuration to the next version.
@@ -30,40 +29,10 @@ import (
 // 2. No removals
 // 3. No Updates
 func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) {
-	// convert Deploy (should be the same)
-	var newDeploy next.DeployConfig
-	if err := pkgutil.CloneThroughJSON(config.Deploy, &newDeploy); err != nil {
-		return nil, errors.Wrap(err, "converting deploy config")
-	}
+	var newConfig next.SkaffoldConfig
 
-	// convert Profiles (should be the same)
-	var newProfiles []next.Profile
-	if config.Profiles != nil {
-		if err := pkgutil.CloneThroughJSON(config.Profiles, &newProfiles); err != nil {
-			return nil, errors.Wrap(err, "converting new profile")
-		}
-	}
+	err := pkgutil.CloneThroughJSON(config, &newConfig)
+	newConfig.APIVersion = next.Version
 
-	// convert Build (should be same)
-	var newBuild next.BuildConfig
-	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.TestCase
-	if err := pkgutil.CloneThroughJSON(config.Test, &newTest); err != nil {
-		return nil, errors.Wrap(err, "converting new test")
-	}
-
-	return &next.SkaffoldConfig{
-		APIVersion: next.Version,
-		Kind:       config.Kind,
-		Pipeline: next.Pipeline{
-			Build:  newBuild,
-			Test:   newTest,
-			Deploy: newDeploy,
-		},
-		Profiles: newProfiles,
-	}, nil
+	return &newConfig, err
 }
diff --git a/pkg/skaffold/schema/v1beta7/upgrade.go b/pkg/skaffold/schema/v1beta7/upgrade.go
@@ -20,7 +20,6 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
 	next "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1beta8"
 	pkgutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
-	"github.com/pkg/errors"
 )
 
 // Upgrade upgrades a configuration to the next version.
@@ -30,40 +29,10 @@ import (
 // 2. No removals
 // 3. No updates
 func (config *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) {
-	// convert Deploy (should be the same)
-	var newDeploy next.DeployConfig
-	if err := pkgutil.CloneThroughJSON(config.Deploy, &newDeploy); err != nil {
-		return nil, errors.Wrap(err, "converting deploy config")
-	}
+	var newConfig next.SkaffoldConfig
 
-	// convert Profiles (should be the same)
-	var newProfiles []next.Profile
-	if config.Profiles != nil {
-		if err := pkgutil.CloneThroughJSON(config.Profiles, &newProfiles); err != nil {
-			return nil, errors.Wrap(err, "converting new profile")
-		}
-	}
+	err := pkgutil.CloneThroughJSON(config, &newConfig)
+	newConfig.APIVersion = next.Version
 
-	// convert Kaniko (should be same)
-	var newBuild next.BuildConfig
-	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.TestCase
-	if err := pkgutil.CloneThroughJSON(config.Test, &newTest); err != nil {
-		return nil, errors.Wrap(err, "converting new test")
-	}
-
-	return &next.SkaffoldConfig{
-		APIVersion: next.Version,
-		Kind:       config.Kind,
-		Pipeline: next.Pipeline{
-			Build:  newBuild,
-			Test:   newTest,
-			Deploy: newDeploy,
-		},
-		Profiles: newProfiles,
-	}, nil
+	return &newConfig, err
 }
diff --git a/pkg/skaffold/schema/v1beta8/upgrade.go b/pkg/skaffold/schema/v1beta8/upgrade.go
@@ -21,7 +21,6 @@ import (
 	next "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1beta9"
 	pkgutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/pkg/errors"
-	"gopkg.in/yaml.v2"
 )
 
 // Upgrade upgrades a configuration to the next version.
@@ -86,24 +85,15 @@ func updateBuild(config *BuildConfig, newBuild *next.BuildConfig) error {
 		}
 		if a.BuilderPlugin.Name == "bazel" {
 			var ba *next.BazelArtifact
-			contents, err := yaml.Marshal(a.BuilderPlugin.Properties)
-			if err != nil {
-				return errors.Wrap(err, "unmarshalling properties")
-			}
-			if err := yaml.Unmarshal(contents, &ba); err != nil {
-				return errors.Wrap(err, "unmarshalling bazel artifact")
+			if err := pkgutil.CloneThroughYAML(a.BuilderPlugin.Properties, &ba); err != nil {
+				return errors.Wrap(err, "converting bazel artifact")
 			}
 			newBuild.Artifacts[i].BazelArtifact = ba
 		}
-
 		if a.BuilderPlugin.Name == "docker" {
 			var da *next.DockerArtifact
-			contents, err := yaml.Marshal(a.BuilderPlugin.Properties)
-			if err != nil {
-				return errors.Wrap(err, "unmarshalling properties")
-			}
-			if err := yaml.Unmarshal(contents, &da); err != nil {
-				return errors.Wrap(err, "unmarshalling bazel artifact")
+			if err := pkgutil.CloneThroughYAML(a.BuilderPlugin.Properties, &da); err != nil {
+				return errors.Wrap(err, "converting docker artifact")
 			}
 			newBuild.Artifacts[i].DockerArtifact = da
 		}
@@ -112,23 +102,15 @@ func updateBuild(config *BuildConfig, newBuild *next.BuildConfig) error {
 	if c := config.ExecutionEnvironment; c != nil {
 		if c.Name == "googleCloudBuild" {
 			var gcb *next.GoogleCloudBuild
-			contents, err := yaml.Marshal(c.Properties)
-			if err != nil {
-				return errors.Wrap(err, "unmarshalling properties")
-			}
-			if err := yaml.Unmarshal(contents, &gcb); err != nil {
-				return errors.Wrap(err, "unmarshalling bazel artifact")
+			if err := pkgutil.CloneThroughYAML(c.Properties, &gcb); err != nil {
+				return errors.Wrap(err, "converting gcb artifact")
 			}
 			newBuild.GoogleCloudBuild = gcb
 		}
 		if c.Name == "local" {
 			var local *next.LocalBuild
-			contents, err := yaml.Marshal(c.Properties)
-			if err != nil {
-				return errors.Wrap(err, "unmarshalling properties")
-			}
-			if err := yaml.Unmarshal(contents, &local); err != nil {
-				return errors.Wrap(err, "unmarshalling bazel artifact")
+			if err := pkgutil.CloneThroughYAML(c.Properties, &local); err != nil {
+				return errors.Wrap(err, "converting local artifact")
 			}
 			newBuild.LocalBuild = local
 		}

diff --git a/pkg/skaffold/util/util.go b/pkg/skaffold/util/util.go
@@ -35,6 +35,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	yaml "gopkg.in/yaml.v2"
 )
 
 const (
@@ -250,6 +251,18 @@ func CloneThroughJSON(old interface{}, new interface{}) error {
 	return nil
 }
 
+// CloneThroughYAML marshals the old interface into the new one
+func CloneThroughYAML(old interface{}, new interface{}) error {
+	contents, err := yaml.Marshal(old)
+	if err != nil {
+		return errors.Wrap(err, "unmarshalling properties")
+	}
+	if err := yaml.Unmarshal(contents, new); err != nil {
+		return errors.Wrap(err, "unmarshalling bazel artifact")
+	}
+	return nil
+}
+
 // AbsolutePaths prepends each path in paths with workspace if the path isn't absolute
 func AbsolutePaths(workspace string, paths []string) []string {
 	var p []string