fix error message using replacement wildcard and create option

[koba1t]

Fix error message using both wildcard and create option on replacement.

Temporarily fix to #4561.

[k8s-ci-robot]

Hi @koba1t. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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]

/assign @natasha41575

[natasha41575]

/ok-to-test

[natasha41575]

could you please also add the test from https://github.com/kubernetes-sigs/kustomize/pull/4574/files that I added to replacement_test.go, called "create Job spec"?

I pulled your branch and it looks like something funny is going on when running with that test, it would be great if you could help fix that.

[natasha41575]

For that case, I think we can do something in the code that's here:

kustomize/api/filters/replacement/replacement.go

Lines 119 to 123 in e5041ba

	if target.Options != nil && target.Options.Create {
	t, err = node.Pipe(yaml.LookupCreate(value.YNode().Kind, fieldPath...))
	} else {
	t, err = node.Pipe(&yaml.PathMatcher{Path: fieldPath})
	}

We can potentially try something like this:

		if target.Options != nil && target.Options.Create && !fieldPathAlreadyExistsInTarget {
			t, err = node.Pipe(yaml.LookupCreate(value.YNode().Kind, fieldPath...))
		} else {
			t, err = node.Pipe(&yaml.PathMatcher{Path: fieldPath})
		}

to see if it fixes the problem.

[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.

[KnVerey]

Won't it be possible for someone to be attempting to use PathGetter with an asterisk but without the create option? I think this error message needs to be about wildcard support rather than create mode specifically because of that (and replacements may need to augment the error to clarify what the problem is in that specific flow).

[natasha41575]

+1

The other option is to move the check into the replacement filter, in replacements.go you can throw this error if create is true and the path contains *.

Also suggest removing the word "now"

[koba1t]

I think I fix it to 01ab069

[natasha41575]

You can just add //nolint:goerr113 to the end of this line. I don't think we need to worry about this dynamic error here since the rest of the code is sprinkled with dynamic errors.

[koba1t]

Thanks!

[natasha41575]

/lgtm
/approve

Thanks for this PR!

Do you have the bandwidth to work on what I described in this comment #4577 (comment) over the next couple of weeks?

If so, please submit a PR with the test and a fix. Otherwise let me know I will probably try to work on it myself when I can.

[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