Strategic merge patches change the order of containers

[shapirus]

What happened?

If only one container out of multiple is referenced in the patch, then the order of the containers in the output is changed.

Refer to #3912, it has all the info.

That issue was auto-closed for inactivity, but the bug still exists.

What did you expect to happen?

The original order of containers has to be preserved.

How can we reproduce it (as minimally and precisely as possible)?

See the original ticket.

Expected output

See the original ticket.

Actual output

See the original ticket.

Kustomize version

5.2.1

Operating system

None

[k8s-ci-robot]

This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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-sigs/prow repository.

[tsekine354]

Not just container, but the order of env variables is also important.

If you use $(VAR_NAME) reference, VAR_NAME must be declared before its reference, as explained in https://kubernetes.io/docs/tasks/inject-data-application/define-interdependent-environment-variables/#define-an-environment-dependent-variable-for-a-container

But strategic merge patch changes the order of env variable, then it could brake dependent variable reference.

[shapirus]

Correct. It is apparently the case for list elements in general.

[tsekine354]

As per source code reading + some help by debugger, I guess elementValues() in https://github.com/kubernetes-sigs/kustomize/blob/master/kyaml/yaml/walk/associative_sequence.go#L312 is the cause.

In strategic patch merge mode, new items might be supposed to get inserted first, as per https://github.com/kubernetes-sigs/kustomize/blob/master/api/filters/patchstrategicmerge/patchstrategicmerge.go#L25

				ListIncreaseDirection: yaml.MergeOptionsListPrepend,

Then, elementValues() unconditionally read second source first

	beginIdx := 0
	if l.MergeOptions.ListIncreaseDirection == yaml.MergeOptionsListPrepend {
		beginIdx = 1
	}
	for i := range l.Sources {
		src := l.Sources[(i+beginIdx)%len(l.Sources)]
...

It should read Sources sequentially always. Then, if there are new items in Sources[1], insert those items into top of returnValues.

[tsekine354]

tsekine354@1ad9646
may be able to the fix this issue. But unfortunately, there are many unrelated test cases which depend on current behavior. I need feedback from project team before fixing those test cases.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale