Add more toolkit methods

The main addition is `.Kubectl`, which seems to be slower than using
the client directly, but can be more ergonomic for development. This
requires a kubeconfig, but one can be generated from the REST config,
only requiring small changes to the main "constructor" function for a
toolkit.

Signed-off-by: Justin Kulikauskas <[email protected]>

pkg/testutils/courtesies.go

@@ -6,6 +6,7 @@ import (
 	"regexp"
 	"time"
 
+	"github.com/onsi/ginkgo/v2"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -49,3 +50,19 @@ func EventFilter(events []corev1.Event, evType, msg string, since time.Time) []c
 
 	return ans
 }
+
+// RegisterDebugMessage returns a pointer to a string which will be logged at the
+// end of the test only if the test fails. This is particularly useful for logging
+// information only once in an Eventually or Consistently function.
+// Note: using a custom description message may be a better practice overall.
+func RegisterDebugMessage() *string {
+	var debugMsg string
+
+	ginkgo.DeferCleanup(func() {
+		if ginkgo.CurrentSpecReport().Failed() {
+			ginkgo.GinkgoWriter.Println(debugMsg)
+		}
+	})
+
+	return &debugMsg
+}

pkg/testutils/toolkit.go

@@ -4,41 +4,113 @@ package testutils
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os"
+	"os/exec"
 	"regexp"
 	"sort"
+	"strings"
 
 	"github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
 	gomegaTypes "github.com/onsi/gomega/types"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/clientcmd"
+	"k8s.io/client-go/tools/clientcmd/api"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+var ErrKubectl = errors.New("")
+
 type Toolkit struct {
 	client.Client
+	Ctx                 context.Context //nolint:containedctx // this is for convenience
+	RestConfig          *rest.Config
+	KubeconfigPath      string
 	EventuallyPoll      string
 	EventuallyTimeout   string
 	ConsistentlyPoll    string
 	ConsistentlyTimeout string
-	BackgroundCtx       context.Context //nolint:containedctx // this is for convenience
 }
 
-// NewToolkit returns a toolkit using the given Client, with some basic defaults.
-// This is the preferred way to get a Toolkit instance, to avoid unset fields.
+// NewToolkitFromRest returns a toolkit using the given REST config. This is
+// the preferred way to get a Toolkit instance, to avoid unset fields.
 //
-//nolint:gocritic // package client is shadowed, but any other name would be confusing
-func NewToolkit(client client.Client) Toolkit {
+// The toolkit will use a new client built from the REST config and the global
+// Scheme. The path to a kubeconfig can also be provided, which will be used
+// for `.Kubectl` calls - if passed an empty string, a temporary kubeconfig
+// will be created based on the REST config.
+func NewToolkitFromRest(tkCfg *rest.Config, kubeconfigPath string) (Toolkit, error) {
+	k8sClient, err := client.New(tkCfg, client.Options{Scheme: scheme.Scheme})
+	if err != nil {
+		return Toolkit{}, err
+	}
+
+	// Create a temporary kubeconfig if one is not provided.
+	if kubeconfigPath == "" {
+		f, err := os.CreateTemp("", "toolkit-kubeconfig-*")
+		if err != nil {
+			return Toolkit{}, err
+		}
+
+		contents, err := createKubeconfigFile(tkCfg)
+		if err != nil {
+			return Toolkit{}, err
+		}
+
+		_, err = f.Write(contents)
+		if err != nil {
+			return Toolkit{}, err
+		}
+
+		kubeconfigPath = f.Name()
+	}
+
 	return Toolkit{
-		Client:              client,
+		Client:              k8sClient,
+		Ctx:                 context.Background(),
+		RestConfig:          tkCfg,
+		KubeconfigPath:      kubeconfigPath,
 		EventuallyPoll:      "100ms",
 		EventuallyTimeout:   "1s",
 		ConsistentlyPoll:    "100ms",
 		ConsistentlyTimeout: "1s",
-		BackgroundCtx:       context.Background(),
-	}
+	}, nil
+}
+
+func (tk Toolkit) WithEPoll(eventuallyPoll string) Toolkit {
+	tk.EventuallyPoll = eventuallyPoll
+
+	return tk
+}
+
+func (tk Toolkit) WithETimeout(eventuallyTimeout string) Toolkit {
+	tk.EventuallyTimeout = eventuallyTimeout
+
+	return tk
+}
+
+func (tk Toolkit) WithCPoll(consistentlyPoll string) Toolkit {
+	tk.ConsistentlyPoll = consistentlyPoll
+
+	return tk
+}
+
+func (tk Toolkit) WithCTimeout(consistentlyTimeout string) Toolkit {
+	tk.ConsistentlyTimeout = consistentlyTimeout
+
+	return tk
+}
+
+func (tk Toolkit) WithCtx(ctx context.Context) Toolkit {
+	tk.Ctx = ctx
+
+	return tk
 }
 
 // CleanlyCreate creates the given object, and registers a callback to delete the object which
