Support Profile patches

Fixes GoogleContainerTools#1536

Signed-off-by: David Gageot <[email protected]>

cmd/skaffold/app/cmd/runner.go

@@ -35,15 +35,16 @@ func newRunner(opts *config.SkaffoldOptions) (*runner.SkaffoldRunner, *latest.Sk
 	}
 
 	config := parsed.(*latest.SkaffoldPipeline)
-	if err := defaults.Set(config); err != nil {
-		return nil, nil, errors.Wrap(err, "setting default values")
-	}
 
 	err = schema.ApplyProfiles(config, opts.Profiles)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "applying profiles")
 	}
 
+	if err := defaults.Set(config); err != nil {
+		return nil, nil, errors.Wrap(err, "setting default values")
+	}
+
 	defaultRepo, err := configutil.GetDefaultRepo(opts.DefaultRepo)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "getting default repo")

integration/examples/annotated-skaffold.yaml

@@ -222,3 +222,8 @@ profiles:
     build:
       googleCloudBuild:
         projectId: k8s-skaffold
+  - name: other
+    # profiles can also patch some values using standard JSON patch notation
+    patches:
+      - path: /build/artifacts/0/docker/dockerfile
+        value: Dockerfile.DEV

pkg/skaffold/schema/latest/config.go

@@ -18,6 +18,7 @@ package latest
 
 import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
+	yamlpatch "github.com/krishicks/yaml-patch"
 )
 
 const Version string = "skaffold/v1beta4"
@@ -242,10 +243,11 @@ type Artifact struct {
 // Profile is additional configuration that overrides default
 // configuration when it is activated.
 type Profile struct {
-	Name   string       `yaml:"name,omitempty"`
-	Build  BuildConfig  `yaml:"build,omitempty"`
-	Test   TestConfig   `yaml:"test,omitempty"`
-	Deploy DeployConfig `yaml:"deploy,omitempty"`
+	Name    string          `yaml:"name,omitempty"`
+	Build   BuildConfig     `yaml:"build,omitempty"`
+	Test    TestConfig      `yaml:"test,omitempty"`
+	Deploy  DeployConfig    `yaml:"deploy,omitempty"`
+	Patches yamlpatch.Patch `yaml:"patches,omitempty"`
 }
 
 type ArtifactType struct {

pkg/skaffold/schema/profile_test.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
+	yamlpatch "github.com/krishicks/yaml-patch"
 )
 
 func TestApplyProfiles(t *testing.T) {
@@ -35,7 +36,6 @@ func TestApplyProfiles(t *testing.T) {
 			description: "unknown profile",
 			config:      config(),
 			profile:     "profile",
-			expected:    config(),
 			shouldErr:   true,
 		},
 		{
@@ -52,7 +52,10 @@ func TestApplyProfiles(t *testing.T) {
 					Build: latest.BuildConfig{
 						BuildType: latest.BuildType{
 							GoogleCloudBuild: &latest.GoogleCloudBuild{
-								ProjectID: "my-project",
+								ProjectID:   "my-project",
+								DockerImage: "gcr.io/cloud-builders/docker",
+								MavenImage:  "gcr.io/cloud-builders/mvn",
+								GradleImage: "gcr.io/cloud-builders/gradle",
 							},
 						},
 					},
@@ -103,17 +106,25 @@ func TestApplyProfiles(t *testing.T) {
 					Name: "profile",
 					Build: latest.BuildConfig{
 						Artifacts: []*latest.Artifact{
-							{ImageName: "image"},
-							{ImageName: "imageProd"},
+							{ImageName: "image", Workspace: ".", ArtifactType: latest.ArtifactType{
+								DockerArtifact: &latest.DockerArtifact{
+									DockerfilePath: "Dockerfile.DEV",
+								},
+							}},
+							{ImageName: "imageProd", Workspace: ".", ArtifactType: latest.ArtifactType{
+								DockerArtifact: &latest.DockerArtifact{
+									DockerfilePath: "Dockerfile.DEV",
+								},
+							}},
 						},
 					},
 				}),
 			),
 			expected: config(
 				withLocalBuild(
 					withGitTagger(),
-					withDockerArtifact("image", ".", "Dockerfile"),
-					withDockerArtifact("imageProd", ".", "Dockerfile"),
+					withDockerArtifact("image", ".", "Dockerfile.DEV"),
+					withDockerArtifact("imageProd", ".", "Dockerfile.DEV"),
 				),
 				withKubectlDeploy("k8s/*.yaml"),
 			),
@@ -142,13 +153,66 @@ func TestApplyProfiles(t *testing.T) {
 				withHelmDeploy(),
 			),
 		},
+		{
+			description: "patch Dockerfile",
+			profile:     "profile",
+			config: config(
+				withLocalBuild(
+					withGitTagger(),
+					withDockerArtifact("image", ".", "Dockerfile"),
+				),
+				withKubectlDeploy("k8s/*.yaml"),
+				withProfiles(latest.Profile{
+					Name: "profile",
+					Patches: yamlpatch.Patch{{
+						Path:  "/build/artifacts/0/docker/dockerfile",
+						Value: yamlpatch.NewNode(str("Dockerfile.DEV")),
+					}},
+				}),
+			),
+			expected: config(
+				withLocalBuild(
+					withGitTagger(),
+					withDockerArtifact("image", ".", "Dockerfile.DEV"),
+				),
+				withKubectlDeploy("k8s/*.yaml"),
+			),
+		},
+		{
+			description: "invalid patch path",
+			profile:     "profile",
+			config: config(
+				withLocalBuild(
+					withGitTagger(),
+					withDockerArtifact("image", ".", "Dockerfile"),
+				),
+				withKubectlDeploy("k8s/*.yaml"),
+				withProfiles(latest.Profile{
+					Name: "profile",
+					Patches: yamlpatch.Patch{{
+						Path: "/unknown",
+						Op:   "replace",
+					}},
+				}),
+			),
+			shouldErr: true,
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
 			err := ApplyProfiles(test.config, []string{test.profile})
 
-			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, test.config)
+			if test.shouldErr {
+				testutil.CheckError(t, test.shouldErr, err)
+			} else {
+				testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, test.config)
+			}
 		})
 	}
 }
