Skaffold: `skaffold dev` breaks on kustomize new `patches:` field format

[strikeout]

Actual behavior

Running skaffold dev with a kustomization.yaml file containing the patches: field to target multiple objects (https://github.com/kubernetes-sigs/kustomize/blob/master/docs/plugins/builtins.md#field-name-patches) fails with:

FATA[0000] watching files for deployer: listing files: listing files: yaml: unmarshal errors:
  line xx: cannot unmarshal !!map into string

In the skaffold kustomize example (https://github.com/GoogleContainerTools/skaffold/blob/master/examples/kustomize/kustomization.yaml) it is clear that skaffold expects the "old" format of defining patches just by filename:

patches: 
- file.yml
- another-file.yml

The new format is more elaborate defining targets and allows for inline patches and can look like this:

# from https://github.com/kubernetes-sigs/kustomize/blob/master/docs/plugins/builtins.md#field-name-patches
patches:
- path: patch.yaml
  target:
    group: apps
    version: v1
    kind: Deployment
    name: deploy.*
    labelSelector: "env=dev"
    annotationSelector: "zone=west"
- patch: |-
    - op: replace
      path: /some/existing/path
      value: new value
  target:
    kind: MyKind
    labelSelector: "env=dev"       

Running skaffold run or skaffold deploy strangely works and produces valid yaml.
Running kustomize directly works and produces valid yaml as well.

Expected behavior

skaffold dev with a kustomization target is expected not to break on usage of the new patches field format.

Information

• Skaffold version: v0.38.0
• Kustomize version: v3.2.0
• Operating system: macOS

[tejal29]

@strikeout thanks for opening the bug,. We should support new kustomize patch format.

[priyawadhwa]

Hey @strikeout my guess is that skaffold dev is failing because it tries to determine file dependencies for the file watcher, which skaffold run and skaffold deploy don't have to do.

The fix for this would lie somewhere in dependenciesForKustomize; we would need some way of applying the patches, and then determining which files are dependencies to be watched.

If you, or anyone else, would you be interested in opening a PR for this, please comment here. I can assign you to the issue & review any PRs.

[dgageot]

@tejal29 @priyawadhwa I wonder if we should try to support both latest and older kustomization files. Or maybe it's ok to just support the latest format.

[dgageot]

My preference would be to support only the latest format and print a warning when the file can't be parsed. This won't break deployments but if users want to skaffold dev to detects changes to referenced files, they would have to upgrade to a more recent kustomize.

[strikeout]

Can this be escalated in priority?
I think it is paramount to fully support the latest valid kustomize syntax.

[strikeout]

@balopat @tejal29
Still an issue in skaffold v1.2.0

[grumps]

PR #3663 supports the "extended" patches.