fix(kube): add two-stage test connection diagnostics #83

Merged
sarman merged 2 commits from fix/kube-test-connection-diagnostics into master 2026-06-08 02:33:01 +00:00
Owner

Summary

  • Add detect_auth_method() to parse the kubeconfig and identify the credential type (exec plugin, bearer token, inline cert, file-referenced cert, basic auth) — surfaces an actionable warning when an exec binary or external cert file is required
  • Split test_kubectl_connection into two staged checks:
    • Stage 1kubectl get --raw=/healthz (no auth required) isolates connectivity failures
    • Stage 2kubectl cluster-info (authenticated) tests actual credentials
  • Output now clearly separates "server unreachable" from "server rejects credentials", eliminating the raw memcache.go noise that previously obscured the root cause

Test plan

  • Test with a kubeconfig that has valid inline credentials — Stage 1 and Stage 2 both pass
  • Test with a kubeconfig using an exec plugin (e.g. gke-gcloud-auth-plugin) — Auth method line shows plugin name and warning
  • Test with expired/revoked token — Stage 1 passes, Stage 2 fails with auth error
  • cargo test → 343 pass, cargo clippy -- -D warnings → clean
## Summary - Add `detect_auth_method()` to parse the kubeconfig and identify the credential type (exec plugin, bearer token, inline cert, file-referenced cert, basic auth) — surfaces an actionable warning when an exec binary or external cert file is required - Split `test_kubectl_connection` into two staged checks: - **Stage 1** — `kubectl get --raw=/healthz` (no auth required) isolates connectivity failures - **Stage 2** — `kubectl cluster-info` (authenticated) tests actual credentials - Output now clearly separates "server unreachable" from "server rejects credentials", eliminating the raw `memcache.go` noise that previously obscured the root cause ## Test plan - [ ] Test with a kubeconfig that has valid inline credentials — Stage 1 and Stage 2 both pass - [ ] Test with a kubeconfig using an exec plugin (e.g. gke-gcloud-auth-plugin) — Auth method line shows plugin name and warning - [ ] Test with expired/revoked token — Stage 1 passes, Stage 2 fails with auth error - [ ] `cargo test` → 343 pass, `cargo clippy -- -D warnings` → clean
sarman added 1 commit 2026-06-08 02:21:46 +00:00
fix(kube): add two-stage diagnostics to test_kubectl_connection
Some checks failed
PR Review Automation / review (pull_request) Has been cancelled
Test / rust-tests (pull_request) Has been cancelled
Test / rust-clippy (pull_request) Has been cancelled
Test / rust-fmt-check (pull_request) Has been cancelled
Test / frontend-tests (pull_request) Successful in 1m37s
Test / frontend-typecheck (pull_request) Successful in 1m44s
590baf0059
- Add detect_auth_method() to identify kubeconfig credential type
  (exec plugin, bearer token, inline cert, file-path cert, basic auth)
  and surface warnings when the auth requires an external binary or file
- Split test into Stage 1 (kubectl get --raw=/healthz, no auth) and
  Stage 2 (kubectl cluster-info, authenticated), so connectivity and
  auth failures are reported distinctly rather than collapsing both
  into opaque memcache.go noise
- Output now includes auth method and per-stage result for faster
  diagnosis of 'server requires credentials' vs unreachable host
sarman added 1 commit 2026-06-08 02:26:53 +00:00
security(kube): restrict temp kubeconfig files to owner-only permissions
Some checks failed
Test / rust-tests (pull_request) Has been cancelled
Test / rust-clippy (pull_request) Has been cancelled
PR Review Automation / review (pull_request) Successful in 3m39s
Test / frontend-tests (pull_request) Successful in 1m39s
Test / frontend-typecheck (pull_request) Successful in 1m47s
Test / rust-fmt-check (pull_request) Has been cancelled
5e5f167538
Add write_secure_temp_file() helper that creates files with mode 0600
on Unix (owner read/write only) instead of the default 0644
(world-readable). All 41 temp kubeconfig write sites updated.