+
+func str(value string) *interface{} {
+	var v interface{} = value
+	return &v
+}

pkg/skaffold/schema/profiles.go

@@ -21,33 +21,32 @@ import (
 	"reflect"
 	"strings"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	yaml "gopkg.in/yaml.v2"
 )
 
 // ApplyProfiles returns configuration modified by the application
 // of a list of profiles.
 func ApplyProfiles(c *latest.SkaffoldPipeline, profiles []string) error {
 	byName := profilesByName(c.Profiles)
+
 	for _, name := range profiles {
 		profile, present := byName[name]
 		if !present {
 			return fmt.Errorf("couldn't find profile %s", name)
 		}
 
-		applyProfile(c, profile)
-	}
-	if err := defaults.Set(c); err != nil {
-		return errors.Wrap(err, "applying default values")
+		if err := applyProfile(c, profile); err != nil {
+			return errors.Wrapf(err, "appying profile %s", name)
+		}
 	}
 
 	return nil
 }
 
-func applyProfile(config *latest.SkaffoldPipeline, profile latest.Profile) {
+func applyProfile(config *latest.SkaffoldPipeline, profile latest.Profile) error {
 	logrus.Infof("applying profile: %s", profile.Name)
 
 	// this intentionally removes the Profiles field from the returned config
@@ -58,6 +57,31 @@ func applyProfile(config *latest.SkaffoldPipeline, profile latest.Profile) {
 		Deploy:     overlayProfileField(config.Deploy, profile.Deploy).(latest.DeployConfig),
 		Test:       overlayProfileField(config.Test, profile.Test).(latest.TestConfig),
 	}
+
+	if len(profile.Patches) == 0 {
+		return nil
+	}
+
+	// Default patch operation to `replace`
+	for i, p := range profile.Patches {
+		if p.Op == "" {
+			p.Op = "replace"
+			profile.Patches[i] = p
+		}
+	}
+
+	// Apply profile patches
+	buf, err := yaml.Marshal(*config)
+	if err != nil {
+		return err
+	}
+
+	buf, err = profile.Patches.Apply(buf)
+	if err != nil {
+		return err
+	}
+
+	return yaml.Unmarshal(buf, config)
 }
 
 func profilesByName(profiles []latest.Profile) map[string]latest.Profile {