fix(k8s): make KubectlOptions marshallable when RestConfig is set

rest.Config holds func-typed fields (WrapTransport, Dial, Proxy) and
encoding/json rejects a func field whether or not it is set, so any marshal
of options built by the public NewKubectlOptionsWithRestConfig constructor
failed with "unsupported type: transport.WrapperFunc". SaveKubectlOptions
turns that into a t.Fatalf, so the test dies.

Tag RestConfig json:"-" so options marshal, and have SaveKubectlOptions
reject a non-nil RestConfig outright rather than dropping it. Silently
dropping it would let LoadKubectlOptions return options that fall back to
the ambient kubeconfig and authenticate against a different cluster than
the one under test.

Serializing a reduced projection of rest.Config was considered and
rejected: it holds interfaces and exec credential plugin wiring that cannot
be rebuilt, and the JSON-safe fields are the credentials themselves, which
should not be written to .test-data.

Also drops the staticcheck half of a nolint in kubectl_options_test.go that
described this failure as pre-existing.

Author: james00012

modules/k8s/kubectl_options.go

@@ -35,13 +35,17 @@ type KubectlOptions struct {
 	// functions, and only after the node's own ExternalIP has been checked, so most callers can leave it nil.
 	// It is skipped when serializing options, since a function cannot be represented as JSON.
 	NodePublicIPLookup NodePublicIPLookup `json:"-"`
-	RestConfig         *rest.Config
-	Logger             *logger.Logger
-	ContextName        string
-	ConfigPath         string
-	Namespace          string
-	RequestTimeout     time.Duration
-	InClusterAuth      bool
+	// RestConfig is skipped when serializing options. rest.Config holds func-typed fields (WrapTransport, Dial,
+	// Proxy) and encoding/json rejects a func field whether or not it is set, so without this tag any marshal of
+	// KubectlOptions carrying a RestConfig fails. Note that SaveKubectlOptions refuses to save such options rather
+	// than dropping the config silently, since a reload would then authenticate against a different cluster.
+	RestConfig     *rest.Config `json:"-"`
+	Logger         *logger.Logger
+	ContextName    string
+	ConfigPath     string
+	Namespace      string
+	RequestTimeout time.Duration
+	InClusterAuth  bool
 }
 
 // NewKubectlOptions will return a pointer to new instance of KubectlOptions with the configured options

modules/k8s/kubectl_options_test.go

@@ -3,12 +3,14 @@ package k8s_test
 import (
 	"context"
 	"encoding/json"
+	"net/http"
 	"testing"
 
 	gotesting "github.com/gruntwork-io/terratest/modules/core/v2/testing"
 	"github.com/gruntwork-io/terratest/modules/k8s/v2"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"k8s.io/client-go/rest"
 )
 
 // TestKubectlOptionsMarshalsWithLookupSet guards the json:"-" tag on NodePublicIPLookup. KubectlOptions is
@@ -22,11 +24,7 @@ func TestKubectlOptionsMarshalsWithLookupSet(t *testing.T) {
 		return nil, nil
 	}
 
-	// nolint below: musttag because KubectlOptions has no json tags, and staticcheck SA1026 because it sees the
-	// func inside RestConfig. RestConfig is nil here, so the only func in play is NodePublicIPLookup, which is
-	// exactly what this test is pinning. Note that marshalling options with RestConfig set fails independently of
-	// this field; that is pre-existing behaviour on main and out of scope for this change.
-	raw, err := json.Marshal(options) //nolint:musttag,staticcheck // see comment above
+	raw, err := json.Marshal(options) //nolint:musttag // KubectlOptions does not have json tags
 	require.NoError(t, err, "options carrying a lookup func must still marshal")
 	assert.NotContains(t, string(raw), "NodePublicIPLookup", "the func field must be omitted from JSON")
 
@@ -37,3 +35,35 @@ func TestKubectlOptionsMarshalsWithLookupSet(t *testing.T) {
 	assert.Equal(t, "default", round.Namespace)
 	assert.Nil(t, round.NodePublicIPLookup, "func field is intentionally not persisted")
 }
+
+// TestKubectlOptionsMarshalsWithRestConfigSet covers the other unserializable field. rest.Config holds func-typed
+// fields, and encoding/json rejects a func field whether or not it is set, so before the json:"-" tag any marshal
+// of options built by NewKubectlOptionsWithRestConfig failed with "unsupported type: transport.WrapperFunc".
+func TestKubectlOptionsMarshalsWithRestConfigSet(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name   string
+		config *rest.Config
+	}{
+		{"plain config", &rest.Config{Host: "https://example.com"}},
+		{"config with WrapTransport set", &rest.Config{
+			Host: "https://example.com",
+			WrapTransport: func(rt http.RoundTripper) http.RoundTripper {
+				return rt
+			},
+		}},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+
+			options := k8s.NewKubectlOptionsWithRestConfig(testCase.config, "default")
+
+			raw, err := json.Marshal(options) //nolint:musttag // KubectlOptions does not have json tags
+			require.NoError(t, err, "options carrying a RestConfig must still marshal")
+			assert.NotContains(t, string(raw), "RestConfig", "the config must be omitted from JSON")
+		})
+	}
+}

modules/k8s/save_test_data.go

