diff --git a/pkg/kubescape/client_init_test.go b/pkg/kubescape/client_init_test.go new file mode 100644 index 0000000..498d70b --- /dev/null +++ b/pkg/kubescape/client_init_test.go @@ -0,0 +1,192 @@ +package kubescape + +import ( + "context" + "errors" + "testing" + "time" + + kubescapefake "github.com/kubescape/storage/pkg/generated/clientset/versioned/fake" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + kubefake "k8s.io/client-go/kubernetes/fake" +) + +// workingClients returns a client set backed by fakes, standing in for a +// successful construction against a reachable API server. +func workingClients() *kubescapeClients { + return &kubescapeClients{ + k8s: kubefake.NewClientset(), + spdx: kubescapefake.NewClientset().SpdxV1beta1(), + } +} + +// toolWithBuilder builds a tool whose client construction and clock are both +// under the test's control, so recovery and rate limiting can be exercised +// without a cluster and without sleeping. +func toolWithBuilder(build func() (*kubescapeClients, error), clock *time.Time) *KubescapeTool { + return &KubescapeTool{ + buildClients: build, + now: func() time.Time { return *clock }, + } +} + +// A client construction that fails once -- because the API server was not yet +// reachable when the pod started -- must not disable the provider for the +// lifetime of the process. This is P5: previously initError was set once and +// every handler short-circuited on it forever. +func TestEnsureClients_RecoversAfterTransientFailure(t *testing.T) { + attempts := 0 + clock := time.Now() + tool := toolWithBuilder(func() (*kubescapeClients, error) { + attempts++ + if attempts == 1 { + return nil, errors.New("dial tcp 10.96.0.1:443: connect: connection refused") + } + return workingClients(), nil + }, &clock) + + result, err := tool.HandleListVulnerabilityManifests(context.Background(), makeRequest(nil)) + require.NoError(t, err) + assert.True(t, result.IsError, "first call should report the construction failure") + + clock = clock.Add(clientRetryInterval + time.Second) + + result, err = tool.HandleListVulnerabilityManifests(context.Background(), makeRequest(nil)) + require.NoError(t, err) + assert.False(t, result.IsError, "provider should recover once the API server is reachable") + assert.Equal(t, 2, attempts) +} + +// Once clients are built they are reused; a healthy provider must not rebuild +// its clients on every call. +func TestEnsureClients_BuildsOnce(t *testing.T) { + attempts := 0 + clock := time.Now() + tool := toolWithBuilder(func() (*kubescapeClients, error) { + attempts++ + return workingClients(), nil + }, &clock) + + for i := 0; i < 3; i++ { + _, err := tool.HandleListVulnerabilityManifests(context.Background(), makeRequest(nil)) + require.NoError(t, err) + } + + assert.Equal(t, 1, attempts) +} + +// A failing construction is cached for clientRetryInterval, so a hot loop of +// tool calls during an outage cannot hammer the API server -- each attempt can +// block for a full dial timeout. +func TestEnsureClients_DoesNotRetryWithinInterval(t *testing.T) { + attempts := 0 + clock := time.Now() + tool := toolWithBuilder(func() (*kubescapeClients, error) { + attempts++ + return nil, errors.New("connection refused") + }, &clock) + + _, err := tool.HandleListVulnerabilityManifests(context.Background(), makeRequest(nil)) + require.NoError(t, err) + + clock = clock.Add(clientRetryInterval / 2) + + result, err := tool.HandleListVulnerabilityManifests(context.Background(), makeRequest(nil)) + require.NoError(t, err) + assert.True(t, result.IsError, "the cached failure is still reported") + assert.Equal(t, 1, attempts, "no second construction attempt within the interval") +} + +// P7: a call missing a required argument must report the missing argument, not +// a kubeconfig error. Argument validation needs no cluster, so it runs first -- +// asserted by the builder never being called. +func TestHandlers_ValidateArgumentsBeforeBuildingClients(t *testing.T) { + tests := []struct { + name string + handler func(*KubescapeTool, context.Context) (string, error) + want string + }{ + { + name: "list_vulnerabilities without manifest_name", + handler: func(k *KubescapeTool, ctx context.Context) (string, error) { + r, err := k.HandleListVulnerabilitiesInManifest(ctx, makeRequest(nil)) + return getResultText(r), err + }, + want: "manifest_name", + }, + { + name: "get_vulnerability_details without cve_id", + handler: func(k *KubescapeTool, ctx context.Context) (string, error) { + r, err := k.HandleGetVulnerabilityDetails(ctx, makeRequest(map[string]interface{}{ + "manifest_name": "some-manifest", + })) + return getResultText(r), err + }, + want: "cve_id", + }, + { + name: "get_configuration_scan without manifest_name", + handler: func(k *KubescapeTool, ctx context.Context) (string, error) { + r, err := k.HandleGetConfigurationScan(ctx, makeRequest(nil)) + return getResultText(r), err + }, + want: "manifest_name", + }, + { + name: "get_application_profile without name", + handler: func(k *KubescapeTool, ctx context.Context) (string, error) { + r, err := k.HandleGetApplicationProfile(ctx, makeRequest(map[string]interface{}{ + "namespace": "default", + })) + return getResultText(r), err + }, + want: "name", + }, + { + name: "get_network_neighborhood without namespace", + handler: func(k *KubescapeTool, ctx context.Context) (string, error) { + r, err := k.HandleGetNetworkNeighborhood(ctx, makeRequest(map[string]interface{}{ + "name": "some-nn", + })) + return getResultText(r), err + }, + want: "namespace", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + attempts := 0 + clock := time.Now() + tool := toolWithBuilder(func() (*kubescapeClients, error) { + attempts++ + return nil, errors.New("failed to create kubernetes config: no configuration has been provided") + }, &clock) + + text, err := tt.handler(tool, context.Background()) + require.NoError(t, err) + + assert.Contains(t, text, tt.want) + assert.NotContains(t, text, "no configuration has been provided") + assert.Equal(t, 0, attempts, "argument validation must not require a cluster") + }) + } +} + +// The exported test helpers keep behaving as the existing suite expects: +// pre-built clients are used as-is, and a tool built with an error stays in +// that error state rather than falling back to an ambient kubeconfig. +func TestNewKubescapeToolWithError_StaysFailed(t *testing.T) { + tool := NewKubescapeToolWithError(errors.New("failed to connect")) + + result, err := tool.HandleListVulnerabilityManifests(context.Background(), makeRequest(nil)) + require.NoError(t, err) + require.True(t, result.IsError) + + tool.lastAttempt = time.Time{} // any retry window has long expired + + result, err = tool.HandleListVulnerabilityManifests(context.Background(), makeRequest(nil)) + require.NoError(t, err) + assert.True(t, result.IsError, "must not fall through to an ambient kubeconfig") +} diff --git a/pkg/kubescape/kubescape.go b/pkg/kubescape/kubescape.go index e2227fa..89394c9 100644 --- a/pkg/kubescape/kubescape.go +++ b/pkg/kubescape/kubescape.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "strings" + "sync" "time" "github.com/kagent-dev/tools/internal/errors" @@ -38,49 +39,114 @@ const ( storagePodLabel = "app.kubernetes.io/name=storage" ) -// KubescapeTool holds the clients for Kubescape and Kubernetes APIs +// clientRetryInterval is how long a failed client construction is cached +// before another attempt is made. Retrying on every call would let a loop of +// failing tool calls hammer the API server -- and each attempt can block for a +// full dial timeout -- while waiting much longer would leave the provider dead +// well after the cluster recovered. +const clientRetryInterval = 30 * time.Second + +// kubescapeClients is the set of API clients the handlers need. They are built +// together because they share one rest.Config: if one can be built they all +// can, so there is no partially-usable state to represent. +type kubescapeClients struct { + spdx spdxv1beta1.SpdxV1beta1Interface + k8s kubernetes.Interface + apiExt apiextensionsclientset.Interface +} + +// KubescapeTool holds the clients for Kubescape and Kubernetes APIs. +// +// The clients are built on first use rather than at registration. In-cluster +// the provider can start before the API server is reachable or before its RBAC +// has been applied; building once at startup meant such a transient failure +// disabled every Kubescape tool until the pod was restarted. +// +// Every field below is read and written only while holding mu, so a failed +// start recovers safely even with concurrent tool calls. type KubescapeTool struct { + mu sync.Mutex spdxClient spdxv1beta1.SpdxV1beta1Interface k8sClient kubernetes.Interface apiExtClient apiextensionsclientset.Interface - initError error + lastErr error + lastAttempt time.Time + + // buildClients and now are injected so tests can exercise recovery and the + // retry interval without a cluster and without sleeping. + buildClients func() (*kubescapeClients, error) + now func() time.Time } -// NewKubescapeTool creates a new KubescapeTool with Kubernetes clients +// NewKubescapeTool creates a new KubescapeTool. It cannot fail: connection +// problems surface on the first tool call, where they are retried, instead of +// permanently at registration time. Use kubescape_check_health to verify the +// installation. func NewKubescapeTool(kubeconfig string) *KubescapeTool { - tool := &KubescapeTool{} + return &KubescapeTool{ + buildClients: func() (*kubescapeClients, error) { return newClients(kubeconfig) }, + now: time.Now, + } +} +// newClients builds all three API clients from a single rest.Config. +func newClients(kubeconfig string) (*kubescapeClients, error) { config, err := getKubeConfig(kubeconfig) if err != nil { - tool.initError = fmt.Errorf("failed to create kubernetes config: %w", err) - return tool + return nil, fmt.Errorf("failed to create kubernetes config: %w", err) } - // Create standard Kubernetes client + // Standard Kubernetes client k8sClient, err := kubernetes.NewForConfig(config) if err != nil { - tool.initError = fmt.Errorf("failed to create kubernetes client: %w", err) - return tool + return nil, fmt.Errorf("failed to create kubernetes client: %w", err) } - tool.k8sClient = k8sClient - // Create API extensions client for CRD checks + // API extensions client for CRD checks apiExtClient, err := apiextensionsclientset.NewForConfig(config) if err != nil { - tool.initError = fmt.Errorf("failed to create apiextensions client: %w", err) - return tool + return nil, fmt.Errorf("failed to create apiextensions client: %w", err) } - tool.apiExtClient = apiExtClient - // Create Kubescape storage client + // Kubescape storage client spdxClient, err := spdxv1beta1.NewForConfig(config) if err != nil { - tool.initError = fmt.Errorf("failed to create kubescape client: %w", err) - return tool + return nil, fmt.Errorf("failed to create kubescape client: %w", err) + } + + return &kubescapeClients{spdx: spdxClient, k8s: k8sClient, apiExt: apiExtClient}, nil +} + +// ensureClients returns nil once the clients are usable. A previous failure is +// reported without a retry for clientRetryInterval and retried after that, so +// the provider recovers on its own once the cluster becomes reachable. +// +// Handlers must call this before touching any client, and must call it after +// validating their arguments -- argument errors need no cluster, and reporting +// a connection error for a malformed call misdirects the caller. +func (k *KubescapeTool) ensureClients() error { + k.mu.Lock() + defer k.mu.Unlock() + + if k.spdxClient != nil { + return nil + } + if k.lastErr != nil && k.now().Sub(k.lastAttempt) < clientRetryInterval { + return k.lastErr + } + + k.lastAttempt = k.now() + clients, err := k.buildClients() + if err != nil { + k.lastErr = err + return err } - tool.spdxClient = spdxClient - return tool + k.spdxClient = clients.spdx + k.k8sClient = clients.k8s + k.apiExtClient = clients.apiExt + k.lastErr = nil + return nil } func getKubeConfig(kubeconfig string) (*rest.Config, error) { @@ -116,8 +182,8 @@ type CheckStatus struct { // handleCheckHealth verifies Kubescape operator installation and readiness func (k *KubescapeTool) handleCheckHealth(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("check_health", k.initError) + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("check_health", err) return toolErr.ToMCPResult(), nil } @@ -464,8 +530,8 @@ func (k *KubescapeTool) handleCheckHealth(ctx context.Context, request mcp.CallT // handleListVulnerabilityManifests lists vulnerability manifests at image and workload levels func (k *KubescapeTool) handleListVulnerabilityManifests(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("list_vulnerability_manifests", k.initError) + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("list_vulnerability_manifests", err) return toolErr.ToMCPResult(), nil } @@ -534,11 +600,6 @@ func (k *KubescapeTool) handleListVulnerabilityManifests(ctx context.Context, re // handleListVulnerabilitiesInManifest lists all CVEs in a specific manifest func (k *KubescapeTool) handleListVulnerabilitiesInManifest(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("list_vulnerabilities", k.initError) - return toolErr.ToMCPResult(), nil - } - namespace := mcp.ParseString(request, "namespace", defaultKubescapeNamespace) manifestName := mcp.ParseString(request, "manifest_name", "") @@ -546,6 +607,11 @@ func (k *KubescapeTool) handleListVulnerabilitiesInManifest(ctx context.Context, return mcp.NewToolResultError("manifest_name parameter is required"), nil } + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("list_vulnerabilities", err) + return toolErr.ToMCPResult(), nil + } + manifest, err := k.spdxClient.VulnerabilityManifests(namespace).Get(ctx, manifestName, metav1.GetOptions{}) if err != nil { toolErr := errors.NewKubescapeError("get_vulnerability_manifest", err). @@ -606,11 +672,6 @@ func (k *KubescapeTool) handleListVulnerabilitiesInManifest(ctx context.Context, // handleGetVulnerabilityDetails gets detailed info about a specific CVE in a manifest func (k *KubescapeTool) handleGetVulnerabilityDetails(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("get_vulnerability_details", k.initError) - return toolErr.ToMCPResult(), nil - } - namespace := mcp.ParseString(request, "namespace", defaultKubescapeNamespace) manifestName := mcp.ParseString(request, "manifest_name", "") cveID := mcp.ParseString(request, "cve_id", "") @@ -622,6 +683,11 @@ func (k *KubescapeTool) handleGetVulnerabilityDetails(ctx context.Context, reque return mcp.NewToolResultError("cve_id parameter is required"), nil } + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("get_vulnerability_details", err) + return toolErr.ToMCPResult(), nil + } + manifest, err := k.spdxClient.VulnerabilityManifests(namespace).Get(ctx, manifestName, metav1.GetOptions{}) if err != nil { toolErr := errors.NewKubescapeError("get_vulnerability_manifest", err). @@ -652,8 +718,8 @@ func (k *KubescapeTool) handleGetVulnerabilityDetails(ctx context.Context, reque // handleListConfigurationScans lists configuration security scan results func (k *KubescapeTool) handleListConfigurationScans(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("list_configuration_scans", k.initError) + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("list_configuration_scans", err) return toolErr.ToMCPResult(), nil } @@ -696,11 +762,6 @@ func (k *KubescapeTool) handleListConfigurationScans(ctx context.Context, reques // handleGetConfigurationScan gets details of a specific configuration scan func (k *KubescapeTool) handleGetConfigurationScan(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("get_configuration_scan", k.initError) - return toolErr.ToMCPResult(), nil - } - namespace := mcp.ParseString(request, "namespace", defaultKubescapeNamespace) manifestName := mcp.ParseString(request, "manifest_name", "") @@ -708,6 +769,11 @@ func (k *KubescapeTool) handleGetConfigurationScan(ctx context.Context, request return mcp.NewToolResultError("manifest_name parameter is required"), nil } + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("get_configuration_scan", err) + return toolErr.ToMCPResult(), nil + } + manifest, err := k.spdxClient.WorkloadConfigurationScans(namespace).Get(ctx, manifestName, metav1.GetOptions{}) if err != nil { toolErr := errors.NewKubescapeError("get_configuration_scan", err). @@ -726,8 +792,8 @@ func (k *KubescapeTool) handleGetConfigurationScan(ctx context.Context, request // handleListApplicationProfiles lists application profiles showing runtime behavior data func (k *KubescapeTool) handleListApplicationProfiles(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("list_application_profiles", k.initError) + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("list_application_profiles", err) return toolErr.ToMCPResult(), nil } @@ -801,11 +867,6 @@ func (k *KubescapeTool) handleListApplicationProfiles(ctx context.Context, reque // handleGetApplicationProfile gets detailed runtime behavior for a specific workload func (k *KubescapeTool) handleGetApplicationProfile(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("get_application_profile", k.initError) - return toolErr.ToMCPResult(), nil - } - namespace := mcp.ParseString(request, "namespace", "") name := mcp.ParseString(request, "name", "") @@ -816,6 +877,11 @@ func (k *KubescapeTool) handleGetApplicationProfile(ctx context.Context, request return mcp.NewToolResultError("namespace parameter is required"), nil } + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("get_application_profile", err) + return toolErr.ToMCPResult(), nil + } + profile, err := k.spdxClient.ApplicationProfiles(namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { toolErr := errors.NewKubescapeError("get_application_profile", err). @@ -877,8 +943,8 @@ func (k *KubescapeTool) handleGetApplicationProfile(ctx context.Context, request // handleListNetworkNeighborhoods lists network communication patterns for workloads func (k *KubescapeTool) handleListNetworkNeighborhoods(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("list_network_neighborhoods", k.initError) + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("list_network_neighborhoods", err) return toolErr.ToMCPResult(), nil } @@ -935,11 +1001,6 @@ func (k *KubescapeTool) handleListNetworkNeighborhoods(ctx context.Context, requ // handleGetNetworkNeighborhood gets detailed network connections for a specific workload func (k *KubescapeTool) handleGetNetworkNeighborhood(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - if k.initError != nil { - toolErr := errors.NewKubescapeError("get_network_neighborhood", k.initError) - return toolErr.ToMCPResult(), nil - } - namespace := mcp.ParseString(request, "namespace", "") name := mcp.ParseString(request, "name", "") @@ -950,6 +1011,11 @@ func (k *KubescapeTool) handleGetNetworkNeighborhood(ctx context.Context, reques return mcp.NewToolResultError("namespace parameter is required"), nil } + if err := k.ensureClients(); err != nil { + toolErr := errors.NewKubescapeError("get_network_neighborhood", err) + return toolErr.ToMCPResult(), nil + } + nn, err := k.spdxClient.NetworkNeighborhoods(namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { toolErr := errors.NewKubescapeError("get_network_neighborhood", err). diff --git a/pkg/kubescape/testing.go b/pkg/kubescape/testing.go index 093e371..3ee42a6 100644 --- a/pkg/kubescape/testing.go +++ b/pkg/kubescape/testing.go @@ -1,6 +1,8 @@ package kubescape import ( + "time" + spdxv1beta1 "github.com/kubescape/storage/pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1" apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" "k8s.io/client-go/kubernetes" @@ -16,13 +18,18 @@ func NewKubescapeToolWithClients( k8sClient: k8sClient, apiExtClient: apiExtClient, spdxClient: spdxClient, - initError: nil, + now: time.Now, } } -// NewKubescapeToolWithError creates a KubescapeTool with an initialization error for testing error paths +// NewKubescapeToolWithError creates a KubescapeTool with an initialization error for testing error paths. +// +// The failure is permanent: the injected builder always returns err, so the +// tool never falls back to whatever kubeconfig the test machine happens to +// have once the retry interval elapses. func NewKubescapeToolWithError(err error) *KubescapeTool { return &KubescapeTool{ - initError: err, + buildClients: func() (*kubescapeClients, error) { return nil, err }, + now: time.Now, } }