Enable receiving a rest config on TestSuite

[alifelan]

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

What this PR does / why we need it:
PR #348 was missing the generated deep copy code. When added, that code had a problem since rest.Config does not implement DeepCopy, but has the method available from the promoted rest.TLSClientConfig, which when being used on rest.Config ended up breaking the code.
There were several solutions considered, each having its downsides:

• Exporting the config in Harness. Downsides were exposing a field for one specific use case, and also having to rename the Config method
• Adding an exported field on Harness, which would be copied to Harness.config. Same as before, having a field for one specific use case wasn't the best
• Implementing the DeepCopy method with the custom copy for rest.Config. In this scenario, the DeepCopy method would no longer be autogenerated.

The implemented solution was having a struct that embeds the rest.Config. While doing that, we can implement our custom DeepCopyInto method, using rest.CopyConfig. After this, the generated code will be compatible. To avoid repeated names while getting the config from TestSuite, the field was named RC.

To avoid running into the same problems as the previous PR, I ran generate, cli, and all targets from the Makefile.

Fixes #347

[kensipe]

@alifelan I'm curious why you provided the deepcopy func... if removed... the make generate process creates the proper functionality in /pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go which looks like

/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go
@@ -21,6 +21,7 @@ package v1beta1

 import (
        runtime "k8s.io/apimachinery/pkg/runtime"
+       rest "k8s.io/client-go/rest"
 )

 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
@@ -63,6 +64,17 @@ func (in *ObjectReference) DeepCopy() *ObjectReference {
        return out
 }

+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *RestConfig) DeepCopyInto(out *RestConfig) {
+       *out = *in
+       if in.RC != nil {
+               in, out := &in.RC, &out.RC
+               *out = new(rest.Config)
+               (*in).DeepCopyInto(*out)
+       }
+       return
+}
+
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RestConfig.

which keeps all the copy code in the same place. I've not taken over the deepcopy functionality before... and thus don't know the pros/cons (yet). However it would be great to keep in the generated place which would allow for refactoring that is tied to the same mechanism (generation). Thoughts?

[kensipe]

I'm otherwise good! Allowing for a response / update if appropriate. Looking to merge after we agree on approach. thanks for the contribution!

[alifelan]

Sure, the problem with the generated code is that rest.Config exposes a DeepCopy method, but it's not its own. It belongs to rest.TLSClientConfig. If we let that method be used, we end up running against the same problem as before. I just tried running it like that to confirm, and we get the same error as before when running make cli.

go build -ldflags "-X github.com/kudobuilder/kuttl/pkg/version.gitVersion=0.11.1 -X github.com/kudobuilder/kuttl/pkg/version.gitCommit=ceee13e9 -X github.com/kudobuilder/kuttl/pkg/version.buildDate=2022-04-19T22:30:59Z" -o bin/kubectl-kuttl ./cmd/kubectl-kuttl
# github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1
pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go:73:22: cannot use *out (variable of type *rest.Config) as type *rest.TLSClientConfig in argument to (*in).DeepCopyInto
make: *** [Makefile:51: cli] Error 1

DeepCopy will create only the methods that are missing, enabling us to define our custom ones. That is useful for the rest.Config, in which we can use its CopyConfig method, instead of the DeepCopyInto.

I think the same on keeping the code in the same place, but we cannot keep it in the generated file since it would be overwritten whenever we generate new code. I was thinking about creating /pkg/apis/testharness/v1beta1/deepcopy.go to keep it separated from the types definition, but I don't know if there's any other standard

Let me know your thoughts on this!

[kensipe]

/lgtm

[kensipe]

thanks for the explanation... all makes sense to me. Thanks for sticking with it!