@@ -11,7 +11,24 @@ const kubectlOptionsFilename = "KubectlOptions.json"
 
 // SaveKubectlOptions serializes and saves KubectlOptions into the given folder. This allows you to create a
 // KubectlOptions during setup and reuse that KubectlOptions later during validation and teardown.
+//
+// Options carrying a RestConfig cannot be saved and will fail the test. RestConfig is not serializable: beyond its
+// func-typed fields it holds interfaces and exec credential plugin wiring that cannot be rebuilt from JSON, and the
+// fields that would survive are the credentials themselves, which have no business being written to .test-data.
+// Failing here is deliberate. Dropping the config silently would let LoadKubectlOptions return options that fall
+// back to the ambient kubeconfig and authenticate against a different cluster than the one under test.
 func SaveKubectlOptions(t testing.TestingT, testFolder string, kubectlOptions *KubectlOptions) {
+	if kubectlOptions != nil && kubectlOptions.RestConfig != nil {
+		t.Fatalf(
+			"SaveKubectlOptions cannot save options built with a RestConfig, because a rest.Config cannot be "+
+				"serialized. Save the values needed to rebuild it instead, or use options built from a kubeconfig "+
+				"path and context name. Path that would have been written: %s",
+			formatKubectlOptionsPath(testFolder),
+		)
+
+		return
+	}
+
 	teststate.Save(t, formatKubectlOptionsPath(testFolder), true, kubectlOptions)
 }
 

modules/k8s/save_test_data_restconfig_test.go

@@ -0,0 +1,91 @@
+package k8s_test
+
+import (
+	"fmt"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/gruntwork-io/terratest/modules/k8s/v2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"k8s.io/client-go/rest"
+)
+
+// fatalRecorder captures a Fatalf instead of failing the enclosing test. FailNow semantics require that the call
+// does not return, so Fatalf ends the goroutine the way testing.T would.
+type fatalRecorder struct {
+	failed bool
+	msg    string
+}
+
+func (r *fatalRecorder) Fail()                 { r.failed = true }
+func (r *fatalRecorder) FailNow()              { r.failed = true; runtime.Goexit() }
+func (r *fatalRecorder) Error(args ...any)     { r.failed = true }
+func (r *fatalRecorder) Errorf(string, ...any) { r.failed = true }
+func (r *fatalRecorder) Fatal(args ...any)     { r.msg = fmt.Sprint(args...); r.FailNow() }
+func (r *fatalRecorder) Fatalf(format string, args ...any) {
+	r.msg = fmt.Sprintf(format, args...)
+	r.FailNow()
+}
+func (r *fatalRecorder) Name() string { return "fatalRecorder" }
+func (r *fatalRecorder) Helper()      {}
+
+// runAndRecover runs fn on its own goroutine so a FailNow inside it does not end the calling test.
+func runAndRecover(fn func()) {
+	done := make(chan struct{})
+
+	go func() {
+		defer close(done)
+		fn()
+	}()
+
+	<-done
+}
+
+// TestSaveKubectlOptionsRejectsRestConfig pins that saving options built with a RestConfig fails loudly rather
+// than writing a file whose reload would silently target the ambient kubeconfig cluster.
+func TestSaveKubectlOptionsRejectsRestConfig(t *testing.T) {
+	t.Parallel()
+
+	folder := t.TempDir()
+	options := k8s.NewKubectlOptionsWithRestConfig(&rest.Config{Host: "https://example.com"}, "default")
+
+	recorder := &fatalRecorder{}
+	runAndRecover(func() { k8s.SaveKubectlOptions(recorder, folder, options) })
+
+	assert.True(t, recorder.failed, "saving options with a RestConfig must fail the test")
+	assert.Contains(t, recorder.msg, "RestConfig", "the message must name the offending field")
+	assert.NoFileExists(t, filepath.Join(folder, ".test-data", "KubectlOptions.json"),
+		"no file may be written when the save is rejected")
+}
+
+// TestSaveKubectlOptionsAcceptsKubeconfigOptions is the companion: the ordinary path is untouched.
+func TestSaveKubectlOptionsAcceptsKubeconfigOptions(t *testing.T) {
+	t.Parallel()
+
+	folder := t.TempDir()
+	options := k8s.NewKubectlOptions("terratest-context", "~/.kube/config", "default")
+
+	k8s.SaveKubectlOptions(t, folder, options)
+	require.FileExists(t, filepath.Join(folder, ".test-data", "KubectlOptions.json"))
+
+	loaded := k8s.LoadKubectlOptions(t, folder)
+	assert.Equal(t, "terratest-context", loaded.ContextName)
+	assert.Equal(t, "default", loaded.Namespace)
+	assert.Nil(t, loaded.RestConfig)
+}
+
+// TestSaveKubectlOptionsMessageIsActionable keeps the failure message useful rather than just a marshal error.
+func TestSaveKubectlOptionsMessageIsActionable(t *testing.T) {
+	t.Parallel()
+
+	recorder := &fatalRecorder{}
+	options := k8s.NewKubectlOptionsWithRestConfig(&rest.Config{Host: "https://example.com"}, "default")
+	runAndRecover(func() { k8s.SaveKubectlOptions(recorder, t.TempDir(), options) })
+
+	assert.NotContains(t, strings.ToLower(recorder.msg), "unsupported type",
+		"the caller should not be shown a raw encoding/json error")
+	assert.Contains(t, recorder.msg, "kubeconfig", "the message should point at the supported alternative")
+}