From b92512aec068e6d792cf59eb49aafdf9b9ab312d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 16:59:09 +0000 Subject: [PATCH] Complete PR #1602 review - all tests passing Co-authored-by: AlexsJones <1235925+AlexsJones@users.noreply.github.com> --- cmd/serve/serve.go | 2 +- pkg/ai/amazonbedrock_mock_test.go | 26 +++---- pkg/ai/factory.go | 4 +- pkg/ai/openai.go | 8 +-- pkg/analyzer/deployment.go | 4 +- pkg/cache/cache_test.go | 86 +++++++++++------------ pkg/cache/file_based_test.go | 112 +++++++++++++++--------------- pkg/custom/client_test.go | 54 +++++++------- pkg/server/mcp_handlers.go | 8 +-- pkg/server/query/query_test.go | 2 +- pkg/server/server.go | 8 +-- 11 files changed, 157 insertions(+), 157 deletions(-) diff --git a/cmd/serve/serve.go b/cmd/serve/serve.go index 004684ac..04cad870 100644 --- a/cmd/serve/serve.go +++ b/cmd/serve/serve.go @@ -42,7 +42,7 @@ var ( mcpPort string mcpHTTP bool // filters can be injected into the server (repeatable flag) - filters []string + filters []string ) var ServeCmd = &cobra.Command{ diff --git a/pkg/ai/amazonbedrock_mock_test.go b/pkg/ai/amazonbedrock_mock_test.go index b31f528d..be54cc75 100644 --- a/pkg/ai/amazonbedrock_mock_test.go +++ b/pkg/ai/amazonbedrock_mock_test.go @@ -20,11 +20,11 @@ type MockBedrockClient struct { func (m *MockBedrockClient) GetInferenceProfile(ctx context.Context, params *bedrock.GetInferenceProfileInput, optFns ...func(*bedrock.Options)) (*bedrock.GetInferenceProfileOutput, error) { args := m.Called(ctx, params) - + if args.Get(0) == nil { return nil, args.Error(1) } - + return args.Get(0).(*bedrock.GetInferenceProfileOutput), args.Error(1) } @@ -35,11 +35,11 @@ type MockBedrockRuntimeClient struct { func (m *MockBedrockRuntimeClient) InvokeModel(ctx context.Context, params *bedrockruntime.InvokeModelInput, optFns ...func(*bedrockruntime.Options)) (*bedrockruntime.InvokeModelOutput, error) { args := m.Called(ctx, params) - + if args.Get(0) == nil { return nil, args.Error(1) } - + return args.Get(0).(*bedrockruntime.InvokeModelOutput), args.Error(1) } @@ -59,21 +59,21 @@ func TestBedrockInferenceProfileARNWithMocks(t *testing.T) { }, }, } - + // Create a client with test models client := &AmazonBedRockClient{models: testModels} - + // Create mock clients mockMgmtClient := new(MockBedrockClient) mockRuntimeClient := new(MockBedrockRuntimeClient) - + // Inject mock clients into the AmazonBedRockClient client.mgmtClient = mockMgmtClient client.client = mockRuntimeClient - + // Test with a valid inference profile ARN inferenceProfileARN := "arn:aws:bedrock:us-east-1:123456789012:inference-profile/my-profile" - + // Setup mock response for GetInferenceProfile mockMgmtClient.On("GetInferenceProfile", mock.Anything, &bedrock.GetInferenceProfileInput{ InferenceProfileIdentifier: aws.String("my-profile"), @@ -84,20 +84,20 @@ func TestBedrockInferenceProfileARNWithMocks(t *testing.T) { }, }, }, nil) - + // Configure the client with the inference profile ARN config := AIProvider{ Model: inferenceProfileARN, ProviderRegion: "us-east-1", } - + // Test the Configure method with the inference profile ARN err := client.Configure(&config) - + // Verify that the configuration was successful assert.NoError(t, err) assert.Equal(t, inferenceProfileARN, client.model.Config.ModelName) - + // Verify that the mock was called mockMgmtClient.AssertExpectations(t) } diff --git a/pkg/ai/factory.go b/pkg/ai/factory.go index f1af83aa..8045c63b 100644 --- a/pkg/ai/factory.go +++ b/pkg/ai/factory.go @@ -45,7 +45,7 @@ func (p *ViperConfigProvider) UnmarshalKey(key string, rawVal interface{}) error // Default instances to be used var ( - DefaultClientFactory = &DefaultAIClientFactory{} + DefaultClientFactory = &DefaultAIClientFactory{} DefaultConfigProvider = &ViperConfigProvider{} ) @@ -84,4 +84,4 @@ func SetTestConfigProvider(provider ConfigProvider) { func ResetTestImplementations() { testAIClientFactory = nil testConfigProvider = nil -} \ No newline at end of file +} diff --git a/pkg/ai/openai.go b/pkg/ai/openai.go index 66ea456a..4065aa4e 100644 --- a/pkg/ai/openai.go +++ b/pkg/ai/openai.go @@ -94,11 +94,11 @@ func (c *OpenAIClient) GetCompletion(ctx context.Context, prompt string) (string Content: prompt, }, }, - Temperature: c.temperature, + Temperature: c.temperature, MaxCompletionTokens: maxToken, - PresencePenalty: presencePenalty, - FrequencyPenalty: frequencyPenalty, - TopP: c.topP, + PresencePenalty: presencePenalty, + FrequencyPenalty: frequencyPenalty, + TopP: c.topP, }) if err != nil { return "", err diff --git a/pkg/analyzer/deployment.go b/pkg/analyzer/deployment.go index 5ece4e5c..0cf370ce 100644 --- a/pkg/analyzer/deployment.go +++ b/pkg/analyzer/deployment.go @@ -55,7 +55,7 @@ func (d DeploymentAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) for _, deployment := range deployments.Items { var failures []common.Failure if *deployment.Spec.Replicas != deployment.Status.ReadyReplicas { - if deployment.Status.Replicas > *deployment.Spec.Replicas { + if deployment.Status.Replicas > *deployment.Spec.Replicas { doc := apiDoc.GetApiDocV2("spec.replicas") failures = append(failures, common.Failure{ @@ -88,7 +88,7 @@ func (d DeploymentAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) Masked: util.MaskString(deployment.Name), }, }}) - } + } } if len(failures) > 0 { preAnalysis[fmt.Sprintf("%s/%s", deployment.Namespace, deployment.Name)] = common.PreAnalysis{ diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index ad9fec09..704eee37 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -1,60 +1,60 @@ package cache import ( - "os" - "testing" + "os" + "testing" - "github.com/spf13/viper" - "github.com/stretchr/testify/require" + "github.com/spf13/viper" + "github.com/stretchr/testify/require" ) func TestNewReturnsExpectedCache(t *testing.T) { - require.IsType(t, &FileBasedCache{}, New("file")) - require.IsType(t, &AzureCache{}, New("azure")) - require.IsType(t, &GCSCache{}, New("gcs")) - require.IsType(t, &S3Cache{}, New("s3")) - require.IsType(t, &InterplexCache{}, New("interplex")) - // default fallback - require.IsType(t, &FileBasedCache{}, New("unknown")) + require.IsType(t, &FileBasedCache{}, New("file")) + require.IsType(t, &AzureCache{}, New("azure")) + require.IsType(t, &GCSCache{}, New("gcs")) + require.IsType(t, &S3Cache{}, New("s3")) + require.IsType(t, &InterplexCache{}, New("interplex")) + // default fallback + require.IsType(t, &FileBasedCache{}, New("unknown")) } func TestNewCacheProvider_InterplexAndInvalid(t *testing.T) { - // valid: interplex - cp, err := NewCacheProvider("interplex", "", "", "localhost:1", "", "", "", false) - require.NoError(t, err) - require.Equal(t, "interplex", cp.CurrentCacheType) - require.Equal(t, "localhost:1", cp.Interplex.ConnectionString) + // valid: interplex + cp, err := NewCacheProvider("interplex", "", "", "localhost:1", "", "", "", false) + require.NoError(t, err) + require.Equal(t, "interplex", cp.CurrentCacheType) + require.Equal(t, "localhost:1", cp.Interplex.ConnectionString) - // invalid type - _, err = NewCacheProvider("not-a-type", "", "", "", "", "", "", false) - require.Error(t, err) + // invalid type + _, err = NewCacheProvider("not-a-type", "", "", "", "", "", "", false) + require.Error(t, err) } func TestAddRemoveRemoteCacheAndGet(t *testing.T) { - // isolate viper with temp config file - tmpFile, err := os.CreateTemp("", "k8sgpt-cache-config-*.yaml") - require.NoError(t, err) - defer func() { - _ = os.Remove(tmpFile.Name()) - }() - viper.Reset() - viper.SetConfigFile(tmpFile.Name()) + // isolate viper with temp config file + tmpFile, err := os.CreateTemp("", "k8sgpt-cache-config-*.yaml") + require.NoError(t, err) + defer func() { + _ = os.Remove(tmpFile.Name()) + }() + viper.Reset() + viper.SetConfigFile(tmpFile.Name()) - // add interplex remote cache - cp := CacheProvider{} - cp.CurrentCacheType = "interplex" - cp.Interplex.ConnectionString = "localhost:1" - require.NoError(t, AddRemoteCache(cp)) + // add interplex remote cache + cp := CacheProvider{} + cp.CurrentCacheType = "interplex" + cp.Interplex.ConnectionString = "localhost:1" + require.NoError(t, AddRemoteCache(cp)) - // read back via GetCacheConfiguration - c, err := GetCacheConfiguration() - require.NoError(t, err) - require.IsType(t, &InterplexCache{}, c) + // read back via GetCacheConfiguration + c, err := GetCacheConfiguration() + require.NoError(t, err) + require.IsType(t, &InterplexCache{}, c) - // remove remote cache - require.NoError(t, RemoveRemoteCache()) - // now default should be file-based - c2, err := GetCacheConfiguration() - require.NoError(t, err) - require.IsType(t, &FileBasedCache{}, c2) -} \ No newline at end of file + // remove remote cache + require.NoError(t, RemoveRemoteCache()) + // now default should be file-based + c2, err := GetCacheConfiguration() + require.NoError(t, err) + require.IsType(t, &FileBasedCache{}, c2) +} diff --git a/pkg/cache/file_based_test.go b/pkg/cache/file_based_test.go index bafac405..df442127 100644 --- a/pkg/cache/file_based_test.go +++ b/pkg/cache/file_based_test.go @@ -1,77 +1,77 @@ package cache import ( - "os" - "path/filepath" - "testing" + "os" + "path/filepath" + "testing" - "github.com/adrg/xdg" - "github.com/stretchr/testify/require" + "github.com/adrg/xdg" + "github.com/stretchr/testify/require" ) // withTempCacheHome sets XDG_CACHE_HOME to a temp dir for test isolation. func withTempCacheHome(t *testing.T) func() { - t.Helper() - tmp, err := os.MkdirTemp("", "k8sgpt-cache-test-*") - require.NoError(t, err) - old := os.Getenv("XDG_CACHE_HOME") - require.NoError(t, os.Setenv("XDG_CACHE_HOME", tmp)) - return func() { - _ = os.Setenv("XDG_CACHE_HOME", old) - _ = os.RemoveAll(tmp) - } + t.Helper() + tmp, err := os.MkdirTemp("", "k8sgpt-cache-test-*") + require.NoError(t, err) + old := os.Getenv("XDG_CACHE_HOME") + require.NoError(t, os.Setenv("XDG_CACHE_HOME", tmp)) + return func() { + _ = os.Setenv("XDG_CACHE_HOME", old) + _ = os.RemoveAll(tmp) + } } func TestFileBasedCache_BasicOps(t *testing.T) { - cleanup := withTempCacheHome(t) - defer cleanup() + cleanup := withTempCacheHome(t) + defer cleanup() - c := &FileBasedCache{} - // Configure should be a no-op - require.NoError(t, c.Configure(CacheProvider{})) - require.Equal(t, "file", c.GetName()) - require.False(t, c.IsCacheDisabled()) - c.DisableCache() - require.True(t, c.IsCacheDisabled()) + c := &FileBasedCache{} + // Configure should be a no-op + require.NoError(t, c.Configure(CacheProvider{})) + require.Equal(t, "file", c.GetName()) + require.False(t, c.IsCacheDisabled()) + c.DisableCache() + require.True(t, c.IsCacheDisabled()) - key := "testkey" - data := "hello" + key := "testkey" + data := "hello" - // Store - require.NoError(t, c.Store(key, data)) + // Store + require.NoError(t, c.Store(key, data)) - // Exists - require.True(t, c.Exists(key)) + // Exists + require.True(t, c.Exists(key)) - // Load - got, err := c.Load(key) - require.NoError(t, err) - require.Equal(t, data, got) + // Load + got, err := c.Load(key) + require.NoError(t, err) + require.Equal(t, data, got) - // List should include our key file - items, err := c.List() - require.NoError(t, err) - // ensure at least one item and that one matches our key - found := false - for _, it := range items { - if it.Name == key { - found = true - break - } - } - require.True(t, found) + // List should include our key file + items, err := c.List() + require.NoError(t, err) + // ensure at least one item and that one matches our key + found := false + for _, it := range items { + if it.Name == key { + found = true + break + } + } + require.True(t, found) - // Remove - require.NoError(t, c.Remove(key)) - require.False(t, c.Exists(key)) + // Remove + require.NoError(t, c.Remove(key)) + require.False(t, c.Exists(key)) } func TestFileBasedCache_PathShape(t *testing.T) { - cleanup := withTempCacheHome(t) - defer cleanup() - // Verify xdg.CacheFile path shape (directory and filename) - p, err := xdg.CacheFile(filepath.Join("k8sgpt", "abc")) - require.NoError(t, err) - require.Equal(t, "abc", filepath.Base(p)) - require.Contains(t, p, "k8sgpt") -} \ No newline at end of file + cleanup := withTempCacheHome(t) + defer cleanup() + // Verify xdg.CacheFile path shape (directory and filename) + p, err := xdg.CacheFile(filepath.Join("k8sgpt", "abc")) + require.NoError(t, err) + require.Equal(t, "abc", filepath.Base(p)) + require.Contains(t, p, "k8sgpt") +} diff --git a/pkg/custom/client_test.go b/pkg/custom/client_test.go index c14bd82a..83c7eebf 100644 --- a/pkg/custom/client_test.go +++ b/pkg/custom/client_test.go @@ -1,41 +1,41 @@ package custom import ( - "context" - "testing" + "context" + "testing" - schemav1 "buf.build/gen/go/k8sgpt-ai/k8sgpt/protocolbuffers/go/schema/v1" - "github.com/stretchr/testify/require" - "google.golang.org/grpc" + schemav1 "buf.build/gen/go/k8sgpt-ai/k8sgpt/protocolbuffers/go/schema/v1" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" ) // mockAnalyzerClient implements rpc.CustomAnalyzerServiceClient for testing -type mockAnalyzerClient struct{ - resp *schemav1.RunResponse - err error +type mockAnalyzerClient struct { + resp *schemav1.RunResponse + err error } func (m *mockAnalyzerClient) Run(ctx context.Context, in *schemav1.RunRequest, opts ...grpc.CallOption) (*schemav1.RunResponse, error) { - return m.resp, m.err + return m.resp, m.err } func TestClientRunMapsResponse(t *testing.T) { - // prepare fake response - resp := &schemav1.RunResponse{ - Result: &schemav1.Result{ - Name: "AnalyzerA", - Kind: "Pod", - Details: "details", - ParentObject: "Deployment/foo", - }, - } - cli := &Client{analyzerClient: &mockAnalyzerClient{resp: resp}} + // prepare fake response + resp := &schemav1.RunResponse{ + Result: &schemav1.Result{ + Name: "AnalyzerA", + Kind: "Pod", + Details: "details", + ParentObject: "Deployment/foo", + }, + } + cli := &Client{analyzerClient: &mockAnalyzerClient{resp: resp}} - got, err := cli.Run() - require.NoError(t, err) - require.Equal(t, "AnalyzerA", got.Name) - require.Equal(t, "Pod", got.Kind) - require.Equal(t, "details", got.Details) - require.Equal(t, "Deployment/foo", got.ParentObject) - require.Len(t, got.Error, 0) -} \ No newline at end of file + got, err := cli.Run() + require.NoError(t, err) + require.Equal(t, "AnalyzerA", got.Name) + require.Equal(t, "Pod", got.Kind) + require.Equal(t, "details", got.Details) + require.Equal(t, "Deployment/foo", got.ParentObject) + require.Len(t, got.Error, 0) +} diff --git a/pkg/server/mcp_handlers.go b/pkg/server/mcp_handlers.go index e885fc6b..79e7de88 100644 --- a/pkg/server/mcp_handlers.go +++ b/pkg/server/mcp_handlers.go @@ -251,10 +251,10 @@ func (s *K8sGptMCPServer) handleListNamespaces(ctx context.Context, request mcp. // handleListEvents lists Kubernetes events func (s *K8sGptMCPServer) handleListEvents(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { var req struct { - Namespace string `json:"namespace,omitempty"` - InvolvedObjectName string `json:"involvedObjectName,omitempty"` - InvolvedObjectKind string `json:"involvedObjectKind,omitempty"` - Limit int64 `json:"limit,omitempty"` + Namespace string `json:"namespace,omitempty"` + InvolvedObjectName string `json:"involvedObjectName,omitempty"` + InvolvedObjectKind string `json:"involvedObjectKind,omitempty"` + Limit int64 `json:"limit,omitempty"` } if err := request.BindArguments(&req); err != nil { return mcp.NewToolResultErrorf("Failed to parse request arguments: %v", err), nil diff --git a/pkg/server/query/query_test.go b/pkg/server/query/query_test.go index 2c29c1a6..1d6d185a 100644 --- a/pkg/server/query/query_test.go +++ b/pkg/server/query/query_test.go @@ -307,4 +307,4 @@ func TestQuery_GetCompletionError(t *testing.T) { mockAI.AssertExpectations(t) mockFactory.AssertExpectations(t) mockConfig.AssertExpectations(t) -} \ No newline at end of file +} diff --git a/pkg/server/server.go b/pkg/server/server.go index 6c81693f..04063f25 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -55,10 +55,10 @@ type Config struct { QueryHandler *query.Handler Logger *zap.Logger // Filters can be injected into the server to limit analysis to specific analyzers - Filters []string - metricsServer *http.Server - listener net.Listener - EnableHttp bool + Filters []string + metricsServer *http.Server + listener net.Listener + EnableHttp bool } type Health struct {