Support appending to list via IstioOperator component patches

[shakti-das]

Signed-off-by: Shakti shaktiprakash.das@salesforce.com

Please provide a description for what this PR is for.
Fixes #24258
Adds support for appending to a list via IstioOperator component patch. We (Salesforce) need this feature to append additional containers to Istiod Deployment. This feature will also help in adding additional command/args values to the list instead of the need to replace the entire command/args list.

And to help us figure out who should review this PR, please
put an X in all the areas that this PR affects.

[x] Configuration Infrastructure
[ ] Docs
[x] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

[shakti-das]

/ok-to-test

[pnambiarsf]

@howardjohn - Could you please review and approve ?

[GregHanson]

is appending to the next index the way to create a new leaf? https://istio.io/docs/reference/config/istio.operator.v1alpha1/#K8sObjectOverlay-PathValue

Value to add, delete or replace. For add, the path should be a new leaf.

The provided examples don't show any add examples. Which is standard practice? When referencing a list via overlay, and specifying a new element, should it just be appended to the existing list or overwrite the list with a new single element list?

[shakti-das]

We have indexed based access to a list. So, if the index doesn't exist then adding to the list would make sense. It will be the same behavior that we have with map, i.e, if something is not found then insert, else replace.

To replace the entire list, we can stop before the indexing into the list and that would replace the entire list.

[shakti-das]

Please note that prior to the commit d464fb8, this was the pre-existing default behavior for appending to list.

The commit mentioned above added the createMissing flag check for list append logic too:

istio/operator/pkg/tpath/tree.go

Lines 183 to 191 in 12e839f

	var foundNode interface{}
	if idx >= len(lst) {
	if !createMissing {
	return nil, false, fmt.Errorf("index %d exceeds list length %d at path %s", idx, len(lst), remainPath)
	}
	foundNode = make(map[string]interface{})
	} else {
	foundNode = lst[idx]
	}

This is the point where the behavior changed to not append items to list and instead throw error exceeds list length.

[shakti-das]

This change in behavior is currently blocking us from using IstioOperator to deploy Istio.

[toonsevrin]

Is there a way to blindly add based on a key? eg. path: spec.template.spec.containers.[name:my_sidecar] instead of using an index?

Using an index hardly seems safe as it may override a sidecar that is added during an istio upgrade.

[shakti-das]

It could still override the sidecar if name:my_sidecar already exists.

[toonsevrin]

Also true, I've just played around and seems like using a very high index is permitted, I'll just do that, consider this question resolved 👍