@@ -52,8 +124,8 @@ func (tk Toolkit) CleanlyCreate(ctx context.Context, obj client.Object, opts ...
 			ginkgo.GinkgoWriter.Printf("Deleting %v %v/%v\n",
 				obj.GetObjectKind().GroupVersionKind().Kind, obj.GetNamespace(), obj.GetName())
 
-			if err := tk.Delete(tk.BackgroundCtx, obj); err != nil {
-				if !errors.IsNotFound(err) {
+			if err := tk.Delete(tk.Ctx, obj); err != nil {
+				if !k8sErrors.IsNotFound(err) {
 					// Use Fail in order to provide a custom message with useful information
 					ginkgo.Fail(fmt.Sprintf("Expected success or 'NotFound' error, got %v", err), 1)
 				}
@@ -188,18 +260,56 @@ func (tk Toolkit) EC(
 	).Should(matcher, cDesc...)
 }
 
-// RegisterDebugMessage returns a pointer to a string which will be logged at the
-// end of the test only if the test fails. This is particularly useful for logging
-// information only once in an Eventually or Consistently function.
-// Note: using a custom description message may be a better practice overall.
-func RegisterDebugMessage() *string {
-	var debugMsg string
+func (tk *Toolkit) Kubectl(args ...string) (string, error) {
+	addKubeconfig := true
+
+	for _, arg := range args {
+		if strings.HasPrefix(arg, "--kubeconfig") {
+			addKubeconfig = false
 
-	ginkgo.DeferCleanup(func() {
-		if ginkgo.CurrentSpecReport().Failed() {
-			ginkgo.GinkgoWriter.Println(debugMsg)
+			break
 		}
-	})
+	}
+
+	if addKubeconfig {
+		args = append([]string{"--kubeconfig=" + tk.KubeconfigPath}, args...)
+	}
+
+	output, err := exec.Command("kubectl", args...).Output()
+
+	var exitError *exec.ExitError
+
+	if errors.As(err, &exitError) {
+		if exitError.Stderr == nil {
+			return string(output), err
+		}
+
+		return string(output), fmt.Errorf("%w%s", ErrKubectl, exitError.Stderr)
+	}
+
+	return string(output), err
+}
+
+func createKubeconfigFile(cfg *rest.Config) ([]byte, error) {
+	identifier := "toolkit"
+
+	kubeconfig := api.NewConfig()
+
+	cluster := api.NewCluster()
+	cluster.Server = cfg.Host
+	cluster.CertificateAuthorityData = cfg.CAData
+	kubeconfig.Clusters[identifier] = cluster
+
+	authInfo := api.NewAuthInfo()
+	authInfo.ClientCertificateData = cfg.CertData
+	authInfo.ClientKeyData = cfg.KeyData
+	kubeconfig.AuthInfos[identifier] = authInfo
+
+	apiContext := api.NewContext()
+	apiContext.Cluster = identifier
+	apiContext.AuthInfo = identifier
+	kubeconfig.Contexts[identifier] = apiContext
+	kubeconfig.CurrentContext = identifier
 
-	return &debugMsg
+	return clientcmd.Write(*kubeconfig)
 }

test/fakepolicy/test/basic/namespaceselection_test.go

@@ -36,7 +36,7 @@ var _ = Describe("FakePolicy NamespaceSelection", Ordered, func() {
 		// constructing the default / allNamespaces lists is complicated because of how ginkgo
 		// runs the table tests... this seems better than other workarounds.
 		nsList := corev1.NamespaceList{}
-		Expect(k8sClient.List(ctx, &nsList)).To(Succeed())
+		Expect(tk.List(ctx, &nsList)).To(Succeed())
 
 		foundNS := make([]string, len(nsList.Items))
 		for i, ns := range nsList.Items {
@@ -57,7 +57,7 @@ var _ = Describe("FakePolicy NamespaceSelection", Ordered, func() {
 
 			Eventually(func(g Gomega) {
 				foundPolicy := fakev1beta1.FakePolicy{}
-				g.Expect(k8sClient.Get(ctx, testutils.ObjNN(&policy), &foundPolicy)).To(Succeed())
+				g.Expect(tk.Get(ctx, testutils.ObjNN(&policy), &foundPolicy)).To(Succeed())
 				g.Expect(foundPolicy.Status.SelectionComplete).To(BeTrue())
 
 				idx, cond := foundPolicy.Status.GetCondition("NamespaceSelection")

test/fakepolicy/test/basic/suite_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/onsi/gomega/format"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -26,12 +25,11 @@ import (
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
 var (
-	cfg       *rest.Config
-	k8sClient client.Client
-	testEnv   *envtest.Environment
-	ctx       context.Context
-	cancel    context.CancelFunc
-	tk        testutils.Toolkit
+	cfg     *rest.Config
+	testEnv *envtest.Environment
+	ctx     context.Context
+	cancel  context.CancelFunc
+	tk      testutils.Toolkit
 )
 
 //nolint:paralleltest // scaffolded this way by ginkgo
@@ -67,12 +65,9 @@ var _ = BeforeSuite(func() {
 
 	//+kubebuilder:scaffold:scheme
 
-	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
+	tk, err = testutils.NewToolkitFromRest(cfg, "")
 	Expect(err).NotTo(HaveOccurred())
-	Expect(k8sClient).NotTo(BeNil())
-
-	tk = testutils.NewToolkit(k8sClient)
-	tk.BackgroundCtx = ctx
+	tk = tk.WithCtx(ctx)
 
 	go func() {
 		defer GinkgoRecover()

test/fakepolicy/test/basic/target_test.go

@@ -53,7 +53,7 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
 		// constructing the default / allConfigMaps lists is complicated because of how ginkgo
 		// runs the table tests... this seems better than other workarounds.
 		cmList := corev1.ConfigMapList{}
-		Expect(k8sClient.List(ctx, &cmList)).To(Succeed())
+		Expect(tk.List(ctx, &cmList)).To(Succeed())
 
 		foundCM := make([]string, len(cmList.Items))
 		for i, cm := range cmList.Items {
@@ -138,7 +138,7 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
 	checkFunc := func(policy fakev1beta1.FakePolicy, desiredMatches []string, selErr string) func(g Gomega) {
 		return func(g Gomega) {
 			foundPolicy := fakev1beta1.FakePolicy{}
-			g.Expect(k8sClient.Get(ctx, testutils.ObjNN(&policy), &foundPolicy)).To(Succeed())
+			g.Expect(tk.Get(ctx, testutils.ObjNN(&policy), &foundPolicy)).To(Succeed())
 			g.Expect(foundPolicy.Status.SelectionComplete).To(BeTrue())
 
 			slices.Sort(desiredMatches)

test/fakepolicy/test/basic/toolkit_test.go

@@ -0,0 +1,35 @@
+// Copyright Contributors to the Open Cluster Management project
+
+package basic
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Additional Toolkit tests", func() {
+	It("Config methods should override values, but preserve the original toolkit", func() {
+		newTK := tk.
+			WithEPoll("50ms").
+			WithETimeout("3s").
+			WithCPoll("500ms").
+			WithCTimeout("2s")
+
+		Expect(newTK.EventuallyPoll).To(Equal("50ms"))
+		Expect(newTK.EventuallyTimeout).To(Equal("3s"))
+		Expect(newTK.ConsistentlyPoll).To(Equal("500ms"))
+		Expect(newTK.ConsistentlyTimeout).To(Equal("2s"))
+
+		Expect(tk.EventuallyPoll).To(Equal("100ms"))
+		Expect(tk.EventuallyTimeout).To(Equal("1s"))
+		Expect(tk.ConsistentlyPoll).To(Equal("100ms"))
+		Expect(tk.ConsistentlyTimeout).To(Equal("1s"))
+	})
+
+	It("Kubectl should return error outputs", func() {
+		output, err := tk.Kubectl("get", "node", "nonexist")
+		Expect(output).To(BeEmpty())
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("not found"))
+	})
+})

test/fakepolicy/test/basic/yamlformat_test.go

@@ -8,11 +8,9 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	nucleusv1beta1 "open-cluster-management.io/governance-policy-nucleus/api/v1beta1"
-	"open-cluster-management.io/governance-policy-nucleus/pkg/testutils"
 	. "open-cluster-management.io/governance-policy-nucleus/test/fakepolicy/test/utils"
 )
 
@@ -52,34 +50,24 @@ var _ = Describe("FakePolicy resource format verification", func() {
 		Operator: metav1.LabelSelectorOpExists,
 	}}
 
-	// input is a clientObject so that either an Unstructured or the "real" type can be provided.
+	// input is a client.Object so that either an Unstructured or the "real" type can be provided.
 	DescribeTable("Verifying spec stability", func(ctx SpecContext, input client.Object, wantFile string) {
 		Expect(tk.CleanlyCreate(ctx, input)).To(Succeed())
 
-		nn := testutils.ObjNN(input)
-		gotObj := &unstructured.Unstructured{
-			Object: map[string]interface{}{
-				"apiVersion": "policy.open-cluster-management.io/v1beta1",
-				"kind":       "FakePolicy",
-			},
+		kind := "fakepolicy"
+		if input.GetName() == "policycore-sample" {
+			kind = "policycore"
 		}
 
-		if nn.Name == "policycore-sample" {
-			gotObj.Object["kind"] = "PolicyCore"
-		}
-
-		Expect(k8sClient.Get(ctx, nn, gotObj)).Should(Succeed())
-
-		// Just compare specs; metadata will be different between runs
-
-		gotSpec, err := json.Marshal(gotObj.Object["spec"])
+		gotSpec, err := tk.Kubectl("get", kind, "-n="+input.GetNamespace(), input.GetName(),
+			"-o=jsonpath={.spec}")
 		Expect(err).ToNot(HaveOccurred())
 
 		wantUnstruct := FromTestdata(wantFile)
 		wantSpec, err := json.Marshal(wantUnstruct.Object["spec"])
 		Expect(err).ToNot(HaveOccurred())
 
-		Expect(string(wantSpec)).To(Equal(string(gotSpec)))
+		Expect(string(wantSpec)).To(Equal(gotSpec))
 	},
 		// These instances should be "stable" - getting them from the cluster after applying them
 		// should return the same information (modulo some metadata, of course)