diff --git a/pkg/skaffold/schema/profiles.go b/pkg/skaffold/schema/profiles.go index 7a2b8184d77..27181d357aa 100644 --- a/pkg/skaffold/schema/profiles.go +++ b/pkg/skaffold/schema/profiles.go @@ -156,12 +156,18 @@ func applyProfile(config *latest.SkaffoldPipeline, profile latest.Profile) error op = "replace" } - patches = append(patches, yamlpatch.Operation{ + patch := yamlpatch.Operation{ Op: yamlpatch.Op(op), Path: yamlpatch.OpPath(patch.Path), From: yamlpatch.OpPath(patch.From), Value: patch.Value, - }) + } + + if !tryPatch(patch, buf) { + return fmt.Errorf("invalid path: %s", patch.Path) + } + + patches = append(patches, patch) } buf, err = yamlpatch.Patch(patches).Apply(buf) @@ -172,6 +178,20 @@ func applyProfile(config *latest.SkaffoldPipeline, profile latest.Profile) error return yaml.Unmarshal(buf, config) } +// tryPatch is here to verify patches one by one before we +// apply them because yamlpatch.Patch is known to panic when a path +// is not valid. +func tryPatch(patch yamlpatch.Operation, buf []byte) (valid bool) { + defer func() { + if errPanic := recover(); errPanic != nil { + valid = false + } + }() + + _, err := yamlpatch.Patch([]yamlpatch.Operation{patch}).Apply(buf) + return err == nil +} + func profilesByName(profiles []latest.Profile) map[string]latest.Profile { byName := make(map[string]latest.Profile) for _, profile := range profiles { diff --git a/pkg/skaffold/schema/profiles_test.go b/pkg/skaffold/schema/profiles_test.go index e6a77b48574..ea5ab41f08f 100644 --- a/pkg/skaffold/schema/profiles_test.go +++ b/pkg/skaffold/schema/profiles_test.go @@ -70,6 +70,34 @@ profiles: testutil.CheckDeepEqual(t, "Dockerfile.second", pipeline.Build.Artifacts[1].DockerArtifact.DockerfilePath) } +func TestApplyInvalidPatch(t *testing.T) { + config := `build: + artifacts: + - image: example +profiles: +- name: patches + patches: + - path: /build/artifacts/0/image/ + value: replacement +` + + tmp, cleanup := testutil.NewTempDir(t) + defer cleanup() + + yaml := fmt.Sprintf("apiVersion: %s\nkind: Config\n%s", latest.Version, config) + tmp.Write("skaffold.yaml", yaml) + + parsed, err := ParseConfig(tmp.Path("skaffold.yaml"), false) + testutil.CheckError(t, false, err) + + pipeline := parsed.(*latest.SkaffoldPipeline) + err = ApplyProfiles(pipeline, &cfg.SkaffoldOptions{ + Profiles: []string{"patches"}, + }) + + testutil.CheckErrorAndDeepEqual(t, true, err, "appying profile patches: invalid path: /build/artifacts/0/image/", err.Error()) +} + func TestApplyProfiles(t *testing.T) { tests := []struct { description string