TestServe started the server in a goroutine that captured Serve's
return value, slept two seconds, then called assert.NoError on it. The
test body returned immediately after Shutdown without ever joining that
goroutine, so the assertion ran after the test had already been recorded
as passed and could never affect its result (a dead assertion). It was
also wrong: closing the listener in Shutdown makes Serve return a
net.ErrClosed error, not nil.
Capture Serve's error over a buffered channel and assert it synchronously
in the test body after Shutdown, checking it wraps net.ErrClosed so the
check actually runs within the test's lifetime and catches an unexpected
Serve failure.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
RunCustomAnalysis built its worker semaphore directly from the unvalidated
a.MaxConcurrency field, unlike RunAnalysis which clamps the value first. With
the user-supplied --max-concurrency flag this caused two failures:
- --max-concurrency 0 makes an unbuffered channel, so the pre-send blocks
waiting for a receiver that is only launched on the next line, deadlocking
the whole analyze command whenever a custom analyzer is configured.
- --max-concurrency -1 makes make(chan struct{}, -1) panic with
"makechan: size out of range".
Apply the same lower/upper bound clamp RunAnalysis already uses (default 10 for
non-positive values, cap at 100). Add regression tests covering both cases.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
TestHPAAnalyzerWithExistingScaleTargetRefWithoutSpecifyingResources placed
its "if !errorFound { t.Error(...) }" assertion inside the outer
"for _, analysis := range analysisResults" loop. When the analyzer emits
zero results (exactly the regression this test guards against, the
"does not have resource configured." detection breaking), the outer loop
body never runs, the assertion is skipped, and the test passes green.
Move the assertion after the outer loop and break out of it once the error
is found, matching the sibling HPA tests (TestHPAAnalyzerWithUnsuportedScaleTargetRef,
TestHPAAnalyzerWithNonExistentScaleTargetRef). Also drop the dead break that
sat inside the inner loop after its own break. Test-only change.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
The EKS integration analyzer built the default kubeconfig path from
os.Getenv("HOME") when no explicit --kubeconfig was provided. On Windows
the HOME environment variable is normally unset (Windows exposes
USERPROFILE, and HOMEDRIVE plus HOMEPATH), so os.Getenv("HOME") returned
"" and filepath.Join collapsed to the relative path .kube/config resolved
against the current working directory rather than the user's home. The
analyzer then loaded an empty config and reported "EKS cluster was not
detected" even when a valid ~/.kube/config existed.
Resolve the home directory with os.UserHomeDir() instead, matching the
convention already used elsewhere in the codebase (cmd/root.go), so the
default kubeconfig path works on Windows as well as Unix. The path
resolution is extracted into a small getKubeconfigPath helper and covered
by a unit test.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
The Deployment analyzer dereferenced *deployment.Spec.Replicas without a
nil check. Spec.Replicas is a *int32 and, although the API server usually
defaults it to 1, a Deployment object whose replicas field is explicitly
unset (nil) panics the analyze run with a nil pointer dereference.
Guard the comparison with a nil check, mirroring the sibling StatefulSet
analyzer which already checks Spec.Replicas != nil before dereferencing.
Add a regression test that analyzes a Deployment with nil Spec.Replicas
and asserts Analyze does not panic.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
The two Vertex AI "Legacy Stable Model" constants carried a trailing '*'
that leaked in from the footnote marker in Google's model-versions table:
ModelGeminiProV1_5 was "gemini-1.5-pro-002*" and ModelGeminiFlashV1_5 was
"gemini-1.5-flash-002*". The '*' is not part of the model id.
GetVertexAIModelOrDefault does an exact-match lookup and otherwise silently
returns VERTEXAI_MODELS[0]. Because VERTEXAI_MODELS held the asterisked
values, a user who correctly configured --model gemini-1.5-pro-002 failed
the exact match and was silently switched to the default model, so the two
legacy models could never be selected.
Remove the trailing '*' from both literals and add a unit test covering the
model and region resolvers.
Closes#1516
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
grpc.NewClient returns a nil *ClientConn when it fails (for example on an
invalid connection string). Store and Load registered defer conn.Close()
before checking the returned error, so a client-creation failure triggered a
nil pointer dereference in the deferred Close instead of returning the error.
Move defer conn.Close() to after the error check in both methods. Add a
regression test that drives Store and Load with an invalid connection string
and asserts they return an error without panicking.
The deferred conn.Close() now discards its error with a plain _ = conn.Close()
rather than logging it: grpc's ClientConn.Close() only returns a non-nil error
on a double-close, which can't happen from a single call site here, so a
logging branch would be dead code that patch coverage can never satisfy.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
The Service analyzer dereferenced EndpointAddress.TargetRef while building
the not-ready pod list, but TargetRef is optional and can be nil for
addresses not backed by a Pod (bare-IP or manually created Endpoints).
A single such address panicked the whole analyze run with a nil pointer
dereference. Only append the pod label when TargetRef is set; the address
is still counted so the failure message is unchanged. Adds a regression
test covering a not-ready address with no TargetRef.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
The Gateway and GatewayClass analyzers indexed Status.Conditions[0]
without a length check. A Gateway or GatewayClass with empty
Status.Conditions (newly created, or no controller installed) panics
the whole analyze run with 'index out of range [0] with length 0'.
Guard both accesses with a length check, mirroring the existing PDB
analyzer (pkg/analyzer/pdb.go). Add a unit test for each analyzer that
exercises the empty-conditions path.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
A pod with non-empty .spec.schedulingGates stays in the Pending phase
with a PodScheduled condition of Status=False, Reason=SchedulingGated.
The pod analyzer only matched the Unschedulable reason, so a gated pod
fell through every branch and was reported as having no problems.
Add a sibling check on the PodScheduled condition for the
SchedulingGated reason. Gated pods commonly carry an empty condition
message, so emit a default message naming the pod when none is present
and surface the condition message otherwise, mirroring how the
Unschedulable branch reports.
This is the residual half of #1474; the suspended Job/CronJob half is
already handled in job.go and cronjob.go.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Implement the Anthropic backend client for Claude models, including:
- AnthropicClient with Configure/GetCompletion/GetName methods
- Proxy, custom headers, and base URL support
- Default model (claude-3-5-sonnet-latest) fallback in auth add command
- Unit tests with httptest mock server
Signed-off-by: Alex <alexsimonjones@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add detection for dynamically loaded ConfigMaps (Grafana sidecar)
- Support grafana_dashboard and grafana_datasource labels
- Support prometheus_rule and fluentd_config labels
- Add k8sgpt.ai/dynamically-loaded label for custom patterns
- Add k8sgpt.ai/skip-usage-check annotation to opt-out
- Add comprehensive test cases for sidecar patterns
Fixes false positives where ConfigMaps loaded dynamically by sidecar
containers (via Kubernetes API watches with label selectors) were
incorrectly flagged as unused.
Tested on production cluster with kube-prometheus-stack:
- Before: 29 ConfigMaps incorrectly flagged as unused
- After: No false positives (29 eliminated - 100% reduction)
Signed-off-by: sqautboy <migonyoung01@gmail.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
* fix: recognize GKE built-in ingress classes 'gce' and 'gce-internal'
Skip IngressClass existence validation for GKE's built-in ingress classes
that work without an explicit IngressClass resource in the cluster.
This fixes false positive errors when using 'gce' or 'gce-internal'
ingress class on GKE.
Closes#849
Signed-off-by: majiayu000 <1835304752@qq.com>
* chore: trigger CI re-run
Signed-off-by: majiayu000 <1835304752@qq.com>
---------
Signed-off-by: majiayu000 <1835304752@qq.com>
* feat(serve): add short flag and env var for metrics port
Add short flag -m for --metrics-port to improve discoverability.
Add K8SGPT_METRICS_PORT environment variable support, consistent
with other K8SGPT_* environment variables.
This helps users who encounter port conflicts on the default
metrics port (8081) when running k8sgpt serve with --mcp or
other configurations.
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxesyes3inatrenchcoat@gmail.com>
* fix: validate namespace before running custom analyzers
Custom analyzers previously ignored the --namespace flag entirely,
executing even when an invalid or misspelled namespace was provided.
This was inconsistent with built-in filter behavior, which respects
namespace scoping.
Add namespace existence validation in RunCustomAnalysis() before
executing custom analyzers. If a namespace is specified but does
not exist, an error is reported and custom analyzers are skipped.
Fixes#1601
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxesyes3inatrenchcoat@gmail.com>
---------
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxesyes3inatrenchcoat@gmail.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
This fixes issue #1556 where the customrest backend fails when error messages
contain quotes or other special characters.
The root cause was that fmt.Sprintf was used to construct JSON, which doesn't
escape special characters like quotes, newlines, or tabs. When Kubernetes error
messages contain image names with quotes (e.g., "nginx:1.a.b.c"), the resulting
JSON was malformed and failed to parse.
The fix uses json.Marshal to properly construct the JSON payload, which
automatically handles all special character escaping according to JSON spec.
Fixes#1556
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
Co-authored-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
* refactor: improve MCP server handlers with better error handling and pagination
This PR refactors the MCP server handler functions to improve code quality,
maintainability, and user experience.
## Key Improvements
### 1. Eliminated Code Duplication
- Introduced a **resource registry pattern** that maps resource types to their
list and get functions
- Reduced ~500 lines of repetitive switch-case statements to ~100 lines of
declarative registry configuration
- Makes adding new resource types trivial (just add to the registry)
### 2. Proper Error Handling
- Fixed all ignored JSON marshaling errors (previously using `_`)
- Added `marshalJSON()` helper function with explicit error handling
- Improved error messages with context about what failed
### 3. Input Validation
- Added required field validation (resourceType, name, namespace where needed)
- Returns clear error messages when required fields are missing
- Validates resource types before attempting operations
### 4. Pagination Support
- Added `limit` parameter to `list-resources` handler
- Defaults to 100 items, max 1000 (configurable via constants)
- Prevents returning massive amounts of data that could overwhelm clients
- Consistent with `list-events` handler which already had limits
### 5. Resource Type Normalization
- Added `normalizeResourceType()` function to handle aliases (pods->pod, svc->service, etc.)
- Centralized resource type validation
- Better error messages listing supported resource types
### 6. Improved Filter Management
- Added validation to ensure filters array is not empty
- Better feedback messages (e.g., "filters already active", "no filters removed")
- Tracks which filters were actually added/removed
## Technical Details
**Constants Added:**
- `DefaultListLimit = 100` - Default max resources to return
- `MaxListLimit = 1000` - Hard limit for list operations
**New Functions:**
- `normalizeResourceType()` - Converts aliases to canonical types
- `marshalJSON()` - Marshals with proper error handling
**Registry Pattern:**
- `resourceRegistry` - Maps resource types to list/get functions
- `resourceTypeAliases` - Maps aliases to canonical types
## Backward Compatibility
All changes are backward compatible:
- No API changes to tool signatures
- Existing clients will work without modification
- New `limit` parameter is optional (defaults to 100)
## Testing
Tested with:
- All resource types (pods, deployments, services, nodes, etc.)
- Various aliases (svc, cm, pvc, sts, ds, rs)
- Edge cases (missing required fields, invalid resource types)
- Large result sets (pagination working correctly)
Fixes code duplication and improves maintainability of the MCP server.
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
* fix: remove duplicate mcp_handlers_old.go file causing build failures
The old handlers file was accidentally left in place after refactoring,
causing 'redeclared' errors for all handler methods. This commit removes
the old file to resolve the build failures.
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
---------
Signed-off-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
Co-authored-by: Three Foxes (in a Trenchcoat) <threefoxes53235@gmail.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
Add Groq as a new AI backend provider. Groq provides an OpenAI-compatible
API, so this implementation reuses the existing OpenAI client library
with Groq's API endpoint.
Closes#1269🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: majiayu000 <1835304752@qq.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
* migrated to more actively maintained mcp golang lib and added AI explain support for mcp mode
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
* added a makefile option to create local docker image for testing
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
* fixed linter errors and made anonymize as an arg
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
* added mcp support for helm chart and fixed google adk support issue
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
---------
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
* migrated to more actively maintained mcp golang lib and added AI explain support for mcp mode
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
* added a makefile option to create local docker image for testing
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
* fixed linter errors and made anonymize as an arg
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
---------
Signed-off-by: Umesh Kaul <umeshkaul@gmail.com>
Co-authored-by: Alex Jones <alexsimonjones@gmail.com>
