Enable receiving a rest config on TestSuite

[alifelan]

Signed-off-by: Ali Felan alifelan@google.com

What this PR does / why we need it:
This adds an extra option to TestSuite, which allows KUTTL to run with a rest config, opening possibilities for customization on Go.

This is still missing testing and documentation, but wanted to share it to get feedback. My first try was making the config an exported name in harness, but that required a lot of changes in names (config to Config and Config() to GetConfig()). Adding it to TestSuite was easier, but it's the only non primitive type there.

Fixes #347

[kensipe]

interesting @alifelan and thank you! Apologies for missing the submitted issue which provided some of the context. let's continue a conversation on the issue and return here. It's a fairly small PR.. just need to understand the consequences of it. Does this PR resolve the env issue for you?

[alifelan]

Hey @kensipe! No problem, hope it makes more sense with the context in the issue, the PR is actually really small. But yeah, by doing this we can have KUTTL connect to the cluster using the rest config.

[kensipe]

@alifelan ok... enough thinking... more doing (on my part). I can't think of an issue here... it is an optional element. As noted below, I have a preference to have config selection fall into the same logic.. additionally this will not immediately return but will create a kubeconfig file which is used for some of the commands if included in the kuttl tests.

It would be great to have a test for this need feature as well.
This change + a test for the new configuration and we can land this.

Is that something you are up for? or should I merge and create a PR to cover it?

[kensipe]

while this looks similar to the quick exit above... it isn't the same IMO. I would rather the switch below be the ways in which the h.config is configured... more importantly we should h.T.Log this.

[alifelan]

Sounds great! I updated it, now the check is in the switch, and also added a test. I'm looking for feedback on that, since I tried creating a test environment using testutils.StartTestEnvironment, and from there passing the config to harness. The problem with that is that the mocked controlplane is skipped on the service account check, otherwise raises an error. To achieve that, I set StartControlPlane to true and changed the order of the switch. I don't feel that's the perfect solution, since setting StartControlPlane in there is a bit of a lie, but I didn't want to add a config just for this specific use case. If you have any other solution, please let me know!

[kensipe]

/lgtm

Thanks so much!

[kensipe]

the CI doesn't seem to run for non-committers... I don't remember the details around this... all standard CI activities tested good. Ready to merge. Adding a todo to enable CI for all PRs

[kensipe]

For awareness.. this broken main on merge... I should have caught it on review... but I blame the lack of CI builds for non-committers as the issue. The make generate is called along with verify-generate.sh in the CI. It rarely changes and is rarely called by developers. We need the safety net of the CI. I will submit a PR fix for the break.

[kensipe]

:( well @alifelan I need to revert this PR. It would be get to support this if it helps... but the generated deepcopy code fails. We need a solution to that in order to move forward.
the linter fails with

pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go:232:22: cannot use *out (variable of type *rest.Config) as *rest.TLSClientConfig value in argument to (*in).DeepCopyInto (typecheck)

and the builds fail

go build -ldflags "-X github.com/kudobuilder/kuttl/pkg/version.gitVersion= -X github.com/kudobuilder/kuttl/pkg/version.gitCommit= -X github.com/kudobuilder/kuttl/pkg/version.buildDate=2022-04-14T13:12:46Z" -o bin/kubectl-kuttl ./cmd/kubectl-kuttl
# github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1
pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go:232:22: cannot use *out (variable of type *rest.Config) as type *rest.TLSClientConfig in argument to (*in).DeepCopyInto

[kensipe]

@alifelan any thoughts on re-attempting this PR? I hated reverting it.. but need main to be buildable.

[alifelan]

Hey @kensipe! I've been taking a look, though I don't have a solution yet. Seems like TLSClientConfig is a promoted field in rest.Config, and it's DeepCopy method is the one being used (rest.Config has a CopyConfig method, but that won't be used by the deepcopy-gen). The only idea I have is moving the field from TestSuite to Harness, but I don't think that's the way to go from a design point of view. I'll think about it, and share my ideas with you!