From 13da204fede7245fcc3fd3e1edaab5e9b7d3a9c3 Mon Sep 17 00:00:00 2001 From: Johannes Hufnagel Date: Wed, 1 Jul 2026 10:25:25 +0200 Subject: [PATCH] fix(namespace): prevent CanIGetPods from panicking on denied access --- internal/adapters/kubernetes/commands.go | 8 +- internal/adapters/kubernetes/commands_test.go | 75 +++++++++++++++++++ internal/namespace/namespace.go | 18 +++-- internal/testutil/executor_mock.go | 48 ++++++++++++ 4 files changed, 143 insertions(+), 6 deletions(-) create mode 100644 internal/adapters/kubernetes/commands_test.go create mode 100644 internal/testutil/executor_mock.go diff --git a/internal/adapters/kubernetes/commands.go b/internal/adapters/kubernetes/commands.go index eb3316c..51717c9 100644 --- a/internal/adapters/kubernetes/commands.go +++ b/internal/adapters/kubernetes/commands.go @@ -2,11 +2,13 @@ package kubernetes import ( "encoding/json" + "strings" + "github.com/conplementAG/copsctl/internal/common" "github.com/conplementAG/copsctl/internal/common/file_processing" "github.com/conplementag/cops-hq/v2/pkg/commands" + "github.com/conplementag/cops-hq/v2/pkg/error_handling" "github.com/sirupsen/logrus" - "strings" ) func PrintAllCopsNamespaces(executor commands.Executor) error { @@ -89,6 +91,10 @@ func DeleteString(executor commands.Executor, content string) (string, error) { } func CanIGetPods(executor commands.Executor, namespace string) bool { + // A non-"yes" answer exits non-zero; tolerate it so the caller can retry instead of panicking. + defer func(previous bool) { error_handling.PanicOnAnyError = previous }(error_handling.PanicOnAnyError) + error_handling.PanicOnAnyError = false + data, err := executor.ExecuteWithProgressInfo("kubectl auth can-i get pods -n " + namespace) return err == nil && strings.TrimSuffix(data, "\n") == "yes" } diff --git a/internal/adapters/kubernetes/commands_test.go b/internal/adapters/kubernetes/commands_test.go new file mode 100644 index 0000000..53918eb --- /dev/null +++ b/internal/adapters/kubernetes/commands_test.go @@ -0,0 +1,75 @@ +package kubernetes + +import ( + "errors" + "testing" + + "github.com/conplementAG/copsctl/internal/testutil" + "github.com/conplementag/cops-hq/v2/pkg/error_handling" + "github.com/stretchr/testify/assert" +) + +func Test_CanIGetPods_result(t *testing.T) { + data := []struct { + testName string + output string + err error + expected bool + }{ + {testName: "yes grants access", output: "yes\n", err: nil, expected: true}, + {testName: "no denies access", output: "no\n", err: nil, expected: false}, + {testName: "error denies access", output: "no\n", err: errors.New("exit status 1"), expected: false}, + {testName: "unexpected output denies access", output: "maybe", err: nil, expected: false}, + } + + for _, test := range data { + t.Run(test.testName, func(t *testing.T) { + executor := &testutil.ExecutorMock{Output: test.output, Err: test.err} + + assert.Equal(t, test.expected, CanIGetPods(executor, "some-namespace")) + }) + } +} + +func Test_CanIGetPods_restoresPanicOnAnyError(t *testing.T) { + data := []struct { + testName string + initial bool + }{ + {testName: "restores true", initial: true}, + {testName: "restores false", initial: false}, + } + + for _, test := range data { + t.Run(test.testName, func(t *testing.T) { + original := error_handling.PanicOnAnyError + defer func() { error_handling.PanicOnAnyError = original }() + error_handling.PanicOnAnyError = test.initial + + // an error would normally trip the global panic behaviour - CanIGetPods must tolerate it + executor := &testutil.ExecutorMock{Output: "no\n", Err: errors.New("exit status 1")} + + CanIGetPods(executor, "some-namespace") + + assert.Equal(t, test.initial, error_handling.PanicOnAnyError, "PanicOnAnyError must be restored to its previous value") + }) + } +} + +func Test_CanIGetPods_disablesPanicOnAnyErrorDuringCommand(t *testing.T) { + original := error_handling.PanicOnAnyError + defer func() { error_handling.PanicOnAnyError = original }() + error_handling.PanicOnAnyError = true + + var flagDuringCommand bool + executor := &testutil.ExecutorMock{ + ExecuteFunc: func(command string) (string, error) { + flagDuringCommand = error_handling.PanicOnAnyError + return "yes\n", nil + }, + } + + CanIGetPods(executor, "some-namespace") + + assert.False(t, flagDuringCommand, "PanicOnAnyError must be disabled while the command runs") +} diff --git a/internal/namespace/namespace.go b/internal/namespace/namespace.go index 4f4a1b4..f6ee5b6 100644 --- a/internal/namespace/namespace.go +++ b/internal/namespace/namespace.go @@ -2,11 +2,12 @@ package namespace import ( "fmt" + "time" + "github.com/conplementAG/copsctl/internal/cmd/flags" "github.com/conplementag/cops-hq/v2/pkg/commands" "github.com/conplementag/cops-hq/v2/pkg/hq" "github.com/sirupsen/logrus" - "time" "github.com/conplementAG/copsctl/internal/adapters/kubernetes" "github.com/spf13/viper" @@ -87,17 +88,24 @@ func (o *Orchestrator) Delete() { } func ensureNamespaceAccess(executor commands.Executor, namespace string) { + // RBAC bindings are reconciled asynchronously, so poll for up to ~60s until access is available. + const attempts = 20 + status := false - for i := 0; i < 20; i++ { + for i := 0; i < attempts; i++ { status = kubernetes.CanIGetPods(executor, namespace) - if status == true { + if status { break } + + logrus.Infof("Namespace access not ready yet (attempt %d/%d), waiting for RBAC bindings to propagate...", i+1, attempts) time.Sleep(3 * time.Second) } - if status == false { - panic("Could not verify access to pods in created namespace.") + if !status { + panic("Could not verify access to pods in the created namespace after ~60s. " + + "The namespace RBAC bindings may not have been reconciled in time - retry the deployment, " + + "and if the problem persists check the events on the CopsNamespace resource.") } } diff --git a/internal/testutil/executor_mock.go b/internal/testutil/executor_mock.go new file mode 100644 index 0000000..b95c14c --- /dev/null +++ b/internal/testutil/executor_mock.go @@ -0,0 +1,48 @@ +// Package testutil provides shared test doubles for the copsctl test suite. +package testutil + +import ( + "os/exec" + + "github.com/conplementag/cops-hq/v2/pkg/commands" +) + +// ExecutorMock is a minimal commands.Executor test double. All string-returning Execute* +// methods route through ExecuteFunc (or the static Output/Err fallback); the *Cmd/TTY/confirm +// methods are not needed by the code under test and panic if unexpectedly called. +type ExecutorMock struct { + // ExecuteFunc, if set, handles every Execute* call and lets a test script per-call behaviour. + ExecuteFunc func(command string) (string, error) + // Output and Err are returned when ExecuteFunc is nil. + Output string + Err error +} + +// compile-time check that the mock satisfies the interface +var _ commands.Executor = (*ExecutorMock)(nil) + +func (m *ExecutorMock) run(command string) (string, error) { + if m.ExecuteFunc != nil { + return m.ExecuteFunc(command) + } + return m.Output, m.Err +} + +func (m *ExecutorMock) Execute(command string) (string, error) { return m.run(command) } +func (m *ExecutorMock) ExecuteWithProgressInfo(command string) (string, error) { + return m.run(command) +} +func (m *ExecutorMock) ExecuteSilent(command string) (string, error) { return m.run(command) } +func (m *ExecutorMock) ExecuteLoud(command string) (string, error) { return m.run(command) } + +func (m *ExecutorMock) ExecuteCmd(cmd *exec.Cmd) (string, error) { panic("not implemented") } +func (m *ExecutorMock) ExecuteCmdWithProgressInfo(cmd *exec.Cmd) (string, error) { + panic("not implemented") +} +func (m *ExecutorMock) ExecuteCmdSilent(cmd *exec.Cmd) (string, error) { panic("not implemented") } +func (m *ExecutorMock) ExecuteTTY(command string) error { panic("not implemented") } +func (m *ExecutorMock) ExecuteCmdTTY(cmd *exec.Cmd) error { panic("not implemented") } +func (m *ExecutorMock) AskUserToConfirm(displayMessage string) bool { panic("not implemented") } +func (m *ExecutorMock) AskUserToConfirmWithKeyword(displayMessage string, keyword string) bool { + panic("not implemented") +}