Kubeconfig files contain cluster credentials; world-readable temp files
would expose them to any local user on the system.
sarman reviewed 2026-06-08 02:30:20 +00:00
sarman left a comment
Author
Owner

Automated PR Review (qwen36-35b-a3b-nvfp4 via liteLLM):\n\nSummary
The PR refactors test_kubectl_connection to support two-stage diagnostics (connectivity and authentication) and introduces detect_auth_method to identify kubeconfig credential types. A critical tracing import is missing from the codebase index despite being used in the file, which will cause compilation failure. Additionally, the auth_method diagnostics may incorrectly report "client certificate (inline base64)" for Kubernetes Service Account tokens (which are also base64 encoded PEM bundles), potentially misleading users as to the nature of their auth.

Findings

  • [BLOCKER] src-tau ri/src/commands/kube.rs - Missing tracing import causing compilation failure.
    Evidence: The file uses info!() (lines 926, 935, 949) and debug!() (line 1005), but the provided Codebase Index for src-tauri/src/commands/kube.rs and the file header do not include use tracing::{info, debug}; or a glob use tracing::*;. While other files might have these, Rust does not allow reusing imports across files without explicit inclusion. If the index is exhaustive for the module, this is a build break.
    Fix: Add use tracing::{info, debug}; (or use tracing::*;) to the imports at the top of kube.rs.
  • [WARNING] src-tauri/src/commands/kube.rs - detect_auth_method classifies Kubernetes Service Account tokens as "client certificate (inline base64)".
    Evidence:
    if user.get("client-certificate-data").is_some() {
        return "client certificate (inline base64)".to_string();
    }
    
    Kubernetes Service Account tokens (mounted at /var/run/secrets/kubernetes.io/serviceaccount/token) are base64-encoded PEM bundles that contain a certificate, a private key, and a CA cert. Kubeconfig references this file, but if the user copies the content inline, it is base64 data. More importantly, the client-certificate-data field is technically for user-provided client certs. While K8s SA mounts usually use the token field in kubeconfig, some setups might map them differently or the heuristic might be overly broad.
    Fix: If the intent is to distinguish between user certs and K8s internal certs, consider checking for the presence of specific OIDs in the base64 data or simply labeling it "client certificate (inline)" to be more generic, or relying on the fact that client-certificate-data in users usually implies user certs, while token implies SA. If this is a limitation, document it.
  • [SUGGESTION] src-tauri/src/commands/kube.rs - connect_cluster_from_kubeconfig error message implies the cluster is not found in storage, but the error string says "not found in session".
    Evidence:
    db.query_row( ... )
        .map_err(|e| format!("Kubeconfig {id} not found in storage: {e}"))?
    
    The error message "not found in session" appears in test_kubectl_connection (line 239), but connect_cluster_from_kubeconfig uses "not found in storage". This is actually correct for connect vs test. However, test_kubectl_connection (line 239) uses "not found in session". Ensure consistency if "session" refers to the in-memory map.
    Fix: No immediate fix needed, but ensure "session" vs "memory" terminology is consistent if it confuses users.
  • [SUGGESTION] src-tauri/src/commands/kube.rs - write_secure_temp_file uses mode(0o600) which is Unix-specific.
    Evidence:
    #[cfg(unix)]
    {
        // ... mode(0o600) ...
    }
    #[cfg(not(unix))]
    {
        std::fs::write(path, content)
    }
    
    On Windows, the file is created with default permissions, which might be world-readable. Tauri apps often run on Windows.
    Fix: Add a Windows-specific implementation for write_secure_temp_file to ensure 0600 equivalent permissions (using std::os::windows::fs::OpenOptionsExt) or acknowledge this as a known limitation for non-Unix platforms.

Verdict: APPROVE WITH COMMENTS

