add `edit fix` for patchesStrategicMerge to patches

[koba1t]

fix https://github.com/kubernetes-sigs/kustomize/issues/4706

[koba1t]

/assign @natasha41575

[k8s-ci-robot]

@koba1t: This PR has multiple commits, and the default merge method is: merge. You can request commits to be squashed using the label: tide/merge-method-squash

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[koba1t]

hi @natasha41575 I fixed a few points from the comments. Could you check it if you have time?

[koba1t]

hi @natasha41575 I fixed a few points. Please recheck it in your free time.

[natasha41575]

@annasong20 Do you mind giving this PR a review? And as a final check, it would be helpful if you could pull down and build the PR and verify that it does the right thing. Btw, when you are testing, it is expected that it won't work unless the patch file exists.

[annasong20]

@natasha41575 The PR binary works as expected on the following setup:

kustomization.yaml

patchesStrategicMerge:
- deploy.yaml
- |-
  apiVersion: apps/v1
  kind: Deployment
  metadata:
    name: nginx
  spec:
    template:
      spec:
        containers:
          - name: nginx
            env:
              - name: CONFIG_FILE_PATH
                value: home.yaml
patchesJson6902:
- path: patch1.yaml
  target:
    kind: Deployment
patches:
- path: patch2.yaml
- patch: |-
    apiVersion: apps/v1
    kind: Deployment
    metadata:
      name: nginx
    spec:
      template:
        spec:
          containers:
            - name: nginx
              env:
                - name: CONFIG_FILE_PATH_2
                  value: home.yaml

deploy.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx
spec:
  template:
    spec:
      containers:
      - name: nginx
        env:
        - name: CONFIG_FILE_PATH_3
          value: home.yaml

On a different note, I think it'd be helpful for the current code in fix_test.go to check for more than just an error. We should care about warnings that the build failed or that the build output is different. I know they're currently printed because we don't provide resources that match the patch targets, but the warnings could technically be the result of us "fixing" the kustomization incorrectly. It doesn't seem safe to ignore them.

Something else that I noticed while testing the binary, but of lesser importance, is that we don't inform users if their initial build was faulty. I found this very confusing because when I saw this warning, I'd assume edit fix introduced an inconsistency to my kustomization, but it turns out that it was the original kustomization that had issues.

Not saying we need to address these concerns in this PR, since the code wasn't introduced here.

[koba1t]

I know you didn't change this line, but could you add a newline to the end of it?

Do you think this commit fixes that?

[koba1t]

Hi, @annasong20 Thanks for your review. I think I completed fixing the problems from your comment. Please recheck it when you have time.

[annasong20]

I know you didn't change this line, but could you add a newline to the end of it?

Do you think this commit fixes that?

Yes, perfect! Looks good to me. Will defer to @natasha41575 to approve.

[koba1t]

@natasha41575 Could you recheck it?

Hi, @annasong20 Thanks for your review.

[annasong20]

/lgtm

[natasha41575]

So sorry for the delay! Reviewing this now

[natasha41575]

Thanks so much for the fix! This means we can get https://github.com/kubernetes-sigs/kustomize/pull/4723 in soon as well. This is great. Please feel free to reach out if you are looking for more issues to pick up.

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: koba1t, natasha41575

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [natasha41575]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment