From 47e0debb21bd77e2d68b8224dd7a0fdf67e31bf8 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Fri, 17 Jul 2026 22:57:33 +0200 Subject: [PATCH] refactor(provisioner): remove reflection from multi-provisioner tests The multi-provisioner tests reached into the Kind provisioner's unexported kubeConfig/kindConfig fields via reflection, coupling them to internal field names and layout. Introduce a package-level, injectable Kind factory seam (swapped in tests via SetKindProvisionerFactory) so the tests assert the kindConfig and kubeconfig arguments the minimal path passes to the factory directly, with no reflection. Production always uses kindprovisioner.CreateProvisioner, so behaviour is unchanged. Fixes #6049 Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/svc/provisioner/cluster/export_test.go | 21 ++++++- pkg/svc/provisioner/cluster/multi.go | 11 +++- pkg/svc/provisioner/cluster/multi_test.go | 68 +++++++++++++--------- 3 files changed, 72 insertions(+), 28 deletions(-) diff --git a/pkg/svc/provisioner/cluster/export_test.go b/pkg/svc/provisioner/cluster/export_test.go index a62280d59a..eba6b813d0 100644 --- a/pkg/svc/provisioner/cluster/export_test.go +++ b/pkg/svc/provisioner/cluster/export_test.go @@ -1,6 +1,25 @@ package clusterprovisioner -import k3dv1alpha5 "github.com/k3d-io/k3d/v5/pkg/config/v1alpha5" +import ( + kindprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster/kind" + k3dv1alpha5 "github.com/k3d-io/k3d/v5/pkg/config/v1alpha5" + "sigs.k8s.io/kind/pkg/apis/config/v1alpha4" +) + +// KindProvisionerFactory is the signature of the injectable Kind provisioner +// factory used by the minimal multi-provisioner path. +type KindProvisionerFactory = func(*v1alpha4.Cluster, string) (*kindprovisioner.Provisioner, error) + +// SetKindProvisionerFactory swaps the Kind provisioner factory for a test double +// and returns a restore func. It lets external tests assert the kindConfig and +// kubeconfig arguments passed to the factory without reflecting over the +// provisioner's unexported fields. +func SetKindProvisionerFactory(factory KindProvisionerFactory) func() { + previous := kindProvisionerFactory + kindProvisionerFactory = factory + + return func() { kindProvisionerFactory = previous } +} // ExportWrapK3kServerArgs exposes wrapK3kServerArgs for testing. func ExportWrapK3kServerArgs(args []string) []string { diff --git a/pkg/svc/provisioner/cluster/multi.go b/pkg/svc/provisioner/cluster/multi.go index 35bceab504..04f036c09c 100644 --- a/pkg/svc/provisioner/cluster/multi.go +++ b/pkg/svc/provisioner/cluster/multi.go @@ -204,6 +204,15 @@ func CreateMinimalProvisioner( } } +// kindProvisionerFactory constructs the Kind provisioner for the minimal +// multi-provisioner path. It is an indirected package var (rather than a direct +// call) so tests can substitute a spy and assert the kindConfig and kubeconfig +// arguments it receives, without reflecting over the provisioner's unexported +// fields. Production always uses kindprovisioner.CreateProvisioner. +// +//nolint:gochecknoglobals // injected for testability; see SetKindProvisionerFactory in export_test.go +var kindProvisionerFactory = kindprovisioner.CreateProvisioner + // createMinimalKindProvisioner builds the smallest valid Kind provisioner // while preserving the caller-owned kubeconfig path. func createMinimalKindProvisioner(clusterName, kubeconfigPath string) (Provisioner, error) { @@ -215,7 +224,7 @@ func createMinimalKindProvisioner(clusterName, kubeconfigPath string) (Provision Name: clusterName, } - provisioner, err := kindprovisioner.CreateProvisioner(kindConfig, kubeconfigPath) + provisioner, err := kindProvisionerFactory(kindConfig, kubeconfigPath) if err != nil { return nil, fmt.Errorf("failed to create Kind provisioner: %w", err) } diff --git a/pkg/svc/provisioner/cluster/multi_test.go b/pkg/svc/provisioner/cluster/multi_test.go index 64d1b249a8..a47454890b 100644 --- a/pkg/svc/provisioner/cluster/multi_test.go +++ b/pkg/svc/provisioner/cluster/multi_test.go @@ -2,7 +2,6 @@ package clusterprovisioner_test import ( "context" - "reflect" "testing" "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" @@ -13,6 +12,7 @@ import ( kwokprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster/kwok" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "sigs.k8s.io/kind/pkg/apis/config/v1alpha4" ) func TestNewMultiProvisioner(t *testing.T) { @@ -65,11 +65,25 @@ func TestCreateMinimalProvisioner_K3s_Succeeds(t *testing.T) { } // TestCreateMinimalProvisioner_VanillaForwardsKubeconfigPath verifies richer -// callers can isolate Kind from the shared kubeconfig. +// callers can isolate Kind from the shared kubeconfig by asserting the +// kubeconfig path the minimal path hands to the Kind factory. +// +// It is not parallel: it swaps the package-level Kind provisioner factory. +// +//nolint:paralleltest // mutates the package-level Kind provisioner factory seam; must run serially. func TestCreateMinimalProvisioner_VanillaForwardsKubeconfigPath(t *testing.T) { - t.Parallel() + const kubeconfigPath = "/tmp/ephemeral-kind-kubeconfig" + + var gotKubeconfig string - kubeconfigPath := "/tmp/ephemeral-kind-kubeconfig" + restore := clusterprovisioner.SetKindProvisionerFactory( + func(_ *v1alpha4.Cluster, kubeconfig string) (*kindprovisioner.Provisioner, error) { + gotKubeconfig = kubeconfig + + return &kindprovisioner.Provisioner{}, nil + }, + ) + defer restore() provisioner, err := clusterprovisioner.CreateMinimalProvisioner( v1alpha1.DistributionVanilla, @@ -79,22 +93,32 @@ func TestCreateMinimalProvisioner_VanillaForwardsKubeconfigPath(t *testing.T) { ) require.NoError(t, err) - - kindProvisioner, ok := provisioner.(*kindprovisioner.Provisioner) - require.True(t, ok, "Vanilla must construct a Kind provisioner") - - value := reflect.ValueOf(kindProvisioner).Elem() - assert.Equal(t, kubeconfigPath, value.FieldByName("kubeConfig").String()) + require.NotNil(t, provisioner) + assert.Equal(t, kubeconfigPath, gotKubeconfig) } // TestCreateMinimalProvisioner_VanillaBuildsValidKindConfig verifies the -// minimal path still emits the TypeMeta required by Kind create. +// minimal path still emits the TypeMeta required by Kind create, by asserting +// the kindConfig it hands to the Kind factory. +// +// It is not parallel: it swaps the package-level Kind provisioner factory. +// +//nolint:paralleltest // mutates the package-level Kind provisioner factory seam; must run serially. func TestCreateMinimalProvisioner_VanillaBuildsValidKindConfig(t *testing.T) { - t.Parallel() - const clusterName = "test-kind" - provisioner, err := clusterprovisioner.CreateMinimalProvisioner( + var gotConfig *v1alpha4.Cluster + + restore := clusterprovisioner.SetKindProvisionerFactory( + func(config *v1alpha4.Cluster, _ string) (*kindprovisioner.Provisioner, error) { + gotConfig = config + + return &kindprovisioner.Provisioner{}, nil + }, + ) + defer restore() + + _, err := clusterprovisioner.CreateMinimalProvisioner( v1alpha1.DistributionVanilla, clusterName, "/tmp/ephemeral-kind-kubeconfig", @@ -102,18 +126,10 @@ func TestCreateMinimalProvisioner_VanillaBuildsValidKindConfig(t *testing.T) { ) require.NoError(t, err) - - kindProvisioner, ok := provisioner.(*kindprovisioner.Provisioner) - require.True(t, ok, "Vanilla must construct a Kind provisioner") - - value := reflect.ValueOf(kindProvisioner).Elem() - kindConfig := value.FieldByName("kindConfig") - require.False(t, kindConfig.IsNil()) - - kindConfig = kindConfig.Elem() - assert.Equal(t, "kind.x-k8s.io/v1alpha4", kindConfig.FieldByName("APIVersion").String()) - assert.Equal(t, "Cluster", kindConfig.FieldByName("Kind").String()) - assert.Equal(t, clusterName, kindConfig.FieldByName("Name").String()) + require.NotNil(t, gotConfig) + assert.Equal(t, "kind.x-k8s.io/v1alpha4", gotConfig.APIVersion) + assert.Equal(t, "Cluster", gotConfig.Kind) + assert.Equal(t, clusterName, gotConfig.Name) } func TestCreateMinimalProvisioner_UnsupportedDistribution(t *testing.T) {