Use reflection to overlay profile onto config

pkg/skaffold/build/gcb/cloud_build.go

@@ -95,7 +95,7 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, tagger tag.T
 		return "", errors.Wrapf(err, "getting build ID from op")
 	}
 	logsObject := fmt.Sprintf("log-%s.txt", remoteID)
-	color.Default.Fprintf(out, "Logs at available at \nhttps://console.cloud.google.com/m/cloudstorage/b/%s/o/%s\n", cbBucket, logsObject)
+	color.Default.Fprintf(out, "Logs are available at \nhttps://console.cloud.google.com/m/cloudstorage/b/%s/o/%s\n", cbBucket, logsObject)
 	var imageID string
 	offset := int64(0)
 watch:

pkg/skaffold/config/profile_test.go

@@ -26,21 +26,22 @@ import (
 func TestApplyProfiles(t *testing.T) {
 	tests := []struct {
 		description string
-		config      SkaffoldConfig
+		config      *SkaffoldConfig
 		profile     string
-		expected    SkaffoldConfig
+		expected    *SkaffoldConfig
 		shouldErr   bool
 	}{
 		{
 			description: "unknown profile",
-			config:      SkaffoldConfig{},
+			config:      &SkaffoldConfig{},
 			profile:     "profile",
+			expected:    &SkaffoldConfig{},
 			shouldErr:   true,
 		},
 		{
 			description: "build type",
 			profile:     "profile",
-			config: SkaffoldConfig{
+			config: &SkaffoldConfig{
 				Build: v1alpha2.BuildConfig{
 					Artifacts: []*v1alpha2.Artifact{
 						{ImageName: "image"},
@@ -61,7 +62,7 @@ func TestApplyProfiles(t *testing.T) {
 					},
 				},
 			},
-			expected: SkaffoldConfig{
+			expected: &SkaffoldConfig{
 				Build: v1alpha2.BuildConfig{
 					Artifacts: []*v1alpha2.Artifact{
 						{
@@ -89,7 +90,7 @@ func TestApplyProfiles(t *testing.T) {
 		{
 			description: "tag policy",
 			profile:     "dev",
-			config: SkaffoldConfig{
+			config: &SkaffoldConfig{
 				Build: v1alpha2.BuildConfig{
 					Artifacts: []*v1alpha2.Artifact{
 						{ImageName: "image"},
@@ -106,7 +107,7 @@ func TestApplyProfiles(t *testing.T) {
 					},
 				},
 			},
-			expected: SkaffoldConfig{
+			expected: &SkaffoldConfig{
 				Build: v1alpha2.BuildConfig{
 					Artifacts: []*v1alpha2.Artifact{
 						{
@@ -130,7 +131,7 @@ func TestApplyProfiles(t *testing.T) {
 		{
 			description: "artifacts",
 			profile:     "profile",
-			config: SkaffoldConfig{
+			config: &SkaffoldConfig{
 				Build: v1alpha2.BuildConfig{
 					Artifacts: []*v1alpha2.Artifact{
 						{ImageName: "image"},
@@ -150,7 +151,7 @@ func TestApplyProfiles(t *testing.T) {
 					},
 				},
 			},
-			expected: SkaffoldConfig{
+			expected: &SkaffoldConfig{
 				Build: v1alpha2.BuildConfig{
 					Artifacts: []*v1alpha2.Artifact{
 						{
@@ -183,7 +184,7 @@ func TestApplyProfiles(t *testing.T) {
 		{
 			description: "deploy",
 			profile:     "profile",
-			config: SkaffoldConfig{
+			config: &SkaffoldConfig{
 				Build: v1alpha2.BuildConfig{},
 				Deploy: v1alpha2.DeployConfig{
 					DeployType: v1alpha2.DeployType{
@@ -201,7 +202,7 @@ func TestApplyProfiles(t *testing.T) {
 					},
 				},
 			},
-			expected: SkaffoldConfig{
+			expected: &SkaffoldConfig{
 				Build: v1alpha2.BuildConfig{
 					TagPolicy: v1alpha2.TagPolicy{
 						GitTagger: &v1alpha2.GitTagger{},

pkg/skaffold/schema/util/util.go

@@ -16,6 +16,11 @@ limitations under the License.
 
 package util
 
+import (
+	"reflect"
+	"strings"
+)
+
 type VersionedConfig interface {
 	GetVersion() string
 	Parse([]byte, bool) error
@@ -24,3 +29,12 @@ type VersionedConfig interface {
 type Config interface {
 	Parse([]byte) (VersionedConfig, error)
 }
+
+func IsOneOf(field reflect.StructField) bool {
+	for _, tag := range strings.Split(field.Tag.Get("yamltags"), ",") {
+		if tag == "oneOf" {
+			return true
+		}
+	}
+	return false
+}

pkg/skaffold/schema/v1alpha2/config.go

@@ -45,10 +45,10 @@ type BuildConfig struct {
 
 // TagPolicy contains all the configuration for the tagging step
 type TagPolicy struct {
-	GitTagger         *GitTagger         `yaml:"gitCommit"`
-	ShaTagger         *ShaTagger         `yaml:"sha256"`
-	EnvTemplateTagger *EnvTemplateTagger `yaml:"envTemplate"`
-	DateTimeTagger    *DateTimeTagger    `yaml:"dateTime"`
+	GitTagger         *GitTagger         `yaml:"gitCommit" yamltags:"oneOf"`
+	ShaTagger         *ShaTagger         `yaml:"sha256" yamltags:"oneOf"`
+	EnvTemplateTagger *EnvTemplateTagger `yaml:"envTemplate" yamltags:"oneOf"`
+	DateTimeTagger    *DateTimeTagger    `yaml:"dateTime" yamltags:"oneOf"`
 }
 
 // ShaTagger contains the configuration for the SHA tagger.
@@ -71,9 +71,9 @@ type DateTimeTagger struct {
 // BuildType contains the specific implementation and parameters needed
 // for the build step. Only one field should be populated.
 type BuildType struct {
-	LocalBuild       *LocalBuild       `yaml:"local"`
-	GoogleCloudBuild *GoogleCloudBuild `yaml:"googleCloudBuild"`
-	KanikoBuild      *KanikoBuild      `yaml:"kaniko"`
+	LocalBuild       *LocalBuild       `yaml:"local" yamltags:"oneOf"`
+	GoogleCloudBuild *GoogleCloudBuild `yaml:"googleCloudBuild" yamltags:"oneOf"`
+	KanikoBuild      *KanikoBuild      `yaml:"kaniko" yamltags:"oneOf"`
 }
 
 // LocalBuild contains the fields needed to do a build on the local docker daemon
@@ -112,9 +112,9 @@ type DeployConfig struct {
 // DeployType contains the specific implementation and parameters needed
 // for the deploy step. Only one field should be populated.
 type DeployType struct {
-	HelmDeploy      *HelmDeploy      `yaml:"helm"`
-	KubectlDeploy   *KubectlDeploy   `yaml:"kubectl"`
-	KustomizeDeploy *KustomizeDeploy `yaml:"kustomize"`
+	HelmDeploy      *HelmDeploy      `yaml:"helm" yamltags:"oneOf"`
+	KubectlDeploy   *KubectlDeploy   `yaml:"kubectl" yamltags:"oneOf"`
+	KustomizeDeploy *KustomizeDeploy `yaml:"kustomize" yamltags:"oneOf"`
 }
 
 // KubectlDeploy contains the configuration needed for deploying with `kubectl apply`
@@ -202,8 +202,8 @@ type Profile struct {
 }
 
 type ArtifactType struct {
-	DockerArtifact *DockerArtifact `yaml:"docker"`
-	BazelArtifact  *BazelArtifact  `yaml:"bazel"`
+	DockerArtifact *DockerArtifact `yaml:"docker" yamltags:"oneOf"`
+	BazelArtifact  *BazelArtifact  `yaml:"bazel" yamltags:"oneOf"`
 }
 
 type DockerArtifact struct {

pkg/skaffold/schema/v1alpha2/profiles.go

@@ -18,47 +18,43 @@ package v1alpha2
 
 import (
 	"fmt"
+	"reflect"
 
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	yaml "gopkg.in/yaml.v2"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
 )
 
 // ApplyProfiles returns configuration modified by the application
 // of a list of profiles.
 func (c *SkaffoldConfig) ApplyProfiles(profiles []string) error {
-	var err error
-
 	byName := profilesByName(c.Profiles)
 	for _, name := range profiles {
 		profile, present := byName[name]
 		if !present {
 			return fmt.Errorf("couldn't find profile %s", name)
 		}
 
-		err = applyProfile(c, profile)
-		if err != nil {
-			return errors.Wrapf(err, "applying profile %s", name)
-		}
+		applyProfile(c, profile)
 	}
-
-	c.Profiles = nil
 	if err := c.setDefaultValues(); err != nil {
 		return errors.Wrap(err, "applying default values")
 	}
 
 	return nil
 }
 
-func applyProfile(config *SkaffoldConfig, profile Profile) error {
-	logrus.Infof("Applying profile: %s", profile.Name)
+func applyProfile(config *SkaffoldConfig, profile Profile) {
+	logrus.Infof("applying profile: %s", profile.Name)
 
-	buf, err := yaml.Marshal(profile)
-	if err != nil {
-		return err
+	// this intentionally removes the Profiles field from the returned config
+	*config = SkaffoldConfig{
+		APIVersion: config.APIVersion,
+		Kind:       config.Kind,
+		Build:      overlayProfileField(config.Build, profile.Build).(BuildConfig),
+		Deploy:     overlayProfileField(config.Deploy, profile.Deploy).(DeployConfig),
 	}
-
-	return yaml.Unmarshal(buf, config)
 }
 
 func profilesByName(profiles []Profile) map[string]Profile {
@@ -68,3 +64,62 @@ func profilesByName(profiles []Profile) map[string]Profile {
 	}
 	return byName
 }
+
+// if we find a oneOf tag, the fields in this struct are themselves pointers to structs,
+// but should be treated as values. the first non-nil one we find is what we should use.
+func overlayOneOfField(config interface{}, profile interface{}) interface{} {
+	v := reflect.ValueOf(profile) // the profile itself
+	t := reflect.TypeOf(profile)  // the type of the profile, used for getting struct field types
+	for i := 0; i < v.NumField(); i++ {
+		fieldType := t.Field(i)              // the field type (e.g. 'LocalBuild' for BuildConfig)
+		fieldValue := v.Field(i).Interface() // the value of the field itself
+
+		if fieldValue != nil && !reflect.ValueOf(fieldValue).IsNil() {
+			ret := reflect.New(t)                                                   // New(t) returns a Value representing pointer to new zero value for type t
+			ret.Elem().FieldByName(fieldType.Name).Set(reflect.ValueOf(fieldValue)) // set the value
+			return reflect.Indirect(ret).Interface()                                // since ret is a pointer, dereference it
+		}
+	}
+	// if we're here, we didn't find any values set in the profile config. just return the original.
+	logrus.Infof("no values found in profile for field %s, using original config values", t.Name())
+	return config
+}
+
+func overlayStructField(config interface{}, profile interface{}) interface{} {
+	// we already know the top level fields for whatever struct we have are themselves structs
+	// (and not one-of values), so we need to recursively overlay them
+	configValue := reflect.ValueOf(config)
+	profileValue := reflect.ValueOf(profile)
+	t := reflect.TypeOf(profile)
+	finalConfig := reflect.New(t)
+
+	for i := 0; i < profileValue.NumField(); i++ {
+		fieldType := t.Field(i)
+		overlay := overlayProfileField(configValue.Field(i).Interface(), profileValue.Field(i).Interface())
+		finalConfig.Elem().FieldByName(fieldType.Name).Set(reflect.ValueOf(overlay))
+	}
+	return reflect.Indirect(finalConfig).Interface() // since finalConfig is a pointer, dereference it
+}
+
+func overlayProfileField(config interface{}, profile interface{}) interface{} {
+	v := reflect.ValueOf(profile) // the profile itself
+	t := reflect.TypeOf(profile)  // the type of the profile, used for getting struct field types
+	logrus.Debugf("overlaying profile on config for field %s", t.Name())
+	switch v.Kind() {
+	case reflect.Struct:
+		// check the first field of the struct for a oneOf yamltag.
+		if util.IsOneOf(t.Field(0)) {
+			return overlayOneOfField(config, profile)
+		}
+		return overlayStructField(config, profile)
+	case reflect.Slice:
+		// either return the values provided in the profile, or the original values if none were provided.
+		if v.Len() == 0 {
+			return config
+		}
+		return v.Interface()
+	default:
+		logrus.Warnf("unknown field type in profile overlay: %s. falling back to original config values", v.Kind())
+		return config
+	}
+}

pkg/skaffold/watch/changes.go

@@ -51,7 +51,7 @@ func lastModified(paths []string) (time.Time, error) {
 	for _, path := range paths {
 		stat, err := os.Stat(path)
 		if err != nil {
-			return last, errors.Wrapf(err, "unable to stat file %s: %v")
+			return last, errors.Wrapf(err, "unable to stat file %s", path)
 		}
 
 		if stat.IsDir() {