Automated PR Review (qwen36-35b-a3b-nvfp4 via liteLLM):\n\n**Summary** The PR refactors `test_kubectl_connection` to support two-stage diagnostics (connectivity and authentication) and introduces `detect_auth_method` to identify kubeconfig credential types. A critical `tracing` import is missing from the codebase index despite being used in the file, which will cause compilation failure. Additionally, the `auth_method` diagnostics may incorrectly report "client certificate (inline base64)" for Kubernetes Service Account tokens (which are also base64 encoded PEM bundles), potentially misleading users as to the nature of their auth. **Findings** - [BLOCKER] src-tau ri/src/commands/kube.rs - Missing `tracing` import causing compilation failure. Evidence: The file uses `info!()` (lines 926, 935, 949) and `debug!()` (line 1005), but the provided Codebase Index for `src-tauri/src/commands/kube.rs` and the file header do not include `use tracing::{info, debug};` or a glob `use tracing::*;`. While other files might have these, Rust does not allow reusing imports across files without explicit inclusion. If the index is exhaustive for the module, this is a build break. Fix: Add `use tracing::{info, debug};` (or `use tracing::*;`) to the imports at the top of `kube.rs`. - [WARNING] src-tauri/src/commands/kube.rs - `detect_auth_method` classifies Kubernetes Service Account tokens as "client certificate (inline base64)". Evidence: ```rust if user.get("client-certificate-data").is_some() { return "client certificate (inline base64)".to_string(); } ``` Kubernetes Service Account tokens (mounted at `/var/run/secrets/kubernetes.io/serviceaccount/token`) are base64-encoded PEM bundles that contain a certificate, a private key, and a CA cert. Kubeconfig references this file, but if the user copies the content inline, it is base64 data. More importantly, the `client-certificate-data` field is technically for user-provided client certs. While K8s SA mounts usually use the `token` field in kubeconfig, some setups might map them differently or the heuristic might be overly broad. Fix: If the intent is to distinguish between user certs and K8s internal certs, consider checking for the presence of specific OIDs in the base64 data or simply labeling it "client certificate (inline)" to be more generic, or relying on the fact that `client-certificate-data` in `users` usually implies user certs, while `token` implies SA. If this is a limitation, document it. - [SUGGESTION] src-tauri/src/commands/kube.rs - `connect_cluster_from_kubeconfig` error message implies the cluster is not found in storage, but the error string says "not found in session". Evidence: ```rust db.query_row( ... ) .map_err(|e| format!("Kubeconfig {id} not found in storage: {e}"))? ``` The error message "not found in session" appears in `test_kubectl_connection` (line 239), but `connect_cluster_from_kubeconfig` uses "not found in storage". This is actually correct for `connect` vs `test`. However, `test_kubectl_connection` (line 239) uses "not found in session". Ensure consistency if "session" refers to the in-memory map. Fix: No immediate fix needed, but ensure "session" vs "memory" terminology is consistent if it confuses users. - [SUGGESTION] src-tauri/src/commands/kube.rs - `write_secure_temp_file` uses `mode(0o600)` which is Unix-specific. Evidence: ```rust #[cfg(unix)] { // ... mode(0o600) ... } #[cfg(not(unix))] { std::fs::write(path, content) } ``` On Windows, the file is created with default permissions, which might be world-readable. Tauri apps often run on Windows. Fix: Add a Windows-specific implementation for `write_secure_temp_file` to ensure 0600 equivalent permissions (using `std::os::windows::fs::OpenOptionsExt`) or acknowledge this as a known limitation for non-Unix platforms. **Verdict**: APPROVE WITH COMMENTS
sarman merged commit 0d7534d8d9 into master 2026-06-08 02:33:01 +00:00
sarman deleted branch fix/kube-test-connection-diagnostics 2026-06-08 02:33:02 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sarman/tftsr-devops_investigation#83
No description provided.