diff --git a/cmd/context.go b/cmd/context.go index e59fc37..c5c945c 100644 --- a/cmd/context.go +++ b/cmd/context.go @@ -10,7 +10,6 @@ import ( "sort" "strings" - "github.com/fatih/color" "github.com/spf13/cobra" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd/api" @@ -184,35 +183,6 @@ func (o *ContextOptions) switchContextInPlace(sm *state.Manager, contextName str return err } - if n := sm.InPlaceSwitchWarnCount(); n < state.InPlaceSwitchWarnMax { - yellow := color.New(color.FgHiYellow).SprintFunc() - remaining := state.InPlaceSwitchWarnMax - n - 1 - var timesNote string - switch remaining { - case 0: - timesNote = "(last time this warning is shown)" - case 1: - timesNote = "(this warning will be shown 1 more time)" - default: - timesNote = fmt.Sprintf("(this warning will be shown %d more times)", remaining) - } - fmt.Fprintf(o.ErrOut, "%s kubert context behavior has changed (v0.8.0+) - showing this warning because you switched contexts in an active kubert shell.\n", - yellow("Warning:")) - fmt.Fprintln(o.ErrOut) - fmt.Fprintln(o.ErrOut, " Contexts are now updated in-place instead of spawning a new nested shell on every switch.") - fmt.Fprintln(o.ErrOut, " This does not break isolation between shells, but kubert will now reuse the existing shell rather than nesting a new one.") - fmt.Fprintln(o.ErrOut) - fmt.Fprintln(o.ErrOut, " You can safely ignore this if you don't rely on a new nested sub-shell being created on every context switch.") - fmt.Fprintln(o.ErrOut, " Use --nested or set 'nested: true' in config to restore the previous behaviour.") - fmt.Fprintln(o.ErrOut) - fmt.Fprintln(o.ErrOut, " See https://github.com/idebeijer/kubert/releases/tag/v0.8.0 for details.") - fmt.Fprintln(o.ErrOut) - fmt.Fprintf(o.ErrOut, " %s\n", timesNote) - if err := sm.RecordInPlaceSwitchWarn(); err != nil { - slog.Warn("Failed to record in-place switch warning count", "error", err) - } - } - contextInState, _ := sm.ContextInfo(contextName) // Fire post-context hook before switching (signals leaving the old context). diff --git a/cmd/context_test.go b/cmd/context_test.go index 2d36103..110d0ef 100644 --- a/cmd/context_test.go +++ b/cmd/context_test.go @@ -721,65 +721,6 @@ func TestFindContextByName(t *testing.T) { }) } -func TestContextOptions_Run_WarningSuppressedAfterMax(t *testing.T) { - setupTestXDGDataHome(t) - sm, err := state.NewManager() - if err != nil { - t.Fatalf("NewManager: %v", err) - } - - existingKubeconfig, err := os.CreateTemp("", "kubert-existing-*.yaml") - if err != nil { - t.Fatalf("Failed to create temp file: %v", err) - } - defer func() { - _ = existingKubeconfig.Close() - _ = os.Remove(existingKubeconfig.Name()) - }() - - t.Setenv(kubert.ShellActiveEnvVar, "1") - t.Setenv(kubert.ShellKubeconfigEnvVar, existingKubeconfig.Name()) - - makeOpts := func(errBuf *bytes.Buffer) *ContextOptions { - return &ContextOptions{ - Out: &bytes.Buffer{}, - ErrOut: errBuf, - Args: []string{"ctx-b"}, - Config: config.Config{}, - ContextLoader: func() ([]kubeconfig.Context, error) { - return []kubeconfig.Context{ - {Name: "ctx-b", WithPath: kubeconfig.WithPath{FilePath: "/tmp/config"}}, - }, nil - }, - StateManager: func() (*state.Manager, error) { return sm, nil }, - IsInteractive: func() bool { return false }, - ShellLauncher: func(_, _, _ string, _ config.Config) error { return nil }, - TempFileWriter: func(_, _, _ string) (*os.File, func(), error) { return nil, nil, nil }, - InPlaceWriter: func(_, _, _, _ string) error { return nil }, - } - } - - // First 3 invocations should print a notice to stderr. - for i := 1; i <= state.InPlaceSwitchWarnMax; i++ { - var errBuf bytes.Buffer - if err := makeOpts(&errBuf).Run(); err != nil { - t.Fatalf("Run() invocation %d: %v", i, err) - } - if !strings.Contains(errBuf.String(), "Warning:") { - t.Errorf("invocation %d: expected warning in stderr, got: %q", i, errBuf.String()) - } - } - - // 4th invocation should produce no notice. - var errBuf bytes.Buffer - if err := makeOpts(&errBuf).Run(); err != nil { - t.Fatalf("Run() invocation 4: %v", err) - } - if strings.Contains(errBuf.String(), "Warning:") { - t.Errorf("invocation 4: warning should not appear after max, got: %q", errBuf.String()) - } -} - func TestContextOptions_Run_InPlaceSwitch(t *testing.T) { var buf bytes.Buffer inPlaceWriterCalled := false diff --git a/internal/state/state.go b/internal/state/state.go index 17fe52e..3704ee8 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -18,9 +18,8 @@ const ( ) type State struct { - Contexts map[string]ContextInfo `json:"contexts"` - LastContext string `json:"last_context,omitempty"` - InPlaceSwitchWarnCount int `json:"in_place_switch_warn_count,omitempty"` + Contexts map[string]ContextInfo `json:"contexts"` + LastContext string `json:"last_context,omitempty"` } type Manager struct { @@ -71,6 +70,16 @@ func NewManager() (*Manager, error) { if err := json.Unmarshal(data, &manager.state); err != nil { return nil, fmt.Errorf("failed to unmarshal state: %w", err) } + + // Prune stale fields left by older versions. + var raw map[string]json.RawMessage + if json.Unmarshal(data, &raw) == nil { + if _, found := raw["in_place_switch_warn_count"]; found { + if err := manager.saveState(); err != nil { + slog.Warn("failed to prune stale state fields", "error", err) + } + } + } } return manager, nil diff --git a/internal/state/state_test.go b/internal/state/state_test.go index 24ea141..9f36645 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -1,6 +1,7 @@ package state import ( + "encoding/json" "errors" "fmt" "os" @@ -515,6 +516,42 @@ func TestManager_LiftContextProtection_NonExistingContext(t *testing.T) { } } +func TestNewManager_PrunesStaleInPlaceSwitchWarnCount(t *testing.T) { + tempDir := t.TempDir() + orig := xdg.DataHome + xdg.DataHome = tempDir + t.Cleanup(func() { xdg.DataHome = orig }) + + // Write a state file that contains the stale field from v0.8.x. + stateDir := fmt.Sprintf("%s/kubert", tempDir) + if err := os.MkdirAll(stateDir, 0o700); err != nil { + t.Fatal(err) + } + stale := []byte(`{"contexts":{},"in_place_switch_warn_count":2}`) + if err := os.WriteFile(stateDir+"/state.json", stale, 0o600); err != nil { + t.Fatal(err) + } + + if _, err := NewManager(); err != nil { + t.Fatalf("NewManager: %v", err) + } + + data, err := os.ReadFile(stateDir + "/state.json") + if err != nil { + t.Fatal(err) + } + if string(data) == string(stale) { + t.Error("state file was not rewritten to prune stale field") + } + var cleaned map[string]any + if err := json.Unmarshal(data, &cleaned); err != nil { + t.Fatalf("state file is not valid JSON after migration: %v", err) + } + if _, found := cleaned["in_place_switch_warn_count"]; found { + t.Error("stale field in_place_switch_warn_count was not removed from state file") + } +} + func TestManager_ClearProtectedUntil_NonExistingContext(t *testing.T) { manager, tempDir := setupTestManager(t) defer cleanupTestManager(tempDir) diff --git a/internal/state/warnings.go b/internal/state/warnings.go deleted file mode 100644 index 366d7c5..0000000 --- a/internal/state/warnings.go +++ /dev/null @@ -1,19 +0,0 @@ -package state - -const InPlaceSwitchWarnMax = 3 - -// InPlaceSwitchWarnCount returns how many times the in-place switch notice has -// been shown to the user. -func (m *Manager) InPlaceSwitchWarnCount() int { - m.mutex.Lock() - defer m.mutex.Unlock() - return m.state.InPlaceSwitchWarnCount -} - -// RecordInPlaceSwitchWarn increments the notice counter and persists it. -func (m *Manager) RecordInPlaceSwitchWarn() error { - return m.withLock(func() error { - m.state.InPlaceSwitchWarnCount++ - return m.saveState() - }) -} diff --git a/internal/state/warnings_test.go b/internal/state/warnings_test.go deleted file mode 100644 index 781efe3..0000000 --- a/internal/state/warnings_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package state - -import ( - "testing" - - "github.com/adrg/xdg" -) - -func TestInPlaceSwitchWarnCount(t *testing.T) { - orig := xdg.DataHome - xdg.DataHome = t.TempDir() - t.Cleanup(func() { xdg.DataHome = orig }) - - m, err := NewManager() - if err != nil { - t.Fatalf("NewManager: %v", err) - } - - if got := m.InPlaceSwitchWarnCount(); got != 0 { - t.Fatalf("initial count: want 0, got %d", got) - } - - for i := 1; i <= InPlaceSwitchWarnMax; i++ { - if err := m.RecordInPlaceSwitchWarn(); err != nil { - t.Fatalf("RecordInPlaceSwitchWarn iteration %d: %v", i, err) - } - if got := m.InPlaceSwitchWarnCount(); got != i { - t.Fatalf("after %d records: want %d, got %d", i, i, got) - } - } -} - -func TestInPlaceSwitchWarnCount_Persisted(t *testing.T) { - orig := xdg.DataHome - xdg.DataHome = t.TempDir() - t.Cleanup(func() { xdg.DataHome = orig }) - - m, err := NewManager() - if err != nil { - t.Fatalf("NewManager: %v", err) - } - if err := m.RecordInPlaceSwitchWarn(); err != nil { - t.Fatalf("RecordInPlaceSwitchWarn: %v", err) - } - - // Re-open the state file and verify the count survived. - m2, err := NewManager() - if err != nil { - t.Fatalf("NewManager (2nd): %v", err) - } - if got := m2.InPlaceSwitchWarnCount(); got != 1 { - t.Fatalf("persisted count: want 1, got %d", got) - } -}