tftsr-devops_investigation/TICKET-kube-pr-review-fixes.md
Shaun Arman 8b227c1837
Some checks failed
Test / frontend-tests (pull_request) Successful in 1m26s
Test / frontend-typecheck (pull_request) Successful in 1m35s
PR Review Automation / review (pull_request) Successful in 5m6s
Test / rust-fmt-check (pull_request) Failing after 11m23s
Test / rust-clippy (pull_request) Successful in 13m2s
Test / rust-tests (pull_request) Successful in 14m47s
fix(kube): resolve automated PR review blockers and warnings
Blockers:
- Replace serde_yaml::from_str with serde_json::from_str in all 6
  parse_*_json functions (parse_namespaces, parse_pods, parse_services,
  parse_deployments, parse_statefulsets, parse_daemonsets). Update
  .as_sequence() → .as_array(), .as_mapping() → .as_object(), and
  mapping iterator patterns throughout. Explicitly type serde_yaml::Value
  in extract_context/extract_server_url which legitimately parse YAML.

Warnings:
- Add containers: Vec<String> to PodInfo struct; parse from
  spec.containers[].name in parse_pods_json
- Fix PodList.tsx to use selectedPod.containers instead of [selectedPod.name]
- Fix exec_pod: add optional shell param with allowlist validation
  (sh/bash/ash/dash); correct arg ordering — -c container now placed
  before -- separator
- Handle empty namespace with --all-namespaces in all 5 list commands
- Fix dialog overflow: overflow-hidden → overflow-y-auto on inner div
- Memoize namespace options with useMemo in ResourceBrowser

Lint cleanup (all pre-existing, surfaced by eslint config fix):
- Deduplicate eslint.config.js (was doubled to 272 lines); move ignores
  to standalone global object; allow console.log in cli section
- Remove stale .eslintignore (migrated to eslint.config.js)
- Remove unused Card/CardTitle imports from Kubernetes list components
- Rename unused props to _clusterId/_namespace in DaemonSetList,
  ServiceList, StatefulSetList
- Fix useEffect/useCallback missing deps in Triage and LogUpload
- Remove debug console.log from App.tsx provider auto-test
- Rename unused hover prop to _hover in TableRow (ui/index.tsx)
- Add #[allow(unused_variables)] to Phase 3 stub Tauri commands
- Restore get_pod_logs, scale_deployment, restart_deployment,
  delete_resource, exec_pod to lib.rs handler registration (were
  accidentally dropped in Phase 3 expansion)

All checks pass: cargo clippy -D warnings, tsc --noEmit,
eslint --max-warnings 0, 331 Rust tests, 98 frontend tests.
2026-06-06 23:55:44 -05:00

6.2 KiB

Kubernetes UI PR Review Fixes

Description

Resolved all findings from the automated PR review (qwen3-coder-next) of the Kubernetes resource discovery and management feature. The review identified two blockers and several warnings across Rust backend and React frontend.

Root cause of blockers: All six JSON parsing functions in kube.rs imported and used serde_yaml::Value / serde_yaml::from_str against kubectl's JSON output (-o json), causing parse failures or incorrect data at runtime. YAML is a superset of JSON and sometimes parses silently incorrectly; the correct parser is serde_json.

Secondary issues: PodInfo lacked container name data, so the log viewer could only show the pod name as the container selector. The exec_pod command had an incorrect kubectl argument order (container -c flag placed after --, so it was passed to the shell inside the pod rather than to kubectl). The "All Namespaces" filter passed an empty string to kubectl -n "" which is invalid.


Acceptance Criteria

  • All six parse_*_json functions use serde_json::from_str and serde_json::Value API (as_array, as_object)
  • PodInfo struct carries containers: Vec<String>; container names parsed from spec.containers[*].name
  • PodList.tsx container selector populates from selectedPod.containers
  • exec_pod container -c flag is placed before -- separator (correct kubectl syntax)
  • exec_pod accepts optional shell parameter with allowlist validation (sh, bash, ash, dash)
  • Empty namespace string routes to --all-namespaces in all five list commands
  • Dialog inner div uses overflow-y-auto to handle content overflow on small screens
  • getNamespaceOptions memoized with useMemo
  • eslint.config.js deduplicated (was 272 lines, duplicate blocks removed), global ignore fixed
  • Unused imports removed from all Kubernetes list components
  • cargo clippy -- -D warnings: zero warnings
  • tsc --noEmit: zero errors
  • eslint . --max-warnings 0: zero warnings
  • 331 Rust tests passing, 98 frontend tests passing

