feature(pkg/engine): introduce RenderWithClientProvider

[porridge]

What this PR does / why we need it:

In order to fully test charts that use the lookup function in environments that do not provide a kube API server (e.g. with helmtest), we need to inject a (fake) kubernetes client to the engine.

This change adds a RenderWithClientProvider function parallel to the already existing (somewhat unfortunately named) RenderWithClient, to make this possible.

Special notes for your reviewer:

Existing functionality is untouched.

If applicable:

• this PR contains documentation
• this PR contains unit tests
• this PR has been tested for backwards compatibility: I made sure that the lookup template function still works, by running a locally built helm binary against a chart which uses this functionality, on a kind cluster

Statement coverage with tests:

file	before	after
engine.go	89.2%	88.7%
files.go	85.3%	85.3%
funcs.go	85.4%	85.4%
lookup_func.go	0%	33.3%

[gjenkins8]

can we detail what is happening here please. Any limitations of this functionality? And how come we can't use the resources gvk when invoked from the lookup function.

(the three levels of function closures are bending my brain a little; I think I might be missing something)

[gjenkins8]

Thinking about this more, if a non-Helmtest user started to use this RenderWithDynamicClient interface, they might (I think likely) be surprised by the meta.UnsafeGuessKindToResource usage here. Given my (admittedly superficial) understanding, that this function is trying to "guess" the resource name from the Version, Kind info (meta.UnsafeGuessKindToResource source).

It seems to me, that it would be better to hoist the clientProviderFunc functionality to external Render interface? Rather than passing in a dynamic client, which gets only some of the way. Then needing to build in a workaround to use it.

Something like:

func RenderWithClientLookupFunc(chrt *chart.Chart, values chartutil.Values, clientLookupFunc ClientLookupFunc) (map[string]string, error) {
	return Engine{
		clientLookupFunc: ClientLookupFunc,
	}.Render(chrt, values)
}

...

if e.clientLookupFunc != nil {
    funcMap["lookup"] = return newLookupFunction(clientLookupFunc)
}			

edit: Is there perhaps a better name than RenderWithClientLookupFunc? I choose the obvious here for the example

[porridge]

@gjenkins8 thank you for the prompt review! You formulated clearly the concerns I had myself floating somewhere on the edge of my perception 😅

I updated the PR to follow your advice. This also has the advantages of:

• not introducing any new function closures, at the expense of adding a new struct-based type, which I think is an acceptable overhead in this place,
• eradicating the somewhat dubious UnsafeGuessKindToResource call to the helmtest codebase, where it's appropriate.

[gjenkins8]

Changes LGTMe, will ask another maintainer to review. Thanks for updating the PR

[gjenkins8]

Nice! IMHO it is great to be able to re-write the existing implementation in terms of this new, more generic, interface

[joejulian]

@porridge There are no tests with this. Are the existing tests sufficient? How can I manually validate this?

[gjenkins8]

@porridge There are no tests with this. Are the existing tests sufficient? How can I manually validate this?

I also realized tests were missing. @porridge was going to add after the holidays iirc

[porridge]

@gjenkins8 @joejulian tests added.

I think they now cover everything that is testable without having a real API server connection.

PTAL

[joejulian]

Thanks!