CNTRLPLANE-550: Refactor CLI client creation to use dependency injection for testability

cmd/cluster/aws/create.go

@@ -126,7 +126,7 @@ func (o *ValidatedCreateOptions) Complete(ctx context.Context, opts *core.Create
 		opts.EtcdStorageClass = "gp3-csi"
 	}
 
-	client, err := util.GetClient()
+	client, err := opts.GetClient()
 	if err != nil {
 		return nil, err
 	}

cmd/cluster/aws/create_test.go

@@ -13,12 +13,16 @@ import (
 	awsinfra "github.com/openshift/hypershift/cmd/infra/aws"
 	awsutil "github.com/openshift/hypershift/cmd/infra/aws/util"
 	"github.com/openshift/hypershift/cmd/util"
+	hyperapi "github.com/openshift/hypershift/support/api"
 	"github.com/openshift/hypershift/support/certs"
 	"github.com/openshift/hypershift/support/testutil"
 	"github.com/openshift/hypershift/test/integration/framework"
 
 	utilrand "k8s.io/apimachinery/pkg/util/rand"
 
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
 	"github.com/spf13/pflag"
 )
 
@@ -90,7 +94,6 @@ func TestCreateCluster(t *testing.T) {
 	certs.UnsafeSeed(1234567890)
 	ctx := framework.InterruptableContext(t.Context())
 	tempDir := t.TempDir()
-	t.Setenv("FAKE_CLIENT", "true")
 
 	rawCreds, err := json.Marshal(&awsutil.STSCreds{
 		Credentials: awsutil.Credentials{
@@ -235,6 +238,9 @@ func TestCreateCluster(t *testing.T) {
 			flags := pflag.NewFlagSet(testCase.name, pflag.ContinueOnError)
 			coreOpts := core.DefaultOptions()
 			core.BindDeveloperOptions(coreOpts, flags)
+			coreOpts.ClientFn = func() (crclient.Client, error) {
+				return fake.NewClientBuilder().WithScheme(hyperapi.Scheme).Build(), nil
+			}
 			awsOpts := DefaultOptions()
 			BindDeveloperOptions(awsOpts, flags)
 			if err := flags.Parse(testCase.args); err != nil {

cmd/cluster/azure/create_test.go

@@ -14,12 +14,15 @@ import (
 	azureinfra "github.com/openshift/hypershift/cmd/infra/azure"
 	azurenodepool "github.com/openshift/hypershift/cmd/nodepool/azure"
 	"github.com/openshift/hypershift/cmd/util"
+	hyperapi "github.com/openshift/hypershift/support/api"
 	"github.com/openshift/hypershift/support/certs"
 	"github.com/openshift/hypershift/support/testutil"
 	"github.com/openshift/hypershift/test/integration/framework"
 
 	utilrand "k8s.io/apimachinery/pkg/util/rand"
 
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/yaml"
 
 	"github.com/spf13/pflag"
@@ -87,8 +90,6 @@ func TestCreateCluster(t *testing.T) {
 	certs.UnsafeSeed(1234567890)
 	ctx := framework.InterruptableContext(t.Context())
 	tempDir := t.TempDir()
-	t.Setenv("FAKE_CLIENT", "true")
-
 	rawCreds, err := yaml.Marshal(&util.AzureCreds{
 		SubscriptionID: "fakeSubscriptionID",
 		ClientID:       "fakeClientID",
@@ -309,6 +310,9 @@ func TestCreateCluster(t *testing.T) {
 		t.Run(testCase.name, func(t *testing.T) {
 			flags := pflag.NewFlagSet(testCase.name, pflag.ContinueOnError)
 			coreOpts := core.DefaultOptions()
+			coreOpts.ClientFn = func() (crclient.Client, error) {
+				return fake.NewClientBuilder().WithScheme(hyperapi.Scheme).Build(), nil
+			}
 			core.BindDeveloperOptions(coreOpts, flags)
 			azureOpts := DefaultOptions()
 			azurenodepool.BindOptions(azureOpts.NodePoolOpts, flags)

cmd/cluster/core/create.go

@@ -188,11 +188,24 @@ type RawCreateOptions struct {
 	// This is intended primarily for e2e testing and should be used with care.
 	BeforeApply func(crclient.Object) `json:"-"`
 
+	// ClientFn returns a Kubernetes client. When nil, util.GetClient is used.
+	// This enables dependency injection for testing without requiring a live cluster.
+	ClientFn func() (crclient.Client, error) `json:"-"`
+
 	// These fields are reverse-completed by the aws CLI since we support a flag that projects
 	// them back up here
 	PublicKey, PrivateKey, PullSecret []byte
 }
 
+// GetClient returns a Kubernetes client using the configured ClientFn,
+// falling back to util.GetClient when ClientFn is nil.
+func (o *RawCreateOptions) GetClient() (crclient.Client, error) {
+	if o.ClientFn != nil {
+		return o.ClientFn()
+	}
+	return util.GetClient()
+}
+
 type resources struct {
 	AdditionalTrustBundle *corev1.ConfigMap
 	Namespace             *corev1.Namespace
@@ -358,7 +371,7 @@ func resolveReleaseImage(ctx context.Context, opts *CreateOptions) error {
 	if len(opts.ReleaseImage) != 0 || len(opts.ReleaseStream) == 0 {
 		return nil
 	}
-	client, err := util.GetClient()
+	client, err := opts.GetClient()
 	if err != nil {
 		return fmt.Errorf("failed to get client: %w", err)
 	}
@@ -612,8 +625,8 @@ func applyFeatureSet(cluster *hyperv1.HostedCluster, opts *CreateOptions) {
 	}
 }
 
-func apply(ctx context.Context, l logr.Logger, infraID string, objects []crclient.Object, waitForRollout bool, mutate func(crclient.Object)) error {
-	client, err := util.GetClient()
+func apply(ctx context.Context, l logr.Logger, infraID string, objects []crclient.Object, waitForRollout bool, mutate func(crclient.Object), clientFn func() (crclient.Client, error)) error {
+	client, err := clientFn()
 	if err != nil {
 		return err
 	}
@@ -753,7 +766,7 @@ func (opts *RawCreateOptions) Validate(ctx context.Context) (*ValidatedCreateOpt
 func (opts *RawCreateOptions) validateVersionAndWait(ctx context.Context) error {
 	if opts.VersionCheck {
 		versionCLI := supportedversion.GetRevision()
-		client, err := util.GetClient()
+		client, err := opts.GetClient()
 		if err != nil {
 			return fmt.Errorf("failed to get client: %w", err)
 		}
@@ -771,7 +784,7 @@ func (opts *RawCreateOptions) validateClusterExistence(ctx context.Context) erro
 	if opts.Render {
 		return nil
 	}
-	client, err := util.GetClient()
+	client, err := opts.GetClient()
 	if err != nil {
 		return err
 	}
@@ -1005,7 +1018,7 @@ func CreateCluster(ctx context.Context, rawOpts *RawCreateOptions, rawPlatform P
 	}
 
 	// Otherwise, apply the objects
-	return apply(ctx, opts.Log, resources.Cluster.Spec.InfraID, resources.asObjects(), opts.Wait, opts.BeforeApply)
+	return apply(ctx, opts.Log, resources.Cluster.Spec.InfraID, resources.asObjects(), opts.Wait, opts.BeforeApply, opts.GetClient)
 }
 
 type DefaultNodePoolConstructor func(platformType hyperv1.PlatformType, suffix string) *hyperv1.NodePool

cmd/cluster/core/create_test.go

@@ -10,6 +10,7 @@ import (
 	. "github.com/onsi/gomega"
 
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
+	hyperapi "github.com/openshift/hypershift/support/api"
 	"github.com/openshift/hypershift/support/config"
 	"github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client"
 	"github.com/openshift/hypershift/support/util/fakeimagemetadataprovider"
@@ -708,6 +709,111 @@ func TestAllocateNodeCIDRsFlag(t *testing.T) {
 	}
 }
 
+func TestCreateOptionsGetClient(t *testing.T) {
+	t.Run("When ClientFn is set it should use the provided function", func(t *testing.T) {
+		g := NewWithT(t)
+		expectedClient := fake.NewClientBuilder().WithScheme(hyperapi.Scheme).Build()
+		opts := &RawCreateOptions{
+			ClientFn: func() (crclient.Client, error) {
+				return expectedClient, nil
+			},
+		}
+		c, err := opts.GetClient()
+		g.Expect(err).ToNot(HaveOccurred())
+		g.Expect(c).To(Equal(expectedClient))
+	})
+
+	t.Run("When ClientFn is set it should be accessible via completed options", func(t *testing.T) {
+		g := NewWithT(t)
+		expectedClient := fake.NewClientBuilder().WithScheme(hyperapi.Scheme).Build()
+		opts := &CreateOptions{
+			completedCreateOptions: &completedCreateOptions{
+				ValidatedCreateOptions: &ValidatedCreateOptions{
+					validatedCreateOptions: &validatedCreateOptions{
+						RawCreateOptions: &RawCreateOptions{
+							ClientFn: func() (crclient.Client, error) {
+								return expectedClient, nil
+							},
+						},
+					},
+				},
+			},
+		}
+		c, err := opts.GetClient()
+		g.Expect(err).ToNot(HaveOccurred())
+		g.Expect(c).To(Equal(expectedClient))
+	})
+}
+
+func TestValidateWithInjectedClient(t *testing.T) {
+	t.Run("When a HostedCluster already exists it should return an error", func(t *testing.T) {
+		g := NewWithT(t)
+		ctx := t.Context()
+		tempDir := t.TempDir()
+
+		pullSecretFile := filepath.Join(tempDir, "pull-secret.json")
+		if err := os.WriteFile(pullSecretFile, []byte(`fake`), 0600); err != nil {
+			t.Fatalf("failed to write pullSecret: %v", err)
+		}
+
+		existingCluster := &hyperv1.HostedCluster{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "existing-cluster",
+				Namespace: "clusters",
+			},
+		}
+		fakeClient := fake.NewClientBuilder().
+			WithScheme(hyperapi.Scheme).
+			WithObjects(existingCluster).
+			Build()
+
+		rawOpts := &RawCreateOptions{
+			Name:           "existing-cluster",
+			Namespace:      "clusters",
+			PullSecretFile: pullSecretFile,
+			Arch:           "amd64",
+			Render:         false,
+			ClientFn: func() (crclient.Client, error) {
+				return fakeClient, nil
+			},
+		}
+
+		_, err := rawOpts.Validate(ctx)
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(err.Error()).To(ContainSubstring("already exists"))
+	})
+
+	t.Run("When no HostedCluster exists and render is true it should pass validation", func(t *testing.T) {
+		g := NewWithT(t)
+		ctx := t.Context()
+		tempDir := t.TempDir()
+
+		pullSecretFile := filepath.Join(tempDir, "pull-secret.json")
+		if err := os.WriteFile(pullSecretFile, []byte(`fake`), 0600); err != nil {
+			t.Fatalf("failed to write pullSecret: %v", err)
+		}
+
+		fakeClient := fake.NewClientBuilder().
+			WithScheme(hyperapi.Scheme).
+			Build()
+
+		rawOpts := &RawCreateOptions{
+			Name:           "new-cluster",
+			Namespace:      "clusters",
+			PullSecretFile: pullSecretFile,
+			Arch:           "amd64",
+			Render:         true,
+			ClientFn: func() (crclient.Client, error) {
+				return fakeClient, nil
+			},
+		}
+
+		validated, err := rawOpts.Validate(ctx)
+		g.Expect(err).ToNot(HaveOccurred())
+		g.Expect(validated).ToNot(BeNil())
+	})
+}
+
 func TestValidateVersion(t *testing.T) {
 	tests := []struct {
 		name             string

cmd/cluster/core/destroy.go

@@ -44,6 +44,19 @@ type DestroyOptions struct {
 	Log                   logr.Logger
 	CredentialSecretName  string
 	RedactBaseDomain      bool
+
+	// ClientFn returns a Kubernetes client. When nil, util.GetClient is used.
+	// This enables dependency injection for testing without requiring a live cluster.
+	ClientFn func() (client.Client, error) `json:"-"`
+}
+
+// GetClient returns a Kubernetes client using the configured ClientFn,
+// falling back to util.GetClient when ClientFn is nil.
+func (o *DestroyOptions) GetClient() (client.Client, error) {
+	if o.ClientFn != nil {
+		return o.ClientFn()
+	}
+	return util.GetClient()
 }
 
 type AWSPlatformDestroyOptions struct {
@@ -85,7 +98,7 @@ type PowerVSPlatformDestroyOptions struct {
 }
 
 func GetCluster(ctx context.Context, o *DestroyOptions) (*hyperv1.HostedCluster, error) {
-	c, err := util.GetClient()
+	c, err := o.GetClient()
 	if err != nil {
 		return nil, err
 	}
@@ -106,7 +119,7 @@ func GetCluster(ctx context.Context, o *DestroyOptions) (*hyperv1.HostedCluster,
 func DestroyCluster(ctx context.Context, hostedCluster *hyperv1.HostedCluster, o *DestroyOptions, destroyPlatformSpecifics DestroyPlatformSpecifics) error {
 	hostedClusterExists := hostedCluster != nil
 	shouldDestroyPlatformSpecifics := destroyPlatformSpecifics != nil
-	c, err := util.GetClient()
+	c, err := o.GetClient()
 	if err != nil {
 		return err
 	}