Work Implemented

src-tauri/src/commands/kube.rs

  • Replaced use serde_yaml::Value with use serde_json::Value
  • extract_context and extract_server_url: explicitly typed as serde_yaml::Value (these legitimately parse YAML kubeconfig files)
  • PodInfo struct: added containers: Vec<String> field
  • parse_pods_json: switched to serde_json::from_str, as_array(); added container name extraction from spec.containers[].name
  • parse_namespaces_json, parse_services_json, parse_deployments_json, parse_statefulsets_json, parse_daemonsets_json: switched to serde_json::from_str, as_array(), as_object(); updated mapping iterators (serde_json object keys are String, not Value)
  • parse_services_json: fixed .as_sequence().as_array() in external_ip ingress chain
  • list_pods, list_services, list_deployments, list_statefulsets, list_daemonsets: handle empty namespace with --all-namespaces
  • exec_pod: added optional shell: Option<String> parameter; allowlist validates against ["sh","bash","ash","dash","/bin/sh","/bin/bash","/bin/ash","/bin/dash"]; fixed argument order so -c container appears before --
  • Phase 3 stub commands: added #[allow(unused_variables)] to suppress Clippy warnings on unimplemented stubs

src/lib/tauriCommands.ts

  • PodInfo interface: added containers: string[]
  • execPodCmd: added optional shell?: string parameter, passed through to IPC

src/components/Kubernetes/PodList.tsx

  • Fixed: const containers = selectedPod ? [selectedPod.name] : []selectedPod?.containers ?? []
  • Fixed: overflow-hiddenoverflow-y-auto on inner dialog content div
  • Removed unused imports: Card, CardContent, CardHeader, CardTitle

src/components/Kubernetes/ResourceBrowser.tsx

  • Added useCallback import; wrapped loadData in useCallback([clusterId, selectedNamespace])
  • useEffect deps updated to [loadData, resourceType]
  • Removed unused CardTitle import
  • getNamespaceOptions converted to memoized namespaceOptions via useMemo

src/components/Kubernetes/DaemonSetList.tsx, ServiceList.tsx, StatefulSetList.tsx

  • Removed unused Card, CardContent, CardHeader, CardTitle imports
  • Renamed unused props: clusterId: _clusterId, namespace: _namespace

src/components/Kubernetes/DeploymentList.tsx

  • Removed unused Card, CardContent, CardHeader, CardTitle imports

src/components/ui/index.tsx

  • TableRow: renamed unused hover prop to _hover

src/App.tsx

  • Removed two debug console.log calls (auto-testing provider connection)

src/pages/Triage/index.tsx

  • useEffect: added addMessage, setActiveDomain, startSession to dependency array (stable Zustand store actions)

src/pages/LogUpload/index.tsx

  • handleImagesUpload: wrapped in useCallback([id]) and moved before handleImageDrop to resolve declaration-order issue
  • handleImageDrop: updated deps from [id] to [handleImagesUpload]

eslint.config.js

  • Removed duplicate config block (file was doubled to 272 lines)
  • Fixed global ignore: moved ignores array to a standalone config object (was incorrectly paired with files)
  • CLI section: added "log" to allowed console methods (CLI tool output)

.eslintignore

  • Deleted — content migrated to eslint.config.js global ignore

Testing Needed

  • Connect a real kubeconfig and verify pod/namespace/service/deployment/statefulset/daemonset lists render correctly with JSON from kubectl
  • Select "All Namespaces" — verify --all-namespaces is used and resources from all namespaces appear
  • Open pod log dialog — verify container dropdown shows actual container names (not pod name)
  • Fetch logs for a multi-container pod — verify correct container logs are returned
  • Test exec_pod via UI with sh (default) and bash — verify both work
  • Test exec_pod with an invalid shell name (e.g., zsh) — verify it returns an error
  • Verify "All Namespaces" view does not trigger empty-namespace kubectl error
  • Smoke test triage and log upload flows to verify useEffect/useCallback hook changes have no regressions