add flexibility around use of post-renderer

[joelanford]

With the helm-operator pkg/client libraries today, the amount of control users have for configuring post renders is somewhat minimal:

1. It isn't possible to turn off the owner reference injection
2. It is difficult to use more than the default owner ref injecting post-render

This PR adds a new chainedPostRenderer and some helper functional options that simplify the process of overriding or chaining more post renderers during installs and upgrades.

This PR also exposes the default post renderer as an overrideable function, making it possible to disable the default or provide an alternative.

This should help resolve and issue we are having in rukpak: operator-framework/rukpak#517 (comment)

Lastly, this PR bumps the repo to Go 1.18 (which was necessary to implement a generic slice concat function)

Closes #194

[coveralls]

Pull Request Test Coverage Report for Build 2974510048

• 83 of 102 (81.37%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.09%) to 89.372%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/client/postrenderer.go	68	87	78.16%

Totals
Change from base Build 2834234345:	0.09%
Covered Lines:	1623
Relevant Lines:	1816

---

💛 - Coveralls

[porridge]

LGTM, modulo an apparent minor logic error in a test and some nitpicks.

[porridge]

Would this work?

Suggested change

	out := make([]T, len(s1))
	out := make([]T, len(s1)+len(s2))

[joelanford]

As it turns out, I don't think concat is necessary at all: https://go.dev/play/p/sqvcMv8lStZ

[porridge]

I thought its purpose was to separate the inputs from the outputs to prevent unexpected "action at a distance"?
With pure append this can happen depending in case the first slice has capacity.

[porridge]

Here the capacity of the default... fields should in theory always match their len, so perhaps this is not a concern? WDYT @joelanford ?

[joelanford]

Ah you're right! I knew there was an edge case here. I think it's best to just go ahead and add the concat back in then. This seems like a potential footgun for our future selves if we don't get this right.

[joelanford]

Okay latest commit has a re-added concat method that I think has the optimal memory allocation.

[porridge]

Wow, TIL I learned of a new way to extend a slice.

[joelanford]

This ends up being a potentially recursive sort of thing. pr might itself be a chainedPostRenderer

[porridge]

Well, if chainedPostRenderer was a struct type, yes. But since it's a slice type, then after multiple calls eventually you just end up with a single long slice, right?

[joelanford]

chainedPostRenderer is a slice of postrender.PostRenderer. The way this is setup now, we always return a wrapper chainedPostRenderer with exactly length 2. It just so happens that pr might already be a chainedPostRenderer, and if so, it is also of length 2.

So after two appends, you'd end up with:

out := chainedPostRender{
	chainedPostRender{
		defaultPostRenderer,
		append1,
	},
	append2,
}

not

out := chainedPostRender{
	defaultPostRenderer,
	append1,
	append2,
}

[porridge]

Right, we get a slice whose first element is a slice. The underlying type of pr does not matter here...

[porridge]

So, should we do anything about this? If user stacks up four renderers, we end up with this leaning tree of two-element ChainedPostRenderers, and the %d on line 98 is going to be somewhat misleading/confusing - they will get a sequence of error messages all mentioning a combination of 1 and 0...
Perhaps use append if pr successfully type-asserts as a chainedPostRenderer?

[joelanford]

the %d on line 98 is going to be somewhat misleading/confusing

good point!

i had the type assertion thing in an early draft and dropped it for simplicity, but it does seems worthwhile, so i'll add that back in. thanks!