From 40c9e45ce17d510c0f903f002db92cf43b714c71 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Sun, 25 Jan 2026 15:35:21 +0200 Subject: [PATCH 01/16] Add remote session cli commands --- internal/cli/local/local.go | 5 + internal/cli/local/remote/copy_recording.go | 128 ++++++++ internal/cli/local/remote/list.go | 155 ++++++++++ internal/cli/local/remote/list_recordings.go | 251 ++++++++++++++++ internal/cli/local/remote/remote.go | 34 +++ internal/cli/local/remote/start.go | 298 +++++++++++++++++++ internal/cli/local/remote/stop.go | 101 +++++++ internal/local/chart/kube.go | 73 ++++- internal/utils/http.go | 80 +++++ 9 files changed, 1124 insertions(+), 1 deletion(-) create mode 100644 internal/cli/local/remote/copy_recording.go create mode 100644 internal/cli/local/remote/list.go create mode 100644 internal/cli/local/remote/list_recordings.go create mode 100644 internal/cli/local/remote/remote.go create mode 100644 internal/cli/local/remote/start.go create mode 100644 internal/cli/local/remote/stop.go create mode 100644 internal/utils/http.go diff --git a/internal/cli/local/local.go b/internal/cli/local/local.go index f8872de..fb0024a 100644 --- a/internal/cli/local/local.go +++ b/internal/cli/local/local.go @@ -6,6 +6,7 @@ import ( "github.com/weka/gohomecli/internal/cli/app/hooks" "github.com/weka/gohomecli/internal/cli/local/cleanup" "github.com/weka/gohomecli/internal/cli/local/debug" + "github.com/weka/gohomecli/internal/cli/local/remote" "github.com/weka/gohomecli/internal/cli/local/setup" "github.com/weka/gohomecli/internal/cli/local/status" "github.com/weka/gohomecli/internal/cli/local/upgrade" @@ -41,6 +42,10 @@ func init() { setup.Cli.InitCobra(localCmd) upgrade.Cli.InitCobra(localCmd) cleanup.Cli.InitCobra(localCmd) + + remoteCli := remote.CliHook() + remoteCli.InitCobra(localCmd) + statusCli := status.CliHook() statusCli.InitCobra(localCmd) }) diff --git a/internal/cli/local/remote/copy_recording.go b/internal/cli/local/remote/copy_recording.go new file mode 100644 index 0000000..2718eec --- /dev/null +++ b/internal/cli/local/remote/copy_recording.go @@ -0,0 +1,128 @@ +package remote + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/spf13/cobra" + + "github.com/weka/gohomecli/internal/local/chart" + "github.com/weka/gohomecli/internal/utils" +) + +type copyRecordingOptions struct { + recording string + clusterID string + all bool + output string +} + +func newCopyRecordingCmd() *cobra.Command { + opts := ©RecordingOptions{} + + cmd := &cobra.Command{ + Use: "copy-recording", + Short: "Copy recording files from PVC to local filesystem", + Long: `Copy session recording files from the recordings PVC to a local directory. + +Recordings can be copied individually or in bulk by cluster ID or all at once. + +Examples: + homecli local remote copy-recording --recording "2024-01-15T10:30:00-abc123.cast" --output /tmp/ + homecli local remote copy-recording --cluster-id "550e8400-e29b-41d4-a716-446655440000" --output /tmp/ + homecli local remote copy-recording --all --output /tmp/ +`, + RunE: func(cmd *cobra.Command, args []string) error { + return copyRecordingRun(cmd, opts) + }, + PreRunE: func(cmd *cobra.Command, args []string) error { + if opts.recording == "" && opts.clusterID == "" && !opts.all { + return fmt.Errorf("must specify one of: --recording, --cluster-id, or --all") + } + if opts.output == "" { + return fmt.Errorf("--output is required") + } + return nil + }, + } + + cmd.Flags().StringVar(&opts.recording, "recording", "", "Specific recording filename to copy") + cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Copy all recordings for a cluster") + cmd.Flags().BoolVar(&opts.all, "all", false, "Copy all recordings") + cmd.Flags().StringVarP(&opts.output, "output", "o", "", "Local destination directory (required)") + _ = cmd.MarkFlagRequired("output") + + return cmd +} + +func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { + ctx := cmd.Context() + + // Ensure output directory exists + if err := os.MkdirAll(opts.output, 0o755); err != nil { + return fmt.Errorf("failed to create output directory: %w", err) + } + + baseURL, err := chart.GetServiceURL(ctx, sessionRecordingsServerLabel) + if err != nil { + return fmt.Errorf("failed to find recordings server: %w", err) + } + + // Determine which files to copy + var filesToCopy []RecordingInfo + + if opts.recording != "" { + // Specific file + filesToCopy = []RecordingInfo{{ + Filename: opts.recording, + ClusterID: opts.clusterID, + }} + } else { + var err error + if opts.clusterID != "" { + filesToCopy, err = listRecordingsForCluster(ctx, baseURL, opts.clusterID) + } else { + // --all case: copy everything when no specific filters provided + filesToCopy, err = listAllRecordings(ctx, baseURL) + } + if err != nil { + return fmt.Errorf("failed to list recordings: %w", err) + } + } + + if len(filesToCopy) == 0 { + utils.UserNote("No matching recordings found") + return nil + } + + utils.UserOutput("Copying %d recording(s) to %s\n", len(filesToCopy), opts.output) + + // Copy each file + copiedCount := 0 + for _, recording := range filesToCopy { + // Construct remote path from ClusterID + Filename + remotePath := recording.Filename + if recording.ClusterID != "" { + remotePath = filepath.Join(recording.ClusterID, recording.Filename) + } + + dstPath := filepath.Join(opts.output, recording.Filename) + + logger.Debug().Str("src", remotePath).Str("dst", dstPath).Msg("Copying file") + + httpClient := utils.NewHTTPClient() + url := fmt.Sprintf("%s/%s", baseURL, remotePath) + err := httpClient.DownloadFile(ctx, url, dstPath, fileDownloadTimeout) + if err != nil { + utils.UserWarning("Failed to copy %s: %v", recording.Filename, err) + continue + } + + utils.UserOutput(" Copied: %s\n", recording.Filename) + copiedCount++ + } + + utils.UserNote("Successfully copied %d/%d recording(s)", copiedCount, len(filesToCopy)) + return nil +} diff --git a/internal/cli/local/remote/list.go b/internal/cli/local/remote/list.go new file mode 100644 index 0000000..5d7a103 --- /dev/null +++ b/internal/cli/local/remote/list.go @@ -0,0 +1,155 @@ +package remote + +import ( + "encoding/json" + "fmt" + "time" + + "github.com/spf13/cobra" + "gopkg.in/yaml.v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/weka/gohomecli/internal/local/chart" + "github.com/weka/gohomecli/internal/utils" +) + +type listOptions struct { + outputFormat string +} + +func newListCmd() *cobra.Command { + opts := &listOptions{} + + cmd := &cobra.Command{ + Use: "list", + Short: "List active remote sessions", + Long: `List active remote sessions by querying pods with app=remote-session label. + + Examples: + homecli local remote list # List all active sessions in table format + homecli local remote list --output json # List sessions in JSON format + homecli local remote list --output yaml # List sessions in YAML format + `, + RunE: func(cmd *cobra.Command, args []string) error { + return listRun(cmd, opts) + }, + } + + cmd.Flags().StringVarP(&opts.outputFormat, "output", "o", "table", "Output format: table, json, yaml") + + return cmd +} + +// SessionInfo represents information about an active remote session +type SessionInfo struct { + SessionID string `json:"sessionId" yaml:"sessionId"` + ClusterID string `json:"clusterId" yaml:"clusterId"` + ClusterName string `json:"clusterName" yaml:"clusterName"` + Duration string `json:"duration" yaml:"duration"` +} + +func listRun(cmd *cobra.Command, opts *listOptions) error { + ctx := cmd.Context() + + clientset, err := getKubernetesClient() + if err != nil { + return fmt.Errorf("failed to create Kubernetes client: %w", err) + } + + // List pods with app=remote-session label + selector := fmt.Sprintf("%s=%s", remoteAccessLabel, remoteAccessValue) + pods, err := clientset.CoreV1().Pods(chart.ReleaseNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: selector, + }) + if err != nil { + return fmt.Errorf("failed to list pods: %w", err) + } + + if len(pods.Items) == 0 { + utils.UserNote("No active remote sessions") + return nil + } + + // Build session info list + sessions := make([]SessionInfo, 0, len(pods.Items)) + for _, pod := range pods.Items { + duration := formatDuration(pod.CreationTimestamp.Time) + sessions = append(sessions, SessionInfo{ + SessionID: pod.Labels["session-id"], + ClusterID: pod.Labels["cluster-id"], + ClusterName: pod.Labels["cluster-name"], + Duration: duration, + }) + } + + // Output based on format + switch opts.outputFormat { + case "json": + return outputSessionsAsJSON(sessions) + case "yaml": + return outputSessionsAsYAML(sessions) + default: + return outputSessionsAsTable(sessions) + } +} + +func formatDuration(creationTime time.Time) string { + duration := time.Since(creationTime) + + if duration < time.Minute { + return fmt.Sprintf("%ds", int(duration.Seconds())) + } + if duration < time.Hour { + return fmt.Sprintf("%dm", int(duration.Minutes())) + } + if duration < 24*time.Hour { + hours := int(duration.Hours()) + minutes := int(duration.Minutes()) % 60 + return fmt.Sprintf("%dh%dm", hours, minutes) + } + days := int(duration.Hours()) / 24 + hours := int(duration.Hours()) % 24 + return fmt.Sprintf("%dd%dh", days, hours) +} + +func outputSessionsAsJSON(sessions []SessionInfo) error { + output, err := json.MarshalIndent(sessions, "", " ") + if err != nil { + return err + } + utils.UserOutputJSON(output) + return nil +} + +func outputSessionsAsYAML(sessions []SessionInfo) error { + output, err := yaml.Marshal(sessions) + if err != nil { + return err + } + fmt.Println(string(output)) + return nil +} + +func outputSessionsAsTable(sessions []SessionInfo) error { + headers := []string{"SESSION", "CLUSTER ID", "CLUSTER NAME", "DURATION"} + index := 0 + utils.RenderTableRows(headers, func() []string { + if index < len(sessions) { + s := sessions[index] + index++ + // Truncate cluster ID for display + clusterIDDisplay := s.ClusterID + if len(clusterIDDisplay) > 36 { + clusterIDDisplay = clusterIDDisplay[:36] + } + return []string{ + s.SessionID, + clusterIDDisplay, + s.ClusterName, + s.Duration, + } + } + return nil + }) + return nil +} diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go new file mode 100644 index 0000000..0fb6a01 --- /dev/null +++ b/internal/cli/local/remote/list_recordings.go @@ -0,0 +1,251 @@ +package remote + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "time" + + "github.com/spf13/cobra" + "gopkg.in/yaml.v3" + + "github.com/weka/gohomecli/internal/local/chart" + "github.com/weka/gohomecli/internal/utils" +) + +const ( + sessionRecordingsServerLabel = "app=remote-access-recordings-server" + httpRequestTimeout = 10 * time.Second + fileDownloadTimeout = 5 * time.Minute + + // dufs path_type values + dufsPathTypeFile = "File" + dufsPathTypeDir = "Dir" +) + +type listRecordingsOptions struct { + clusterID string + outputFormat string +} + +func newListRecordingsCmd() *cobra.Command { + opts := &listRecordingsOptions{} + + cmd := &cobra.Command{ + Use: "list-recordings", + Short: "List available session recordings", + Long: `List available session recordings stored in the recordings PVC. + +Recordings are stored as asciinema .cast files and can be filtered by cluster ID. + +Examples: + homecli local remote list-recordings + homecli local remote list-recordings --cluster-id 550e8400-e29b-41d4-a716-446655440000 + homecli local remote list-recordings --output json +`, + RunE: func(cmd *cobra.Command, args []string) error { + return listRecordingsRun(cmd, opts) + }, + } + + cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Filter by cluster ID") + cmd.Flags().StringVarP(&opts.outputFormat, "output", "o", "table", "Output format: table, json, yaml") + + return cmd +} + +// RecordingInfo represents information about a recording file +type RecordingInfo struct { + Filename string `json:"filename" yaml:"filename"` + Size int64 `json:"size" yaml:"size"` + ModTime int64 `json:"modTime" yaml:"modTime"` + ClusterID string `json:"clusterId,omitempty" yaml:"clusterId,omitempty"` +} + +// dufsResponse represents the JSON response from dufs directory listing +type dufsResponse struct { + Paths []dufsEntry `json:"paths"` +} + +// dufsEntry represents a file/directory entry from dufs JSON response +type dufsEntry struct { + Name string `json:"name"` + PathType string `json:"path_type"` // "File" or "Dir" + Size int64 `json:"size"` + Mtime int64 `json:"mtime"` // milliseconds since epoch +} + +func listRecordingsRun(cmd *cobra.Command, opts *listRecordingsOptions) error { + ctx := cmd.Context() + + baseURL, err := chart.GetServiceURL(ctx, sessionRecordingsServerLabel) + if err != nil { + return fmt.Errorf("failed to find recordings server: %w", err) + } + + var recordings []RecordingInfo + + if opts.clusterID != "" { + recordings, err = listRecordingsForCluster(ctx, baseURL, opts.clusterID) + } else { + recordings, err = listAllRecordings(ctx, baseURL) + } + if err != nil { + return err + } + + if len(recordings) == 0 { + utils.UserNote("No recordings found") + return nil + } + + // Output based on format + switch opts.outputFormat { + case "json": + return outputRecordingsAsJSON(recordings) + case "yaml": + return outputRecordingsAsYAML(recordings) + default: + return outputRecordingsAsTable(recordings) + } +} + +// listRecordingsForCluster lists recordings for a specific cluster ID +func listRecordingsForCluster(ctx context.Context, baseURL, clusterID string) ([]RecordingInfo, error) { + url := fmt.Sprintf("%s/%s?json", baseURL, clusterID) + entries, err := fetchDufsDirectory(ctx, url) + if err != nil { + // Directory might not exist + return nil, nil + } + + return filterAndConvertEntries(entries, clusterID), nil +} + +// listAllRecordings lists all recordings from cluster subdirectories and root level +func listAllRecordings(ctx context.Context, baseURL string) ([]RecordingInfo, error) { + url := fmt.Sprintf("%s?json", baseURL) + entries, err := fetchDufsDirectory(ctx, url) + if err != nil { + return nil, fmt.Errorf("failed to list recordings: %w", err) + } + + var recordings []RecordingInfo + + for _, entry := range entries { + if entry.PathType == dufsPathTypeDir { + // Directory = cluster_id subdirectory + clusterRecordings, _ := listRecordingsForCluster(ctx, baseURL, entry.Name) + recordings = append(recordings, clusterRecordings...) + } else if entry.PathType == dufsPathTypeFile && strings.HasSuffix(entry.Name, ".cast") { + // Root level .cast file (backward compat - no cluster_id) + recordings = append(recordings, RecordingInfo{ + Filename: entry.Name, + Size: entry.Size, + ModTime: entry.Mtime / 1000, // Convert milliseconds to seconds + ClusterID: "", + }) + } + } + + return recordings, nil +} + +// fetchDufsDirectory fetches directory listing from dufs server +func fetchDufsDirectory(ctx context.Context, url string) ([]dufsEntry, error) { + httpClient := utils.NewHTTPClient() + + body, err := httpClient.Get(ctx, url, httpRequestTimeout) + if err != nil { + return nil, err + } + + var response dufsResponse + if err := json.Unmarshal(body, &response); err != nil { + return nil, err + } + + return response.Paths, nil +} + +// filterAndConvertEntries filters entries by .cast extension +func filterAndConvertEntries(entries []dufsEntry, clusterID string) []RecordingInfo { + var recordings []RecordingInfo + + for _, entry := range entries { + if entry.PathType != dufsPathTypeFile || !strings.HasSuffix(entry.Name, ".cast") { + continue + } + + recordings = append(recordings, RecordingInfo{ + Filename: entry.Name, + Size: entry.Size, + ModTime: entry.Mtime / 1000, // Convert milliseconds to seconds + ClusterID: clusterID, + }) + } + + return recordings +} + +func outputRecordingsAsJSON(recordings []RecordingInfo) error { + output, err := json.MarshalIndent(recordings, "", " ") + if err != nil { + return err + } + utils.UserOutputJSON(output) + return nil +} + +func outputRecordingsAsYAML(recordings []RecordingInfo) error { + output, err := yaml.Marshal(recordings) + if err != nil { + return err + } + fmt.Println(string(output)) + return nil +} + +func outputRecordingsAsTable(recordings []RecordingInfo) error { + headers := []string{"FILENAME", "SIZE", "MODIFIED", "CLUSTER ID"} + index := 0 + utils.RenderTableRows(headers, func() []string { + if index < len(recordings) { + r := recordings[index] + index++ + clusterID := r.ClusterID + if clusterID == "" { + clusterID = "-" + } + return []string{ + r.Filename, + formatSize(r.Size), + formatTime(r.ModTime), + clusterID, + } + } + return nil + }) + return nil +} + +// formatSize formats bytes into human-readable format +func formatSize(bytes int64) string { + const unit = 1024 + if bytes < unit { + return fmt.Sprintf("%d B", bytes) + } + div, exp := int64(unit), 0 + for n := bytes / unit; n >= unit; n /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.1f %cB", float64(bytes)/float64(div), "KMGTPE"[exp]) +} + +// formatTime formats unix timestamp into human-readable format +func formatTime(timestamp int64) string { + t := time.Unix(timestamp, 0) + return t.Format("2006-01-02 15:04") +} diff --git a/internal/cli/local/remote/remote.go b/internal/cli/local/remote/remote.go new file mode 100644 index 0000000..357b6e3 --- /dev/null +++ b/internal/cli/local/remote/remote.go @@ -0,0 +1,34 @@ +// Package remote provides CLI commands for managing remote tmate sessions +package remote + +import ( + "github.com/spf13/cobra" + + "github.com/weka/gohomecli/internal/cli/app/hooks" + "github.com/weka/gohomecli/internal/utils" +) + +var logger = utils.GetLogger("Remote") + +// CliHook returns a Cobra CLI hook for the remote command +func CliHook() hooks.Cli { + var cli hooks.Cli + + remoteCmd := &cobra.Command{ + Use: "remote", + Short: "Manage remote tmate sessions", + Long: "Start, stop, and manage remote tmate sessions for cluster debugging", + } + + remoteCmd.AddCommand(newStartCmd()) + remoteCmd.AddCommand(newStopCmd()) + remoteCmd.AddCommand(newListCmd()) + remoteCmd.AddCommand(newListRecordingsCmd()) + remoteCmd.AddCommand(newCopyRecordingCmd()) + + cli.AddHook(func(localCmd *cobra.Command) { + localCmd.AddCommand(remoteCmd) + }) + + return cli +} diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go new file mode 100644 index 0000000..5fafc44 --- /dev/null +++ b/internal/cli/local/remote/start.go @@ -0,0 +1,298 @@ +package remote + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "strings" + + "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/clientcmd" + + "github.com/weka/gohomecli/internal/env" + "github.com/weka/gohomecli/internal/local/chart" + "github.com/weka/gohomecli/internal/utils" +) + +const ( + remoteAccessLabel = "app" + remoteAccessValue = "remote-access" + remoteAccessImage = "public.ecr.aws/weka/weka-remote-access:v1.0.0" + sessionRecordingsPVCLabel = "app=remote-access-recordings" +) + +type startOptions struct { + clusterID string + clusterName string + sshKeysPath string + cloudURL string + hostName string + terminalCols int + terminalLines int + debug bool + // Tmate server flags (optional overrides - tmate.py has built-in config for known cloud URLs) + tmateServerHost string + tmateServerPort string + tmateServerRSA string + tmateServerEd25519 string + tmateServerECDSA string +} + +func newStartCmd() *cobra.Command { + opts := &startOptions{} + + cmd := &cobra.Command{ + Use: "start", + Short: "Start a new remote-access tmate session", + Long: `Start a remote-access tmate session for remote cluster connection. + Tmate server config is resolved from cloud URL. Use --tmate-server-* flags for custom servers. + + Examples: + homecli local remote start --cluster-id "550e8400-..." --cluster-name "prod" --ssh-keys-path "/root/.ssh" # Start a session with cloud URL from config + `, + RunE: func(cmd *cobra.Command, args []string) error { + return startRun(cmd, opts) + }, + } + + // Required flags + cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Cluster GUID (required)") + cmd.Flags().StringVar(&opts.clusterName, "cluster-name", "", "Human-readable cluster name (required)") + cmd.Flags().StringVar(&opts.sshKeysPath, "ssh-keys-path", "", "Host path to SSH keys directory (required)") + _ = cmd.MarkFlagRequired("cluster-id") + _ = cmd.MarkFlagRequired("cluster-name") + _ = cmd.MarkFlagRequired("ssh-keys-path") + + // Optional flags + cmd.Flags().StringVar(&opts.cloudURL, "cloud-url", "", "Cloud Weka Home URL (default: from config)") + cmd.Flags().StringVar(&opts.hostName, "host-name", "", "Override hostname (default: system hostname)") + cmd.Flags().IntVar(&opts.terminalCols, "terminal-cols", 0, "Terminal width (default: 158)") + cmd.Flags().IntVar(&opts.terminalLines, "terminal-lines", 0, "Terminal height (default: 35)") + cmd.Flags().BoolVar(&opts.debug, "debug", false, "Enable debug logging") + + // Tmate server override flags (optional - tmate.py has built-in config for known cloud URLs) + cmd.Flags().StringVar(&opts.tmateServerHost, "tmate-server-host", "", "Override tmate SSH server hostname") + cmd.Flags().StringVar(&opts.tmateServerPort, "tmate-server-port", "", "Override tmate SSH server port") + cmd.Flags().StringVar(&opts.tmateServerRSA, "tmate-server-rsa-fingerprint", "", "Override RSA fingerprint") + cmd.Flags().StringVar(&opts.tmateServerEd25519, "tmate-server-ed25519-fingerprint", "", "Override Ed25519 fingerprint") + cmd.Flags().StringVar(&opts.tmateServerECDSA, "tmate-server-ecdsa-fingerprint", "", "Override ECDSA fingerprint") + + return cmd +} + +func startRun(cmd *cobra.Command, opts *startOptions) error { + ctx := cmd.Context() + + // Resolve cloud URL from flag or config + cloudURL := opts.cloudURL + if cloudURL == "" && env.CurrentSiteConfig != nil { + cloudURL = env.CurrentSiteConfig.CloudURL + } + if cloudURL == "" { + return fmt.Errorf("cloud URL not found in site config, please provide --cloud-url flag explicitly") + } + + // Get local LWH address from ingress + lwhAddress, err := chart.GetIngressAddress(ctx) + if err != nil { + return fmt.Errorf("failed to get LWH address: %w", err) + } + localLWHURL := "http://" + lwhAddress + + // Build webhook base URLs (tmate.py builds full paths using WEBHOOK_URL_TEMPLATE) + webhookURLs := buildWebhookBaseURLs(cloudURL, localLWHURL) + + // Generate session ID + sessionID := generateShortID() + + // Create Kubernetes client + clientset, err := getKubernetesClient() + if err != nil { + return fmt.Errorf("failed to create Kubernetes client: %w", err) + } + + // Get recordings PVC name + recordingsPVCName, err := chart.GetPVCName(ctx, sessionRecordingsPVCLabel) + if err != nil { + return fmt.Errorf("failed to find recordings PVC: %w", err) + } + + // Create the pod + pod := buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, opts) + + logger.Info(). + Str("sessionID", sessionID). + Str("clusterID", opts.clusterID). + Str("clusterName", opts.clusterName). + Msg("Creating remote session pod...") + + createdPod, err := clientset.CoreV1().Pods(chart.ReleaseNamespace).Create(ctx, pod, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("failed to create session pod: %w", err) + } + + utils.UserOutput("Remote session started successfully\n") + utils.UserOutput(" Session ID: %s\n", sessionID) + + logger.Info().Str("pod", createdPod.Name).Msg("Remote session pod created") + + return nil +} + +func buildWebhookBaseURLs(cloudURL, localAPIURL string) string { + var urls []string + + // Add cloud base URL if configured + if cloudURL != "" { + urls = append(urls, strings.TrimSuffix(cloudURL, "/")) + } + + // Always add local LWH base URL + urls = append(urls, localAPIURL) + + return strings.Join(urls, ",") +} + +func generateShortID() string { + bytes := make([]byte, 3) // 3 bytes = 6 hex chars + _, _ = rand.Read(bytes) + return hex.EncodeToString(bytes) +} + +func getKubernetesClient() (*kubernetes.Clientset, error) { + kubeconfig, err := chart.ReadKubeConfig(chart.KubeConfigPath) + if err != nil { + return nil, err + } + + config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) + if err != nil { + return nil, err + } + + return kubernetes.NewForConfig(config) +} + +func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName string, opts *startOptions) *corev1.Pod { + const sharedSocketPath = "/shared/tmate.socket" + + // Environment variables for tmate container + tmateEnv := []corev1.EnvVar{ + {Name: "CLUSTER_ID", Value: opts.clusterID}, + {Name: "CLUSTER_NAME", Value: opts.clusterName}, + {Name: "SHARED_SOCKET", Value: sharedSocketPath}, + {Name: "CLOUD_URL", Value: cloudURL}, + {Name: "WEBHOOK_URLS", Value: webhookURLs}, + } + + // Add HOST_NAME only if explicitly provided (tmate.py uses socket.gethostname() as default) + if opts.hostName != "" { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "HOST_NAME", Value: opts.hostName}) + } + + // Add terminal dimensions only if explicitly provided (tmate.py defaults to 158x35) + if opts.terminalCols > 0 { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TERMINAL_COLS", Value: fmt.Sprintf("%d", opts.terminalCols)}) + } + if opts.terminalLines > 0 { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TERMINAL_LINES", Value: fmt.Sprintf("%d", opts.terminalLines)}) + } + + // Add tmate server overrides only if provided (tmate.py will use these if CLOUD_URL not in its CONFIG) + if opts.tmateServerHost != "" { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_HOST", Value: opts.tmateServerHost}) + } + if opts.tmateServerPort != "" { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_PORT", Value: opts.tmateServerPort}) + } + if opts.tmateServerRSA != "" { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_RSA_FINGERPRINT", Value: opts.tmateServerRSA}) + } + if opts.tmateServerEd25519 != "" { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_ED25519_FINGERPRINT", Value: opts.tmateServerEd25519}) + } + if opts.tmateServerECDSA != "" { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_ECDSA_FINGERPRINT", Value: opts.tmateServerECDSA}) + } + + if opts.debug { + tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "DEBUG", Value: "1"}) + } + + // Volume mounts for tmate container + tmateMounts := []corev1.VolumeMount{ + {Name: "shared-socket", MountPath: "/shared"}, + {Name: "ssh-keys", MountPath: "/root/.ssh", ReadOnly: true}, + } + + // Recorder environment + recorderEnv := []corev1.EnvVar{ + {Name: "SHARED_SOCKET", Value: sharedSocketPath}, + {Name: "CLUSTER_ID", Value: opts.clusterID}, + } + + // Volume mounts for recorder container + recorderMounts := []corev1.VolumeMount{ + {Name: "shared-socket", MountPath: "/shared"}, + {Name: "recordings", MountPath: "/recordings"}, + } + + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("remote-session-%s", sessionID), + Namespace: chart.ReleaseNamespace, + Labels: map[string]string{ + remoteAccessLabel: remoteAccessValue, + "session-id": sessionID, + "cluster-id": opts.clusterID, + "cluster-name": opts.clusterName, + }, + }, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyAlways, + Containers: []corev1.Container{ + { + Name: "tmate", + Image: remoteAccessImage, + Command: []string{"python3", "/tmate/tmate.py"}, + Env: tmateEnv, + VolumeMounts: tmateMounts, + }, + { + Name: "recorder", + Image: remoteAccessImage, + Command: []string{"python3", "/recorder/recorder.py"}, + Env: recorderEnv, + VolumeMounts: recorderMounts, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "shared-socket", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: "recordings", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: recordingsPVCName, + }, + }, + }, + { + Name: "ssh-keys", + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: opts.sshKeysPath, + }, + }, + }, + }, + }, + } +} diff --git a/internal/cli/local/remote/stop.go b/internal/cli/local/remote/stop.go new file mode 100644 index 0000000..bd50bae --- /dev/null +++ b/internal/cli/local/remote/stop.go @@ -0,0 +1,101 @@ +package remote + +import ( + "fmt" + + "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/weka/gohomecli/internal/local/chart" + "github.com/weka/gohomecli/internal/utils" +) + +type stopOptions struct { + sessionID string + clusterID string + all bool +} + +func newStopCmd() *cobra.Command { + opts := &stopOptions{} + + cmd := &cobra.Command{ + Use: "stop", + Short: "Stop running remote session(s)", + Long: `Stop running remote session(s) by deleting the pod. + + Pod deletion triggers graceful shutdown via SIGTERM, allowing the session + to properly close connections and finalize recordings. + + Examples: + homecli local remote stop --session-id a1b2c3 # Stop a specific session by ID + homecli local remote stop --cluster-id 550e8400-e29b-41d4-a716-446655440000 # Stop all sessions for a cluster + homecli local remote stop --all # Stop all active sessions + `, + RunE: func(cmd *cobra.Command, args []string) error { + return stopRun(cmd, opts) + }, + PreRunE: func(cmd *cobra.Command, args []string) error { + if opts.sessionID == "" && opts.clusterID == "" && !opts.all { + return fmt.Errorf("must specify one of: --session-id, --cluster-id, or --all") + } + return nil + }, + } + + cmd.Flags().StringVar(&opts.sessionID, "session-id", "", "Stop specific session by ID") + cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Stop all sessions for a cluster") + cmd.Flags().BoolVar(&opts.all, "all", false, "Stop all active sessions") + + return cmd +} + +func stopRun(cmd *cobra.Command, opts *stopOptions) error { + ctx := cmd.Context() + + clientset, err := getKubernetesClient() + if err != nil { + return fmt.Errorf("failed to create Kubernetes client: %w", err) + } + + // Build label selector + selector := fmt.Sprintf("%s=%s", remoteAccessLabel, remoteAccessValue) + if opts.sessionID != "" { + selector += ",session-id=" + opts.sessionID + } else if opts.clusterID != "" { + selector += ",cluster-id=" + opts.clusterID + } + // --all: no additional filter + + logger.Debug().Str("selector", selector).Msg("Listing pods to stop") + + // List matching pods + pods, err := clientset.CoreV1().Pods(chart.ReleaseNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: selector, + }) + if err != nil { + return fmt.Errorf("failed to list pods: %w", err) + } + + if len(pods.Items) == 0 { + utils.UserNote("No matching sessions found") + return nil + } + + // Delete each pod + stoppedCount := 0 + for _, pod := range pods.Items { + sessionID := pod.Labels["session-id"] + logger.Info().Str("pod", pod.Name).Str("sessionID", sessionID).Msg("Stopping session pod") + + if err := clientset.CoreV1().Pods(chart.ReleaseNamespace).Delete(ctx, pod.Name, metav1.DeleteOptions{}); err != nil { + utils.UserWarning("Failed to stop session %s: %v", sessionID, err) + } else { + utils.UserOutput("Stopped session: %s (pod: %s)\n", sessionID, pod.Name) + stoppedCount++ + } + } + + utils.UserNote("Stopped %d session(s)", stoppedCount) + return nil +} diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index ca45a5a..4a93458 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -16,7 +16,10 @@ import ( "github.com/weka/gohomecli/internal/utils" ) -const KubeConfigPath = "/etc/rancher/k3s/k3s.yaml" +const ( + KubeConfigPath = "/etc/rancher/k3s/k3s.yaml" + clusterServiceURLFormat = "http://%s.%s.svc.cluster.local:%d" +) // ReadKubeConfig reads the kubeconfig from the given path with fallback to ~/.kube/config func ReadKubeConfig(kubeConfigPath string) ([]byte, error) { @@ -193,6 +196,74 @@ func getContainerStatusReason(containerStatus corev1.ContainerStatus, podReason return "Unknown" } +// GetServiceURL returns the cluster-internal URL of a service found by label selector +func GetServiceURL(ctx context.Context, labelSelector string) (string, error) { + kubeconfig, err := ReadKubeConfig(KubeConfigPath) + if err != nil { + return "", err + } + + config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) + if err != nil { + return "", err + } + + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return "", err + } + + services, err := clientset.CoreV1().Services(ReleaseNamespace).List(ctx, v1.ListOptions{ + LabelSelector: labelSelector, + }) + if err != nil { + return "", err + } + + if len(services.Items) == 0 { + return "", fmt.Errorf("no service found with label selector %q in namespace %s", labelSelector, ReleaseNamespace) + } + + svc := services.Items[0] + if len(svc.Spec.Ports) == 0 { + return "", fmt.Errorf("service %s has no ports defined", svc.Name) + } + + return fmt.Sprintf(clusterServiceURLFormat, + svc.Name, svc.Namespace, svc.Spec.Ports[0].Port), nil +} + +// GetPVCName returns the name of a PVC found by label selector +func GetPVCName(ctx context.Context, labelSelector string) (string, error) { + kubeconfig, err := ReadKubeConfig(KubeConfigPath) + if err != nil { + return "", err + } + + config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) + if err != nil { + return "", err + } + + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return "", err + } + + pvcs, err := clientset.CoreV1().PersistentVolumeClaims(ReleaseNamespace).List(ctx, v1.ListOptions{ + LabelSelector: labelSelector, + }) + if err != nil { + return "", err + } + + if len(pvcs.Items) == 0 { + return "", fmt.Errorf("no PVC found with label selector %q in namespace %s", labelSelector, ReleaseNamespace) + } + + return pvcs.Items[0].Name, nil +} + // GetIngressAddress returns the address of the ingress in ReleaseNamespace namespace func GetIngressAddress(ctx context.Context) (string, error) { kubeconfig, err := ReadKubeConfig(KubeConfigPath) diff --git a/internal/utils/http.go b/internal/utils/http.go new file mode 100644 index 0000000..c993de2 --- /dev/null +++ b/internal/utils/http.go @@ -0,0 +1,80 @@ +package utils + +import ( + "context" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "time" +) + +// HTTPClient provides common HTTP operations +type HTTPClient struct { + client *http.Client +} + +// NewHTTPClient creates a new HTTP client +func NewHTTPClient() *HTTPClient { + return &HTTPClient{ + client: http.DefaultClient, + } +} + +// doGet performs an HTTP GET request and returns the response body reader +// Caller is responsible for closing the response body +func (c *HTTPClient) doGet(ctx context.Context, url string, timeout time.Duration) (*http.Response, error) { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + + resp, err := c.client.Do(req) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusOK { + resp.Body.Close() + return nil, fmt.Errorf("HTTP %d", resp.StatusCode) + } + + return resp, nil +} + +// Get performs an HTTP GET request with timeout and returns the response body +func (c *HTTPClient) Get(ctx context.Context, url string, timeout time.Duration) ([]byte, error) { + resp, err := c.doGet(ctx, url, timeout) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + return io.ReadAll(resp.Body) +} + +// DownloadFile downloads a file from URL to local path with timeout +func (c *HTTPClient) DownloadFile(ctx context.Context, url, localPath string, timeout time.Duration) error { + resp, err := c.doGet(ctx, url, timeout) + if err != nil { + return err + } + defer resp.Body.Close() + + if err := os.MkdirAll(filepath.Dir(localPath), 0o755); err != nil { + return err + } + + out, err := os.Create(localPath) + if err != nil { + return err + } + defer out.Close() + + _, err = io.Copy(out, resp.Body) + return err +} From bc0e2a39b916746262a23739b2e90a2ee61f1e67 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Mon, 26 Jan 2026 15:09:50 +0200 Subject: [PATCH 02/16] get image from configMap --- internal/cli/local/remote/start.go | 20 +++++++++++------ internal/local/chart/kube.go | 36 ++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go index 5fafc44..6891a20 100644 --- a/internal/cli/local/remote/start.go +++ b/internal/cli/local/remote/start.go @@ -18,9 +18,9 @@ import ( ) const ( - remoteAccessLabel = "app" - remoteAccessValue = "remote-access" - remoteAccessImage = "public.ecr.aws/weka/weka-remote-access:v1.0.0" + remoteAccessLabel = "app" + remoteAccessValue = "remote-access" + remoteAccessConfigLabel = "app=remote-access-config" sessionRecordingsPVCLabel = "app=remote-access-recordings" ) @@ -120,8 +120,14 @@ func startRun(cmd *cobra.Command, opts *startOptions) error { return fmt.Errorf("failed to find recordings PVC: %w", err) } + // Get remote-access image from ConfigMap + remoteAccessImage, err := chart.GetConfigMapData(ctx, remoteAccessConfigLabel, "image") + if err != nil { + return fmt.Errorf("failed to get remote-access image from config: %w", err) + } + // Create the pod - pod := buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, opts) + pod := buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, remoteAccessImage, opts) logger.Info(). Str("sessionID", sessionID). @@ -176,7 +182,7 @@ func getKubernetesClient() (*kubernetes.Clientset, error) { return kubernetes.NewForConfig(config) } -func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName string, opts *startOptions) *corev1.Pod { +func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, image string, opts *startOptions) *corev1.Pod { const sharedSocketPath = "/shared/tmate.socket" // Environment variables for tmate container @@ -256,14 +262,14 @@ func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName string, Containers: []corev1.Container{ { Name: "tmate", - Image: remoteAccessImage, + Image: image, Command: []string{"python3", "/tmate/tmate.py"}, Env: tmateEnv, VolumeMounts: tmateMounts, }, { Name: "recorder", - Image: remoteAccessImage, + Image: image, Command: []string{"python3", "/recorder/recorder.py"}, Env: recorderEnv, VolumeMounts: recorderMounts, diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index 4a93458..6089c30 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -264,6 +264,42 @@ func GetPVCName(ctx context.Context, labelSelector string) (string, error) { return pvcs.Items[0].Name, nil } +// GetConfigMapData returns a specific key's value from a ConfigMap found by label selector +func GetConfigMapData(ctx context.Context, labelSelector, key string) (string, error) { + kubeconfig, err := ReadKubeConfig(KubeConfigPath) + if err != nil { + return "", err + } + + config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) + if err != nil { + return "", err + } + + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return "", err + } + + configMaps, err := clientset.CoreV1().ConfigMaps(ReleaseNamespace).List(ctx, v1.ListOptions{ + LabelSelector: labelSelector, + }) + if err != nil { + return "", err + } + + if len(configMaps.Items) == 0 { + return "", fmt.Errorf("no ConfigMap found with label selector %q in namespace %s", labelSelector, ReleaseNamespace) + } + + value, ok := configMaps.Items[0].Data[key] + if !ok { + return "", fmt.Errorf("key %q not found in ConfigMap %s", key, configMaps.Items[0].Name) + } + + return value, nil +} + // GetIngressAddress returns the address of the ingress in ReleaseNamespace namespace func GetIngressAddress(ctx context.Context) (string, error) { kubeconfig, err := ReadKubeConfig(KubeConfigPath) From 38e54c1c63a9bec7549c384c356740b1428ed417 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Tue, 27 Jan 2026 11:18:17 +0200 Subject: [PATCH 03/16] enable remote access values for LWH --- internal/local/chart/v3.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/local/chart/v3.go b/internal/local/chart/v3.go index bae5f90..0fec757 100644 --- a/internal/local/chart/v3.go +++ b/internal/local/chart/v3.go @@ -259,6 +259,11 @@ func configureLWH(*config_v1.Configuration) (yamlMap, error) { writeMapEntry(cfg, "victoria-metrics-k8s-stack.vmalert.enabled", false), // grafana writeMapEntry(cfg, "grafana.initChownData.enabled", false), + // disable large dashboards to stay under Helm 1MB secret limit + writeMapEntry(cfg, "grafana.enabledDashboards.observe-dashboard", false), + writeMapEntry(cfg, "grafana.enabledDashboards.events-insights", false), + // enable remote session client for LWH + writeMapEntry(cfg, "remoteSessionClient.enabled", true), // redis writeMapEntry(cfg, "redis-cluster", yamlMap{ "cluster": yamlMap{ From 64450caab6c78c8421f0c395fad9a81efb54ad44 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Tue, 27 Jan 2026 13:13:58 +0200 Subject: [PATCH 04/16] copy using sidecar deployment --- internal/cli/local/remote/copy_recording.go | 47 +++-- internal/cli/local/remote/list.go | 4 +- internal/cli/local/remote/list_recordings.go | 148 ++++++-------- internal/cli/local/remote/start.go | 20 +- internal/cli/local/remote/stop.go | 6 +- internal/local/chart/kube.go | 202 +++++++++++++++---- 6 files changed, 263 insertions(+), 164 deletions(-) diff --git a/internal/cli/local/remote/copy_recording.go b/internal/cli/local/remote/copy_recording.go index 2718eec..77bd7dd 100644 --- a/internal/cli/local/remote/copy_recording.go +++ b/internal/cli/local/remote/copy_recording.go @@ -4,13 +4,18 @@ import ( "fmt" "os" "path/filepath" + "regexp" + "github.com/google/uuid" "github.com/spf13/cobra" "github.com/weka/gohomecli/internal/local/chart" "github.com/weka/gohomecli/internal/utils" ) +// safeFilenamePattern allows safe characters for recording filenames +var safeFilenamePattern = regexp.MustCompile(`^[a-zA-Z0-9_\-:.]+\.cast$`) + type copyRecordingOptions struct { recording string clusterID string @@ -64,28 +69,34 @@ func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { return fmt.Errorf("failed to create output directory: %w", err) } - baseURL, err := chart.GetServiceURL(ctx, sessionRecordingsServerLabel) + // Create exec client + execClient, err := chart.NewK8sExecClient(ctx, recordingsSidecarLabel, recordingsSidecarContainer) if err != nil { - return fmt.Errorf("failed to find recordings server: %w", err) + return fmt.Errorf("failed to create Kubernetes client: %w", err) } // Determine which files to copy var filesToCopy []RecordingInfo if opts.recording != "" { + // Validate inputs to prevent path traversal + if !safeFilenamePattern.MatchString(opts.recording) { + return fmt.Errorf("invalid recording filename: must be a .cast file with safe characters") + } + if opts.clusterID != "" { + if _, parseErr := uuid.Parse(opts.clusterID); parseErr != nil { + return fmt.Errorf("invalid cluster ID: must be a valid UUID") + } + } // Specific file filesToCopy = []RecordingInfo{{ Filename: opts.recording, ClusterID: opts.clusterID, }} } else { - var err error - if opts.clusterID != "" { - filesToCopy, err = listRecordingsForCluster(ctx, baseURL, opts.clusterID) - } else { - // --all case: copy everything when no specific filters provided - filesToCopy, err = listAllRecordings(ctx, baseURL) - } + // List by cluster ID or all (empty clusterID = all) + // listRecordings validates clusterID internally + filesToCopy, err = listRecordings(ctx, execClient, opts.clusterID) if err != nil { return fmt.Errorf("failed to list recordings: %w", err) } @@ -102,24 +113,24 @@ func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { copiedCount := 0 for _, recording := range filesToCopy { // Construct remote path from ClusterID + Filename - remotePath := recording.Filename + // Filename may contain subdirectories (e.g., "subdir/file.cast") + remotePath := filepath.Join(recordingsPath, recording.Filename) if recording.ClusterID != "" { - remotePath = filepath.Join(recording.ClusterID, recording.Filename) + remotePath = filepath.Join(recordingsPath, recording.ClusterID, recording.Filename) } - dstPath := filepath.Join(opts.output, recording.Filename) + // Use basename for local destination (flatten directory structure) + localFilename := filepath.Base(recording.Filename) + dstPath := filepath.Join(opts.output, localFilename) logger.Debug().Str("src", remotePath).Str("dst", dstPath).Msg("Copying file") - httpClient := utils.NewHTTPClient() - url := fmt.Sprintf("%s/%s", baseURL, remotePath) - err := httpClient.DownloadFile(ctx, url, dstPath, fileDownloadTimeout) - if err != nil { - utils.UserWarning("Failed to copy %s: %v", recording.Filename, err) + if err := execClient.CopyFromPod(ctx, remotePath, dstPath); err != nil { + utils.UserWarning("Failed to copy %s: %v", localFilename, err) continue } - utils.UserOutput(" Copied: %s\n", recording.Filename) + utils.UserOutput(" Copied: %s\n", localFilename) copiedCount++ } diff --git a/internal/cli/local/remote/list.go b/internal/cli/local/remote/list.go index 5d7a103..fe4601f 100644 --- a/internal/cli/local/remote/list.go +++ b/internal/cli/local/remote/list.go @@ -51,14 +51,14 @@ type SessionInfo struct { func listRun(cmd *cobra.Command, opts *listOptions) error { ctx := cmd.Context() - clientset, err := getKubernetesClient() + k8s, err := chart.NewKubernetesClient() if err != nil { return fmt.Errorf("failed to create Kubernetes client: %w", err) } // List pods with app=remote-session label selector := fmt.Sprintf("%s=%s", remoteAccessLabel, remoteAccessValue) - pods, err := clientset.CoreV1().Pods(chart.ReleaseNamespace).List(ctx, metav1.ListOptions{ + pods, err := k8s.Clientset.CoreV1().Pods(chart.ReleaseNamespace).List(ctx, metav1.ListOptions{ LabelSelector: selector, }) if err != nil { diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go index 0fb6a01..c4eb24e 100644 --- a/internal/cli/local/remote/list_recordings.go +++ b/internal/cli/local/remote/list_recordings.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "fmt" + "strconv" "strings" "time" + "github.com/google/uuid" "github.com/spf13/cobra" "gopkg.in/yaml.v3" @@ -15,13 +17,9 @@ import ( ) const ( - sessionRecordingsServerLabel = "app=remote-access-recordings-server" - httpRequestTimeout = 10 * time.Second - fileDownloadTimeout = 5 * time.Minute - - // dufs path_type values - dufsPathTypeFile = "File" - dufsPathTypeDir = "Dir" + recordingsSidecarLabel = "app=remote-access-recordings-sidecar" + recordingsSidecarContainer = "sidecar" + recordingsPath = "/recordings" ) type listRecordingsOptions struct { @@ -63,34 +61,16 @@ type RecordingInfo struct { ClusterID string `json:"clusterId,omitempty" yaml:"clusterId,omitempty"` } -// dufsResponse represents the JSON response from dufs directory listing -type dufsResponse struct { - Paths []dufsEntry `json:"paths"` -} - -// dufsEntry represents a file/directory entry from dufs JSON response -type dufsEntry struct { - Name string `json:"name"` - PathType string `json:"path_type"` // "File" or "Dir" - Size int64 `json:"size"` - Mtime int64 `json:"mtime"` // milliseconds since epoch -} - func listRecordingsRun(cmd *cobra.Command, opts *listRecordingsOptions) error { ctx := cmd.Context() - baseURL, err := chart.GetServiceURL(ctx, sessionRecordingsServerLabel) + // Create exec client + execClient, err := chart.NewK8sExecClient(ctx, recordingsSidecarLabel, recordingsSidecarContainer) if err != nil { - return fmt.Errorf("failed to find recordings server: %w", err) + return fmt.Errorf("failed to create Kubernetes client: %w", err) } - var recordings []RecordingInfo - - if opts.clusterID != "" { - recordings, err = listRecordingsForCluster(ctx, baseURL, opts.clusterID) - } else { - recordings, err = listAllRecordings(ctx, baseURL) - } + recordings, err := listRecordings(ctx, execClient, opts.clusterID) if err != nil { return err } @@ -111,77 +91,77 @@ func listRecordingsRun(cmd *cobra.Command, opts *listRecordingsOptions) error { } } -// listRecordingsForCluster lists recordings for a specific cluster ID -func listRecordingsForCluster(ctx context.Context, baseURL, clusterID string) ([]RecordingInfo, error) { - url := fmt.Sprintf("%s/%s?json", baseURL, clusterID) - entries, err := fetchDufsDirectory(ctx, url) - if err != nil { - // Directory might not exist - return nil, nil - } - - return filterAndConvertEntries(entries, clusterID), nil -} - -// listAllRecordings lists all recordings from cluster subdirectories and root level -func listAllRecordings(ctx context.Context, baseURL string) ([]RecordingInfo, error) { - url := fmt.Sprintf("%s?json", baseURL) - entries, err := fetchDufsDirectory(ctx, url) - if err != nil { - return nil, fmt.Errorf("failed to list recordings: %w", err) - } - - var recordings []RecordingInfo - - for _, entry := range entries { - if entry.PathType == dufsPathTypeDir { - // Directory = cluster_id subdirectory - clusterRecordings, _ := listRecordingsForCluster(ctx, baseURL, entry.Name) - recordings = append(recordings, clusterRecordings...) - } else if entry.PathType == dufsPathTypeFile && strings.HasSuffix(entry.Name, ".cast") { - // Root level .cast file (backward compat - no cluster_id) - recordings = append(recordings, RecordingInfo{ - Filename: entry.Name, - Size: entry.Size, - ModTime: entry.Mtime / 1000, // Convert milliseconds to seconds - ClusterID: "", - }) +// listRecordings lists recordings, optionally filtered by cluster ID +func listRecordings(ctx context.Context, client *chart.K8sExecClient, clusterID string) ([]RecordingInfo, error) { + searchPath := recordingsPath + if clusterID != "" { + // Validate clusterID is a valid UUID to prevent shell injection + if _, err := uuid.Parse(clusterID); err != nil { + return nil, fmt.Errorf("invalid cluster ID: must be a valid UUID") } + searchPath = fmt.Sprintf("%s/%s", recordingsPath, clusterID) } - return recordings, nil -} - -// fetchDufsDirectory fetches directory listing from dufs server -func fetchDufsDirectory(ctx context.Context, url string) ([]dufsEntry, error) { - httpClient := utils.NewHTTPClient() - - body, err := httpClient.Get(ctx, url, httpRequestTimeout) + // Check if directory exists first + _, err := client.Exec(ctx, "test", "-d", searchPath) if err != nil { - return nil, err + // Directory doesn't exist - return empty results (not an error) + return nil, nil //nolint:nilerr // test -d exits non-zero if dir doesn't exist; that's not an error for us } - var response dufsResponse - if err := json.Unmarshal(body, &response); err != nil { - return nil, err + // Find all .cast files recursively with stat info + // Output format: /recordings/[clusterID/]filename.cast|size|mtime + output, err := client.Exec(ctx, "sh", "-c", + fmt.Sprintf(`find %s -name '*.cast' -type f -exec stat -c '%%n|%%s|%%Y' {} \;`, searchPath)) + if err != nil { + // Directory exists but find failed - this is a real error + return nil, fmt.Errorf("failed to list recordings: %w", err) } - return response.Paths, nil + return parseStatOutput(output), nil } -// filterAndConvertEntries filters entries by .cast extension -func filterAndConvertEntries(entries []dufsEntry, clusterID string) []RecordingInfo { +// parseStatOutput parses the output of stat command into RecordingInfo structs +// Path format: /recordings/filename.cast or /recordings/clusterID/filename.cast +func parseStatOutput(output string) []RecordingInfo { var recordings []RecordingInfo - for _, entry := range entries { - if entry.PathType != dufsPathTypeFile || !strings.HasSuffix(entry.Name, ".cast") { + lines := strings.Split(strings.TrimSpace(output), "\n") + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" { + continue + } + + parts := strings.Split(line, "|") + if len(parts) != 3 { continue } + // Extract filename and cluster ID from path + // /recordings/file.cast -> clusterID="", filename="file.cast" + // /recordings/abc-123/file.cast -> clusterID="abc-123", filename="file.cast" + // /recordings/abc-123/subdir/file.cast -> clusterID="abc-123", filename="subdir/file.cast" + fullPath := parts[0] + relPath := strings.TrimPrefix(fullPath, recordingsPath+"/") + pathParts := strings.Split(relPath, "/") + + var filename, clusterID string + if len(pathParts) == 1 { + filename = pathParts[0] + } else { + clusterID = pathParts[0] + // Keep the full relative path after cluster ID (handles nested dirs) + filename = strings.Join(pathParts[1:], "/") + } + + size, _ := strconv.ParseInt(parts[1], 10, 64) + mtime, _ := strconv.ParseInt(parts[2], 10, 64) + recordings = append(recordings, RecordingInfo{ - Filename: entry.Name, - Size: entry.Size, - ModTime: entry.Mtime / 1000, // Convert milliseconds to seconds + Filename: filename, + Size: size, + ModTime: mtime, ClusterID: clusterID, }) } diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go index 6891a20..6aac096 100644 --- a/internal/cli/local/remote/start.go +++ b/internal/cli/local/remote/start.go @@ -9,8 +9,6 @@ import ( "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/tools/clientcmd" "github.com/weka/gohomecli/internal/env" "github.com/weka/gohomecli/internal/local/chart" @@ -109,7 +107,7 @@ func startRun(cmd *cobra.Command, opts *startOptions) error { sessionID := generateShortID() // Create Kubernetes client - clientset, err := getKubernetesClient() + k8s, err := chart.NewKubernetesClient() if err != nil { return fmt.Errorf("failed to create Kubernetes client: %w", err) } @@ -135,7 +133,7 @@ func startRun(cmd *cobra.Command, opts *startOptions) error { Str("clusterName", opts.clusterName). Msg("Creating remote session pod...") - createdPod, err := clientset.CoreV1().Pods(chart.ReleaseNamespace).Create(ctx, pod, metav1.CreateOptions{}) + createdPod, err := k8s.Clientset.CoreV1().Pods(chart.ReleaseNamespace).Create(ctx, pod, metav1.CreateOptions{}) if err != nil { return fmt.Errorf("failed to create session pod: %w", err) } @@ -168,20 +166,6 @@ func generateShortID() string { return hex.EncodeToString(bytes) } -func getKubernetesClient() (*kubernetes.Clientset, error) { - kubeconfig, err := chart.ReadKubeConfig(chart.KubeConfigPath) - if err != nil { - return nil, err - } - - config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) - if err != nil { - return nil, err - } - - return kubernetes.NewForConfig(config) -} - func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, image string, opts *startOptions) *corev1.Pod { const sharedSocketPath = "/shared/tmate.socket" diff --git a/internal/cli/local/remote/stop.go b/internal/cli/local/remote/stop.go index bd50bae..39859f6 100644 --- a/internal/cli/local/remote/stop.go +++ b/internal/cli/local/remote/stop.go @@ -53,7 +53,7 @@ func newStopCmd() *cobra.Command { func stopRun(cmd *cobra.Command, opts *stopOptions) error { ctx := cmd.Context() - clientset, err := getKubernetesClient() + k8s, err := chart.NewKubernetesClient() if err != nil { return fmt.Errorf("failed to create Kubernetes client: %w", err) } @@ -70,7 +70,7 @@ func stopRun(cmd *cobra.Command, opts *stopOptions) error { logger.Debug().Str("selector", selector).Msg("Listing pods to stop") // List matching pods - pods, err := clientset.CoreV1().Pods(chart.ReleaseNamespace).List(ctx, metav1.ListOptions{ + pods, err := k8s.Clientset.CoreV1().Pods(chart.ReleaseNamespace).List(ctx, metav1.ListOptions{ LabelSelector: selector, }) if err != nil { @@ -88,7 +88,7 @@ func stopRun(cmd *cobra.Command, opts *stopOptions) error { sessionID := pod.Labels["session-id"] logger.Info().Str("pod", pod.Name).Str("sessionID", sessionID).Msg("Stopping session pod") - if err := clientset.CoreV1().Pods(chart.ReleaseNamespace).Delete(ctx, pod.Name, metav1.DeleteOptions{}); err != nil { + if err := k8s.Clientset.CoreV1().Pods(chart.ReleaseNamespace).Delete(ctx, pod.Name, metav1.DeleteOptions{}); err != nil { utils.UserWarning("Failed to stop session %s: %v", sessionID, err) } else { utils.UserOutput("Stopped session: %s (pod: %s)\n", sessionID, pod.Name) diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index 6089c30..a9d9996 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -1,8 +1,11 @@ package chart import ( + "archive/tar" + "bytes" "context" "fmt" + "io" "net" "os" "path/filepath" @@ -11,16 +14,174 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/tools/remotecommand" "github.com/weka/gohomecli/internal/utils" ) const ( - KubeConfigPath = "/etc/rancher/k3s/k3s.yaml" - clusterServiceURLFormat = "http://%s.%s.svc.cluster.local:%d" + KubeConfigPath = "/etc/rancher/k3s/k3s.yaml" ) +// K8sClient holds a Kubernetes clientset and REST config +type K8sClient struct { + Clientset *kubernetes.Clientset + Config *rest.Config +} + +// NewKubernetesClient creates a new Kubernetes client from the default kubeconfig +func NewKubernetesClient() (*K8sClient, error) { + kubeconfig, err := ReadKubeConfig(KubeConfigPath) + if err != nil { + return nil, err + } + + config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) + if err != nil { + return nil, err + } + + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return nil, err + } + + return &K8sClient{ + Clientset: clientset, + Config: config, + }, nil +} + +// K8sExecClient holds Kubernetes client info for executing commands in a specific pod +type K8sExecClient struct { + K8s *K8sClient + PodName string + Container string +} + +// NewK8sExecClient creates a client for executing commands in a pod found by label selector +func NewK8sExecClient(ctx context.Context, labelSelector, container string) (*K8sExecClient, error) { + k8s, err := NewKubernetesClient() + if err != nil { + return nil, fmt.Errorf("failed to create Kubernetes client: %w", err) + } + + pods, err := k8s.Clientset.CoreV1().Pods(ReleaseNamespace).List(ctx, v1.ListOptions{ + LabelSelector: labelSelector, + }) + if err != nil { + return nil, fmt.Errorf("failed to list pods: %w", err) + } + + if len(pods.Items) == 0 { + return nil, fmt.Errorf("no pod found with label selector %q", labelSelector) + } + + return &K8sExecClient{ + K8s: k8s, + PodName: pods.Items[0].Name, + Container: container, + }, nil +} + +// newExecutor creates a remotecommand executor for the given command +func (c *K8sExecClient) newExecutor(command []string) (remotecommand.Executor, error) { + req := c.K8s.Clientset.CoreV1().RESTClient().Post(). + Resource("pods"). + Name(c.PodName). + Namespace(ReleaseNamespace). + SubResource("exec"). + VersionedParams(&corev1.PodExecOptions{ + Container: c.Container, + Command: command, + Stdout: true, + Stderr: true, + }, scheme.ParameterCodec) + + return remotecommand.NewSPDYExecutor(c.K8s.Config, "POST", req.URL()) +} + +// Exec runs a command in the pod and returns the output +func (c *K8sExecClient) Exec(ctx context.Context, command ...string) (string, error) { + executor, err := c.newExecutor(command) + if err != nil { + return "", fmt.Errorf("failed to create executor: %w", err) + } + + var stdout, stderr bytes.Buffer + err = executor.StreamWithContext(ctx, remotecommand.StreamOptions{ + Stdout: &stdout, + Stderr: &stderr, + }) + if err != nil { + return "", fmt.Errorf("exec failed: %s - %w", stderr.String(), err) + } + + return stdout.String(), nil +} + +// CopyFromPod copies a file from the pod to the local filesystem using streaming +func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath string) error { + executor, err := c.newExecutor([]string{"tar", "cf", "-", "-C", filepath.Dir(remotePath), filepath.Base(remotePath)}) + if err != nil { + return fmt.Errorf("failed to create executor: %w", err) + } + + // Use pipe for streaming (memory efficient for large files) + reader, writer := io.Pipe() + var stderr bytes.Buffer + execErrCh := make(chan error, 1) + + // Stream tar from pod in background + go func() { + defer writer.Close() + execErrCh <- executor.StreamWithContext(ctx, remotecommand.StreamOptions{ + Stdout: writer, + Stderr: &stderr, + }) + }() + + // Extract file from tar stream + extractErr := extractTarFile(reader, localPath) + + // Wait for goroutine to complete and get its error + execErr := <-execErrCh + + if extractErr != nil { + return extractErr + } + if execErr != nil { + return fmt.Errorf("tar exec failed: %s - %w", stderr.String(), execErr) + } + return nil +} + +// extractTarFile extracts a single file from a tar stream +func extractTarFile(reader io.Reader, destPath string) error { + tr := tar.NewReader(reader) + for { + header, err := tr.Next() + if err == io.EOF { + return fmt.Errorf("file not found in tar archive") + } + if err != nil { + return fmt.Errorf("failed to read tar: %w", err) + } + if header.Typeflag == tar.TypeReg { + outFile, err := os.Create(destPath) + if err != nil { + return fmt.Errorf("failed to create file: %w", err) + } + _, err = io.Copy(outFile, tr) + outFile.Close() + return err + } + } +} + // ReadKubeConfig reads the kubeconfig from the given path with fallback to ~/.kube/config func ReadKubeConfig(kubeConfigPath string) ([]byte, error) { if kubeConfigPath == "" { @@ -196,43 +357,6 @@ func getContainerStatusReason(containerStatus corev1.ContainerStatus, podReason return "Unknown" } -// GetServiceURL returns the cluster-internal URL of a service found by label selector -func GetServiceURL(ctx context.Context, labelSelector string) (string, error) { - kubeconfig, err := ReadKubeConfig(KubeConfigPath) - if err != nil { - return "", err - } - - config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) - if err != nil { - return "", err - } - - clientset, err := kubernetes.NewForConfig(config) - if err != nil { - return "", err - } - - services, err := clientset.CoreV1().Services(ReleaseNamespace).List(ctx, v1.ListOptions{ - LabelSelector: labelSelector, - }) - if err != nil { - return "", err - } - - if len(services.Items) == 0 { - return "", fmt.Errorf("no service found with label selector %q in namespace %s", labelSelector, ReleaseNamespace) - } - - svc := services.Items[0] - if len(svc.Spec.Ports) == 0 { - return "", fmt.Errorf("service %s has no ports defined", svc.Name) - } - - return fmt.Sprintf(clusterServiceURLFormat, - svc.Name, svc.Namespace, svc.Spec.Ports[0].Port), nil -} - // GetPVCName returns the name of a PVC found by label selector func GetPVCName(ctx context.Context, labelSelector string) (string, error) { kubeconfig, err := ReadKubeConfig(KubeConfigPath) From d1247379557c6f2f4bdbf47846d986fc6c9eba25 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Tue, 27 Jan 2026 13:18:41 +0200 Subject: [PATCH 05/16] remove http.go --- internal/utils/http.go | 80 ------------------------------------------ 1 file changed, 80 deletions(-) delete mode 100644 internal/utils/http.go diff --git a/internal/utils/http.go b/internal/utils/http.go deleted file mode 100644 index c993de2..0000000 --- a/internal/utils/http.go +++ /dev/null @@ -1,80 +0,0 @@ -package utils - -import ( - "context" - "fmt" - "io" - "net/http" - "os" - "path/filepath" - "time" -) - -// HTTPClient provides common HTTP operations -type HTTPClient struct { - client *http.Client -} - -// NewHTTPClient creates a new HTTP client -func NewHTTPClient() *HTTPClient { - return &HTTPClient{ - client: http.DefaultClient, - } -} - -// doGet performs an HTTP GET request and returns the response body reader -// Caller is responsible for closing the response body -func (c *HTTPClient) doGet(ctx context.Context, url string, timeout time.Duration) (*http.Response, error) { - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return nil, err - } - - resp, err := c.client.Do(req) - if err != nil { - return nil, err - } - - if resp.StatusCode != http.StatusOK { - resp.Body.Close() - return nil, fmt.Errorf("HTTP %d", resp.StatusCode) - } - - return resp, nil -} - -// Get performs an HTTP GET request with timeout and returns the response body -func (c *HTTPClient) Get(ctx context.Context, url string, timeout time.Duration) ([]byte, error) { - resp, err := c.doGet(ctx, url, timeout) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - return io.ReadAll(resp.Body) -} - -// DownloadFile downloads a file from URL to local path with timeout -func (c *HTTPClient) DownloadFile(ctx context.Context, url, localPath string, timeout time.Duration) error { - resp, err := c.doGet(ctx, url, timeout) - if err != nil { - return err - } - defer resp.Body.Close() - - if err := os.MkdirAll(filepath.Dir(localPath), 0o755); err != nil { - return err - } - - out, err := os.Create(localPath) - if err != nil { - return err - } - defer out.Close() - - _, err = io.Copy(out, resp.Body) - return err -} From a3e3564753bdb651331cd40dc2d7e4bb75b68fbe Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Tue, 27 Jan 2026 15:49:04 +0200 Subject: [PATCH 06/16] fix copy recordings --- internal/cli/app/app.go | 4 +- internal/cli/cli.go | 2 + internal/cli/local/local.go | 4 -- internal/cli/local/remote/copy_recording.go | 33 ++++++++++---- internal/cli/local/remote/list.go | 10 ++-- internal/cli/local/remote/list_recordings.go | 6 +-- internal/cli/local/remote/remote.go | 47 +++++++++++-------- internal/cli/local/remote/start.go | 12 +++-- internal/cli/local/remote/stop.go | 12 ++--- internal/env/config.go | 5 +- internal/local/chart/kube.go | 48 ++++++++++++++++++-- 11 files changed, 123 insertions(+), 60 deletions(-) diff --git a/internal/cli/app/app.go b/internal/cli/app/app.go index acd84fe..4928d56 100644 --- a/internal/cli/app/app.go +++ b/internal/cli/app/app.go @@ -10,6 +10,7 @@ import ( "github.com/weka/gohomecli/internal/cli/api" "github.com/weka/gohomecli/internal/cli/config" + "github.com/weka/gohomecli/internal/cli/local/remote" "github.com/weka/gohomecli/internal/env" ) @@ -28,7 +29,8 @@ var appCmd = &cobra.Command{ env.InitEnv() - if cmdHasGroup(cmd, api.APIGroup.ID, config.ConfigGroup.ID) { + if cmdHasGroup(cmd, api.APIGroup.ID, config.ConfigGroup.ID, remote.RemoteAccessGroup.ID) { + env.SkipAPIKeyValidation = cmdHasGroup(cmd, remote.RemoteAccessGroup.ID) env.InitConfig(env.SiteName) } diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 78231f6..949b7c1 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -7,6 +7,7 @@ import ( "github.com/weka/gohomecli/internal/cli/app" "github.com/weka/gohomecli/internal/cli/config" "github.com/weka/gohomecli/internal/cli/local" + "github.com/weka/gohomecli/internal/cli/local/remote" "github.com/weka/gohomecli/internal/utils" ) @@ -14,6 +15,7 @@ func init() { api.Cli.InitCobra(app.Cmd()) config.Cli.InitCobra(app.Cmd()) local.Cli.InitCobra(app.Cmd()) + remote.Cli.InitCobra(app.Cmd()) } // Execute adds all child commands to the root command and sets flags appropriately. diff --git a/internal/cli/local/local.go b/internal/cli/local/local.go index fb0024a..a0f1c64 100644 --- a/internal/cli/local/local.go +++ b/internal/cli/local/local.go @@ -6,7 +6,6 @@ import ( "github.com/weka/gohomecli/internal/cli/app/hooks" "github.com/weka/gohomecli/internal/cli/local/cleanup" "github.com/weka/gohomecli/internal/cli/local/debug" - "github.com/weka/gohomecli/internal/cli/local/remote" "github.com/weka/gohomecli/internal/cli/local/setup" "github.com/weka/gohomecli/internal/cli/local/status" "github.com/weka/gohomecli/internal/cli/local/upgrade" @@ -43,9 +42,6 @@ func init() { upgrade.Cli.InitCobra(localCmd) cleanup.Cli.InitCobra(localCmd) - remoteCli := remote.CliHook() - remoteCli.InitCobra(localCmd) - statusCli := status.CliHook() statusCli.InitCobra(localCmd) }) diff --git a/internal/cli/local/remote/copy_recording.go b/internal/cli/local/remote/copy_recording.go index 77bd7dd..f669e08 100644 --- a/internal/cli/local/remote/copy_recording.go +++ b/internal/cli/local/remote/copy_recording.go @@ -34,9 +34,9 @@ func newCopyRecordingCmd() *cobra.Command { Recordings can be copied individually or in bulk by cluster ID or all at once. Examples: - homecli local remote copy-recording --recording "2024-01-15T10:30:00-abc123.cast" --output /tmp/ - homecli local remote copy-recording --cluster-id "550e8400-e29b-41d4-a716-446655440000" --output /tmp/ - homecli local remote copy-recording --all --output /tmp/ + homecli remote-access copy-recording --recording "2024-01-15T10:30:00-abc123.cast" --output /tmp/ + homecli remote-access copy-recording --cluster-id "550e8400-e29b-41d4-a716-446655440000" --output /tmp/ + homecli remote-access copy-recording --all --output /tmp/ `, RunE: func(cmd *cobra.Command, args []string) error { return copyRecordingRun(cmd, opts) @@ -87,12 +87,27 @@ func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { if _, parseErr := uuid.Parse(opts.clusterID); parseErr != nil { return fmt.Errorf("invalid cluster ID: must be a valid UUID") } + // Cluster ID explicitly provided + filesToCopy = []RecordingInfo{{ + Filename: opts.recording, + ClusterID: opts.clusterID, + }} + } else { + // No cluster ID provided - search for the recording to find its cluster ID + allRecordings, listErr := listRecordings(ctx, execClient, "") + if listErr != nil { + return fmt.Errorf("failed to list recordings: %w", listErr) + } + for _, r := range allRecordings { + if r.Filename == opts.recording { + filesToCopy = []RecordingInfo{r} + break + } + } + if len(filesToCopy) == 0 { + return fmt.Errorf("recording not found: %s", opts.recording) + } } - // Specific file - filesToCopy = []RecordingInfo{{ - Filename: opts.recording, - ClusterID: opts.clusterID, - }} } else { // List by cluster ID or all (empty clusterID = all) // listRecordings validates clusterID internally @@ -123,8 +138,6 @@ func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { localFilename := filepath.Base(recording.Filename) dstPath := filepath.Join(opts.output, localFilename) - logger.Debug().Str("src", remotePath).Str("dst", dstPath).Msg("Copying file") - if err := execClient.CopyFromPod(ctx, remotePath, dstPath); err != nil { utils.UserWarning("Failed to copy %s: %v", localFilename, err) continue diff --git a/internal/cli/local/remote/list.go b/internal/cli/local/remote/list.go index fe4601f..35691ac 100644 --- a/internal/cli/local/remote/list.go +++ b/internal/cli/local/remote/list.go @@ -23,12 +23,12 @@ func newListCmd() *cobra.Command { cmd := &cobra.Command{ Use: "list", Short: "List active remote sessions", - Long: `List active remote sessions by querying pods with app=remote-session label. + Long: `List active remote sessions. - Examples: - homecli local remote list # List all active sessions in table format - homecli local remote list --output json # List sessions in JSON format - homecli local remote list --output yaml # List sessions in YAML format +Examples: + homecli remote-access list # List all active sessions in table format + homecli remote-access list --output json # List sessions in JSON format + homecli remote-access list --output yaml # List sessions in YAML format `, RunE: func(cmd *cobra.Command, args []string) error { return listRun(cmd, opts) diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go index c4eb24e..e18acbe 100644 --- a/internal/cli/local/remote/list_recordings.go +++ b/internal/cli/local/remote/list_recordings.go @@ -38,9 +38,9 @@ func newListRecordingsCmd() *cobra.Command { Recordings are stored as asciinema .cast files and can be filtered by cluster ID. Examples: - homecli local remote list-recordings - homecli local remote list-recordings --cluster-id 550e8400-e29b-41d4-a716-446655440000 - homecli local remote list-recordings --output json + homecli remote-access list-recordings + homecli remote-access list-recordings --cluster-id 550e8400-e29b-41d4-a716-446655440000 + homecli remote-access list-recordings --output json `, RunE: func(cmd *cobra.Command, args []string) error { return listRecordingsRun(cmd, opts) diff --git a/internal/cli/local/remote/remote.go b/internal/cli/local/remote/remote.go index 357b6e3..5cc6f59 100644 --- a/internal/cli/local/remote/remote.go +++ b/internal/cli/local/remote/remote.go @@ -10,25 +10,32 @@ import ( var logger = utils.GetLogger("Remote") -// CliHook returns a Cobra CLI hook for the remote command -func CliHook() hooks.Cli { - var cli hooks.Cli - - remoteCmd := &cobra.Command{ - Use: "remote", - Short: "Manage remote tmate sessions", - Long: "Start, stop, and manage remote tmate sessions for cluster debugging", - } - - remoteCmd.AddCommand(newStartCmd()) - remoteCmd.AddCommand(newStopCmd()) - remoteCmd.AddCommand(newListCmd()) - remoteCmd.AddCommand(newListRecordingsCmd()) - remoteCmd.AddCommand(newCopyRecordingCmd()) - - cli.AddHook(func(localCmd *cobra.Command) { - localCmd.AddCommand(remoteCmd) - }) +// RemoteAccessGroup is the command group for remote access commands +var RemoteAccessGroup = cobra.Group{ + ID: "remote-access", + Title: "Remote Access Commands", +} + +// Cli is the hooks.Cli instance for remote access commands +var Cli hooks.Cli + +func init() { + Cli.AddHook(func(appCmd *cobra.Command) { + appCmd.AddGroup(&RemoteAccessGroup) - return cli + remoteCmd := &cobra.Command{ + Use: "remote-access", + Short: "Manage remote tmate sessions", + Long: "Start, stop, and manage remote tmate sessions and recordings for cluster debugging", + GroupID: RemoteAccessGroup.ID, + } + + remoteCmd.AddCommand(newStartCmd()) + remoteCmd.AddCommand(newStopCmd()) + remoteCmd.AddCommand(newListCmd()) + remoteCmd.AddCommand(newListRecordingsCmd()) + remoteCmd.AddCommand(newCopyRecordingCmd()) + + appCmd.AddCommand(remoteCmd) + }) } diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go index 6aac096..f13ac63 100644 --- a/internal/cli/local/remote/start.go +++ b/internal/cli/local/remote/start.go @@ -46,10 +46,11 @@ func newStartCmd() *cobra.Command { Use: "start", Short: "Start a new remote-access tmate session", Long: `Start a remote-access tmate session for remote cluster connection. - Tmate server config is resolved from cloud URL. Use --tmate-server-* flags for custom servers. +Tmate server config is resolved from cloud URL. Use --tmate-server-* flags for custom servers. - Examples: - homecli local remote start --cluster-id "550e8400-..." --cluster-name "prod" --ssh-keys-path "/root/.ssh" # Start a session with cloud URL from config +Examples: + homecli remote-access start --cluster-id "550e8400-..." --cluster-name "prod" --ssh-keys-path "/root/.ssh" # Start a session with cloud URL from config + homecli remote-access start --cluster-id "550e8400-..." --cluster-name "prod" --ssh-keys-path "/root/.ssh" --tmate-server-host "tmate.example.com" --tmate-server-port "22" --tmate-server-rsa-fingerprint "1234567890" --tmate-server-ed25519-fingerprint "1234567890" --tmate-server-ecdsa-fingerprint "1234567890" # Start a session with custom tmate server `, RunE: func(cmd *cobra.Command, args []string) error { return startRun(cmd, opts) @@ -84,13 +85,14 @@ func newStartCmd() *cobra.Command { func startRun(cmd *cobra.Command, opts *startOptions) error { ctx := cmd.Context() - // Resolve cloud URL from flag or config + // Resolve cloud URL from flag, config, or default cloudURL := opts.cloudURL if cloudURL == "" && env.CurrentSiteConfig != nil { cloudURL = env.CurrentSiteConfig.CloudURL } if cloudURL == "" { - return fmt.Errorf("cloud URL not found in site config, please provide --cloud-url flag explicitly") + cloudURL = env.DefaultCloudURL + utils.UserNote("Cloud URL not configured, using default: %s", cloudURL) } // Get local LWH address from ingress diff --git a/internal/cli/local/remote/stop.go b/internal/cli/local/remote/stop.go index 39859f6..1d54225 100644 --- a/internal/cli/local/remote/stop.go +++ b/internal/cli/local/remote/stop.go @@ -22,15 +22,13 @@ func newStopCmd() *cobra.Command { cmd := &cobra.Command{ Use: "stop", Short: "Stop running remote session(s)", - Long: `Stop running remote session(s) by deleting the pod. + Long: `Stop running remote session(s). - Pod deletion triggers graceful shutdown via SIGTERM, allowing the session - to properly close connections and finalize recordings. - Examples: - homecli local remote stop --session-id a1b2c3 # Stop a specific session by ID - homecli local remote stop --cluster-id 550e8400-e29b-41d4-a716-446655440000 # Stop all sessions for a cluster - homecli local remote stop --all # Stop all active sessions +Examples: + homecli remote-access stop --session-id a1b2c3 # Stop a specific session by ID + homecli remote-access stop --cluster-id 550e8400-e29b-41d4-a716-446655440000 # Stop all sessions for a cluster + homecli remote-access stop --all # Stop all active sessions `, RunE: func(cmd *cobra.Command, args []string) error { return stopRun(cmd, opts) diff --git a/internal/env/config.go b/internal/env/config.go index 57e8ecb..c8e91aa 100644 --- a/internal/env/config.go +++ b/internal/env/config.go @@ -54,6 +54,9 @@ type Config struct { DefaultSite string `toml:"default_site"` } +// SkipAPIKeyValidation can be set to true before calling InitConfig to skip API key validation. +var SkipAPIKeyValidation bool + func InitConfig(siteNameFromCommandLine string) { if initialized { return @@ -174,7 +177,7 @@ func getSiteConfig(config *Config, siteNameFromCommandLine string) (*SiteConfig, } func validateSiteConfig(siteConfig *SiteConfig, siteName string) { - if siteConfig.APIKey == "" { + if !SkipAPIKeyValidation && siteConfig.APIKey == "" { utils.UserWarning("config error: \"api_key\" is unset for site %s", siteName) } if siteConfig.CloudURL == "" { diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index a9d9996..06b024b 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -9,6 +9,8 @@ import ( "net" "os" "path/filepath" + "strings" + "time" helmclient "github.com/mittwald/go-helm-client" corev1 "k8s.io/api/core/v1" @@ -125,11 +127,18 @@ func (c *K8sExecClient) Exec(ctx context.Context, command ...string) (string, er // CopyFromPod copies a file from the pod to the local filesystem using streaming func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath string) error { - executor, err := c.newExecutor([]string{"tar", "cf", "-", "-C", filepath.Dir(remotePath), filepath.Base(remotePath)}) + // Use sh -c to cd first, then tar - more portable across tar implementations (busybox, gnu) + tarCmd := fmt.Sprintf("cd %q && tar cf - %q", filepath.Dir(remotePath), filepath.Base(remotePath)) + + executor, err := c.newExecutor([]string{"sh", "-c", tarCmd}) if err != nil { return fmt.Errorf("failed to create executor: %w", err) } + // Add timeout to prevent indefinite hangs + copyCtx, cancel := context.WithTimeout(ctx, 60*time.Second) + defer cancel() + // Use pipe for streaming (memory efficient for large files) reader, writer := io.Pipe() var stderr bytes.Buffer @@ -138,7 +147,7 @@ func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath s // Stream tar from pod in background go func() { defer writer.Close() - execErrCh <- executor.StreamWithContext(ctx, remotecommand.StreamOptions{ + execErrCh <- executor.StreamWithContext(copyCtx, remotecommand.StreamOptions{ Stdout: writer, Stderr: &stderr, }) @@ -147,30 +156,61 @@ func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath s // Extract file from tar stream extractErr := extractTarFile(reader, localPath) + // Close reader to unblock the goroutine if extraction fails/completes + reader.Close() + // Wait for goroutine to complete and get its error execErr := <-execErrCh + // Log stderr if there's any output + if stderr.Len() > 0 { + logger.Debug().Str("stderr", stderr.String()).Msg("CopyFromPod tar stderr") + } + if extractErr != nil { + logger.Debug().Err(extractErr).Str("stderr", stderr.String()).Msg("CopyFromPod extract failed") return extractErr } if execErr != nil { + // Ignore "closed pipe" error when extraction succeeded - this happens when we close + // the reader after extracting the file but before tar finishes streaming + if isClosedPipeError(execErr) { + return nil + } + // Check for context timeout + if copyCtx.Err() == context.DeadlineExceeded { + return fmt.Errorf("copy timed out after 60s") + } return fmt.Errorf("tar exec failed: %s - %w", stderr.String(), execErr) } return nil } +// isClosedPipeError checks if the error is a "closed pipe" error which is expected +// when we close the reader early after successfully extracting the file +func isClosedPipeError(err error) bool { + if err == nil { + return false + } + return strings.Contains(err.Error(), "closed pipe") +} + // extractTarFile extracts a single file from a tar stream func extractTarFile(reader io.Reader, destPath string) error { tr := tar.NewReader(reader) + entryCount := 0 for { header, err := tr.Next() if err == io.EOF { - return fmt.Errorf("file not found in tar archive") + return fmt.Errorf("file not found in tar archive (found %d entries)", entryCount) } if err != nil { return fmt.Errorf("failed to read tar: %w", err) } - if header.Typeflag == tar.TypeReg { + entryCount++ + + // Accept both TypeReg ('0') and TypeRegA ('\x00') for compatibility + if header.Typeflag == tar.TypeReg || header.Typeflag == tar.TypeRegA { outFile, err := os.Create(destPath) if err != nil { return fmt.Errorf("failed to create file: %w", err) From 0d03b60c6195cf11ce5d22ff1910771d979d5268 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Tue, 27 Jan 2026 22:32:17 +0200 Subject: [PATCH 07/16] lint + fixes --- go.mod | 2 +- internal/cli/local/remote/copy_recording.go | 164 ++++++++++++------- internal/cli/local/remote/list.go | 52 ++++-- internal/cli/local/remote/list_recordings.go | 56 ++++--- internal/cli/local/remote/remote.go | 19 ++- internal/cli/local/remote/start.go | 135 ++++++++------- internal/cli/local/remote/stop.go | 16 +- internal/local/chart/kube.go | 16 +- 8 files changed, 284 insertions(+), 176 deletions(-) diff --git a/go.mod b/go.mod index 68d5f67..e9f8ebf 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( k8s.io/api v0.31.1 k8s.io/apimachinery v0.31.1 k8s.io/client-go v0.31.1 + k8s.io/utils v0.0.0-20240902221715-702e33fdd3c3 ) require ( @@ -170,7 +171,6 @@ require ( k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect k8s.io/kubectl v0.31.0 // indirect - k8s.io/utils v0.0.0-20240902221715-702e33fdd3c3 // indirect oras.land/oras-go v1.2.5 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/kustomize/api v0.17.2 // indirect diff --git a/internal/cli/local/remote/copy_recording.go b/internal/cli/local/remote/copy_recording.go index f669e08..f6e4969 100644 --- a/internal/cli/local/remote/copy_recording.go +++ b/internal/cli/local/remote/copy_recording.go @@ -1,6 +1,8 @@ package remote import ( + "context" + "errors" "fmt" "os" "path/filepath" @@ -13,15 +15,32 @@ import ( "github.com/weka/gohomecli/internal/utils" ) -// safeFilenamePattern allows safe characters for recording filenames -var safeFilenamePattern = regexp.MustCompile(`^[a-zA-Z0-9_\-:.]+\.cast$`) +const outputDirPerms = 0o750 -type copyRecordingOptions struct { - recording string - clusterID string - all bool - output string -} +var ( + // safeFilenamePattern allows safe characters for recording filenames + safeFilenamePattern = regexp.MustCompile(`^[a-zA-Z0-9_\-:.]+\.cast$`) + + // ErrMissingCopyFilter is returned when no filter is specified for copy-recording command + ErrMissingCopyFilter = errors.New("must specify one of: --recording, --cluster-id, or --all") + + // ErrInvalidFilename is returned when a recording filename contains unsafe characters + ErrInvalidFilename = errors.New("invalid recording filename: must be a .cast file with safe characters") +) + +type ( + copyRecordingOptions struct { + recording string + clusterID string + output string + all bool + } + + // recordingNotFoundError is returned when a specific recording cannot be found. + recordingNotFoundError struct { + name string + } +) func newCopyRecordingCmd() *cobra.Command { opts := ©RecordingOptions{} @@ -38,16 +57,14 @@ Examples: homecli remote-access copy-recording --cluster-id "550e8400-e29b-41d4-a716-446655440000" --output /tmp/ homecli remote-access copy-recording --all --output /tmp/ `, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { return copyRecordingRun(cmd, opts) }, - PreRunE: func(cmd *cobra.Command, args []string) error { + PreRunE: func(_ *cobra.Command, _ []string) error { if opts.recording == "" && opts.clusterID == "" && !opts.all { - return fmt.Errorf("must specify one of: --recording, --cluster-id, or --all") - } - if opts.output == "" { - return fmt.Errorf("--output is required") + return ErrMissingCopyFilter } + return nil }, } @@ -56,7 +73,8 @@ Examples: cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Copy all recordings for a cluster") cmd.Flags().BoolVar(&opts.all, "all", false, "Copy all recordings") cmd.Flags().StringVarP(&opts.output, "output", "o", "", "Local destination directory (required)") - _ = cmd.MarkFlagRequired("output") + //nolint:errcheck,gosec // cobra returns error only for unknown flags, which won't happen with our flag + cmd.MarkFlagRequired("output") return cmd } @@ -65,7 +83,7 @@ func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { ctx := cmd.Context() // Ensure output directory exists - if err := os.MkdirAll(opts.output, 0o755); err != nil { + if err := os.MkdirAll(opts.output, outputDirPerms); err != nil { return fmt.Errorf("failed to create output directory: %w", err) } @@ -76,49 +94,14 @@ func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { } // Determine which files to copy - var filesToCopy []RecordingInfo - - if opts.recording != "" { - // Validate inputs to prevent path traversal - if !safeFilenamePattern.MatchString(opts.recording) { - return fmt.Errorf("invalid recording filename: must be a .cast file with safe characters") - } - if opts.clusterID != "" { - if _, parseErr := uuid.Parse(opts.clusterID); parseErr != nil { - return fmt.Errorf("invalid cluster ID: must be a valid UUID") - } - // Cluster ID explicitly provided - filesToCopy = []RecordingInfo{{ - Filename: opts.recording, - ClusterID: opts.clusterID, - }} - } else { - // No cluster ID provided - search for the recording to find its cluster ID - allRecordings, listErr := listRecordings(ctx, execClient, "") - if listErr != nil { - return fmt.Errorf("failed to list recordings: %w", listErr) - } - for _, r := range allRecordings { - if r.Filename == opts.recording { - filesToCopy = []RecordingInfo{r} - break - } - } - if len(filesToCopy) == 0 { - return fmt.Errorf("recording not found: %s", opts.recording) - } - } - } else { - // List by cluster ID or all (empty clusterID = all) - // listRecordings validates clusterID internally - filesToCopy, err = listRecordings(ctx, execClient, opts.clusterID) - if err != nil { - return fmt.Errorf("failed to list recordings: %w", err) - } + filesToCopy, err := resolveFilesToCopy(ctx, execClient, opts) + if err != nil { + return err } if len(filesToCopy) == 0 { utils.UserNote("No matching recordings found") + return nil } @@ -140,6 +123,7 @@ func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { if err := execClient.CopyFromPod(ctx, remotePath, dstPath); err != nil { utils.UserWarning("Failed to copy %s: %v", localFilename, err) + continue } @@ -148,5 +132,75 @@ func copyRecordingRun(cmd *cobra.Command, opts *copyRecordingOptions) error { } utils.UserNote("Successfully copied %d/%d recording(s)", copiedCount, len(filesToCopy)) + return nil } + +// resolveFilesToCopy determines which recording files to copy based on options. +func resolveFilesToCopy( + ctx context.Context, + execClient *chart.K8sExecClient, + opts *copyRecordingOptions, +) ([]RecordingInfo, error) { + if opts.recording == "" { + // List by cluster ID or all (empty clusterID = all) + // listRecordings validates clusterID internally + return listRecordings(ctx, execClient, opts.clusterID) + } + + return resolveSpecificRecording(ctx, execClient, opts) +} + +// resolveSpecificRecording finds a specific recording by name. +func resolveSpecificRecording( + ctx context.Context, + execClient *chart.K8sExecClient, + opts *copyRecordingOptions, +) ([]RecordingInfo, error) { + // Validate inputs to prevent path traversal + if !safeFilenamePattern.MatchString(opts.recording) { + return nil, ErrInvalidFilename + } + + if opts.clusterID != "" { + return resolveWithClusterID(opts) + } + + return searchForRecording(ctx, execClient, opts.recording) +} + +// resolveWithClusterID returns recording info when cluster ID is explicitly provided. +func resolveWithClusterID(opts *copyRecordingOptions) ([]RecordingInfo, error) { + if _, parseErr := uuid.Parse(opts.clusterID); parseErr != nil { + return nil, ErrInvalidClusterID + } + + return []RecordingInfo{{ + Filename: opts.recording, + ClusterID: opts.clusterID, + }}, nil +} + +// searchForRecording searches all recordings to find the one matching the filename. +func searchForRecording( + ctx context.Context, + execClient *chart.K8sExecClient, + recordingName string, +) ([]RecordingInfo, error) { + allRecordings, listErr := listRecordings(ctx, execClient, "") + if listErr != nil { + return nil, fmt.Errorf("failed to list recordings: %w", listErr) + } + + for _, r := range allRecordings { + if r.Filename == recordingName { + return []RecordingInfo{r}, nil + } + } + + return nil, fmt.Errorf("recording not found: %w", &recordingNotFoundError{name: recordingName}) +} + +func (e *recordingNotFoundError) Error() string { + return e.name +} diff --git a/internal/cli/local/remote/list.go b/internal/cli/local/remote/list.go index 35691ac..1d912f7 100644 --- a/internal/cli/local/remote/list.go +++ b/internal/cli/local/remote/list.go @@ -13,9 +13,24 @@ import ( "github.com/weka/gohomecli/internal/utils" ) -type listOptions struct { - outputFormat string -} +const ( + minutesPerHour = 60 + hoursPerDay = 24 +) + +type ( + listOptions struct { + outputFormat string + } + + // SessionInfo represents information about an active remote session + SessionInfo struct { + SessionID string `json:"sessionId" yaml:"sessionId"` + ClusterID string `json:"clusterId" yaml:"clusterId"` + ClusterName string `json:"clusterName" yaml:"clusterName"` + Duration string `json:"duration" yaml:"duration"` + } +) func newListCmd() *cobra.Command { opts := &listOptions{} @@ -30,7 +45,7 @@ Examples: homecli remote-access list --output json # List sessions in JSON format homecli remote-access list --output yaml # List sessions in YAML format `, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { return listRun(cmd, opts) }, } @@ -40,14 +55,6 @@ Examples: return cmd } -// SessionInfo represents information about an active remote session -type SessionInfo struct { - SessionID string `json:"sessionId" yaml:"sessionId"` - ClusterID string `json:"clusterId" yaml:"clusterId"` - ClusterName string `json:"clusterName" yaml:"clusterName"` - Duration string `json:"duration" yaml:"duration"` -} - func listRun(cmd *cobra.Command, opts *listOptions) error { ctx := cmd.Context() @@ -67,12 +74,14 @@ func listRun(cmd *cobra.Command, opts *listOptions) error { if len(pods.Items) == 0 { utils.UserNote("No active remote sessions") + return nil } // Build session info list sessions := make([]SessionInfo, 0, len(pods.Items)) - for _, pod := range pods.Items { + for i := range pods.Items { + pod := &pods.Items[i] duration := formatDuration(pod.CreationTimestamp.Time) sessions = append(sessions, SessionInfo{ SessionID: pod.Labels["session-id"], @@ -102,13 +111,15 @@ func formatDuration(creationTime time.Time) string { if duration < time.Hour { return fmt.Sprintf("%dm", int(duration.Minutes())) } - if duration < 24*time.Hour { + if duration < hoursPerDay*time.Hour { hours := int(duration.Hours()) - minutes := int(duration.Minutes()) % 60 + minutes := int(duration.Minutes()) % minutesPerHour + return fmt.Sprintf("%dh%dm", hours, minutes) } - days := int(duration.Hours()) / 24 - hours := int(duration.Hours()) % 24 + days := int(duration.Hours()) / hoursPerDay + hours := int(duration.Hours()) % hoursPerDay + return fmt.Sprintf("%dd%dh", days, hours) } @@ -118,6 +129,7 @@ func outputSessionsAsJSON(sessions []SessionInfo) error { return err } utils.UserOutputJSON(output) + return nil } @@ -127,6 +139,7 @@ func outputSessionsAsYAML(sessions []SessionInfo) error { return err } fmt.Println(string(output)) + return nil } @@ -139,9 +152,10 @@ func outputSessionsAsTable(sessions []SessionInfo) error { index++ // Truncate cluster ID for display clusterIDDisplay := s.ClusterID - if len(clusterIDDisplay) > 36 { + if len(clusterIDDisplay) > 36 { //nolint:mnd // UUID length clusterIDDisplay = clusterIDDisplay[:36] } + return []string{ s.SessionID, clusterIDDisplay, @@ -149,7 +163,9 @@ func outputSessionsAsTable(sessions []SessionInfo) error { s.Duration, } } + return nil }) + return nil } diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go index e18acbe..a9baf5c 100644 --- a/internal/cli/local/remote/list_recordings.go +++ b/internal/cli/local/remote/list_recordings.go @@ -3,6 +3,7 @@ package remote import ( "context" "encoding/json" + "errors" "fmt" "strconv" "strings" @@ -20,12 +21,26 @@ const ( recordingsSidecarLabel = "app=remote-access-recordings-sidecar" recordingsSidecarContainer = "sidecar" recordingsPath = "/recordings" + statOutputParts = 3 // number of parts in stat output: path|size|mtime ) -type listRecordingsOptions struct { - clusterID string - outputFormat string -} +// ErrInvalidClusterID is returned when a cluster ID is not a valid UUID +var ErrInvalidClusterID = errors.New("invalid cluster ID: must be a valid UUID") + +type ( + listRecordingsOptions struct { + clusterID string + outputFormat string + } + + // RecordingInfo represents information about a recording file + RecordingInfo struct { + Filename string `json:"filename" yaml:"filename"` + ClusterID string `json:"clusterId,omitempty" yaml:"clusterId,omitempty"` + Size int64 `json:"size" yaml:"size"` + ModTime int64 `json:"modTime" yaml:"modTime"` + } +) func newListRecordingsCmd() *cobra.Command { opts := &listRecordingsOptions{} @@ -42,7 +57,7 @@ Examples: homecli remote-access list-recordings --cluster-id 550e8400-e29b-41d4-a716-446655440000 homecli remote-access list-recordings --output json `, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { return listRecordingsRun(cmd, opts) }, } @@ -53,14 +68,6 @@ Examples: return cmd } -// RecordingInfo represents information about a recording file -type RecordingInfo struct { - Filename string `json:"filename" yaml:"filename"` - Size int64 `json:"size" yaml:"size"` - ModTime int64 `json:"modTime" yaml:"modTime"` - ClusterID string `json:"clusterId,omitempty" yaml:"clusterId,omitempty"` -} - func listRecordingsRun(cmd *cobra.Command, opts *listRecordingsOptions) error { ctx := cmd.Context() @@ -77,6 +84,7 @@ func listRecordingsRun(cmd *cobra.Command, opts *listRecordingsOptions) error { if len(recordings) == 0 { utils.UserNote("No recordings found") + return nil } @@ -97,7 +105,7 @@ func listRecordings(ctx context.Context, client *chart.K8sExecClient, clusterID if clusterID != "" { // Validate clusterID is a valid UUID to prevent shell injection if _, err := uuid.Parse(clusterID); err != nil { - return nil, fmt.Errorf("invalid cluster ID: must be a valid UUID") + return nil, ErrInvalidClusterID } searchPath = fmt.Sprintf("%s/%s", recordingsPath, clusterID) } @@ -124,9 +132,8 @@ func listRecordings(ctx context.Context, client *chart.K8sExecClient, clusterID // parseStatOutput parses the output of stat command into RecordingInfo structs // Path format: /recordings/filename.cast or /recordings/clusterID/filename.cast func parseStatOutput(output string) []RecordingInfo { - var recordings []RecordingInfo - lines := strings.Split(strings.TrimSpace(output), "\n") + recordings := make([]RecordingInfo, 0, len(lines)) for _, line := range lines { line = strings.TrimSpace(line) if line == "" { @@ -134,7 +141,7 @@ func parseStatOutput(output string) []RecordingInfo { } parts := strings.Split(line, "|") - if len(parts) != 3 { + if len(parts) != statOutputParts { continue } @@ -155,8 +162,12 @@ func parseStatOutput(output string) []RecordingInfo { filename = strings.Join(pathParts[1:], "/") } - size, _ := strconv.ParseInt(parts[1], 10, 64) - mtime, _ := strconv.ParseInt(parts[2], 10, 64) + size, sizeErr := strconv.ParseInt(parts[1], 10, 64) + mtime, mtimeErr := strconv.ParseInt(parts[2], 10, 64) + if sizeErr != nil || mtimeErr != nil { + // Skip malformed entries - log at debug level + continue + } recordings = append(recordings, RecordingInfo{ Filename: filename, @@ -175,6 +186,7 @@ func outputRecordingsAsJSON(recordings []RecordingInfo) error { return err } utils.UserOutputJSON(output) + return nil } @@ -184,6 +196,7 @@ func outputRecordingsAsYAML(recordings []RecordingInfo) error { return err } fmt.Println(string(output)) + return nil } @@ -198,6 +211,7 @@ func outputRecordingsAsTable(recordings []RecordingInfo) error { if clusterID == "" { clusterID = "-" } + return []string{ r.Filename, formatSize(r.Size), @@ -205,8 +219,10 @@ func outputRecordingsAsTable(recordings []RecordingInfo) error { clusterID, } } + return nil }) + return nil } @@ -221,11 +237,13 @@ func formatSize(bytes int64) string { div *= unit exp++ } + return fmt.Sprintf("%.1f %cB", float64(bytes)/float64(div), "KMGTPE"[exp]) } // formatTime formats unix timestamp into human-readable format func formatTime(timestamp int64) string { t := time.Unix(timestamp, 0) + return t.Format("2006-01-02 15:04") } diff --git a/internal/cli/local/remote/remote.go b/internal/cli/local/remote/remote.go index 5cc6f59..76e2672 100644 --- a/internal/cli/local/remote/remote.go +++ b/internal/cli/local/remote/remote.go @@ -8,17 +8,20 @@ import ( "github.com/weka/gohomecli/internal/utils" ) -var logger = utils.GetLogger("Remote") +var ( + logger = utils.GetLogger("Remote") //nolint:gochecknoglobals // standard pattern -// RemoteAccessGroup is the command group for remote access commands -var RemoteAccessGroup = cobra.Group{ - ID: "remote-access", - Title: "Remote Access Commands", -} + // RemoteAccessGroup is the command group for remote access commands + RemoteAccessGroup = cobra.Group{ //nolint:gochecknoglobals // cobra pattern + ID: "remote-access", + Title: "Remote Access Commands", + } -// Cli is the hooks.Cli instance for remote access commands -var Cli hooks.Cli + // Cli is the hooks.Cli instance for remote access commands + Cli hooks.Cli //nolint:gochecknoglobals // cobra pattern +) +//nolint:gochecknoinits // cobra CLI pattern func init() { Cli.AddHook(func(appCmd *cobra.Command) { appCmd.AddGroup(&RemoteAccessGroup) diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go index f13ac63..324915f 100644 --- a/internal/cli/local/remote/start.go +++ b/internal/cli/local/remote/start.go @@ -4,11 +4,13 @@ import ( "crypto/rand" "encoding/hex" "fmt" + "strconv" "strings" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "github.com/weka/gohomecli/internal/env" "github.com/weka/gohomecli/internal/local/chart" @@ -20,23 +22,25 @@ const ( remoteAccessValue = "remote-access" remoteAccessConfigLabel = "app=remote-access-config" sessionRecordingsPVCLabel = "app=remote-access-recordings" + sessionIDBytes = 3 // 3 bytes = 6 hex chars + sharedSocketPath = "/shared/tmate.socket" ) type startOptions struct { - clusterID string - clusterName string - sshKeysPath string - cloudURL string - hostName string - terminalCols int - terminalLines int - debug bool // Tmate server flags (optional overrides - tmate.py has built-in config for known cloud URLs) tmateServerHost string tmateServerPort string tmateServerRSA string tmateServerEd25519 string tmateServerECDSA string + clusterID string + clusterName string + sshKeysPath string + cloudURL string + hostName string + terminalCols int + terminalLines int + debug bool } func newStartCmd() *cobra.Command { @@ -52,7 +56,7 @@ Examples: homecli remote-access start --cluster-id "550e8400-..." --cluster-name "prod" --ssh-keys-path "/root/.ssh" # Start a session with cloud URL from config homecli remote-access start --cluster-id "550e8400-..." --cluster-name "prod" --ssh-keys-path "/root/.ssh" --tmate-server-host "tmate.example.com" --tmate-server-port "22" --tmate-server-rsa-fingerprint "1234567890" --tmate-server-ed25519-fingerprint "1234567890" --tmate-server-ecdsa-fingerprint "1234567890" # Start a session with custom tmate server `, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { return startRun(cmd, opts) }, } @@ -61,9 +65,9 @@ Examples: cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Cluster GUID (required)") cmd.Flags().StringVar(&opts.clusterName, "cluster-name", "", "Human-readable cluster name (required)") cmd.Flags().StringVar(&opts.sshKeysPath, "ssh-keys-path", "", "Host path to SSH keys directory (required)") - _ = cmd.MarkFlagRequired("cluster-id") - _ = cmd.MarkFlagRequired("cluster-name") - _ = cmd.MarkFlagRequired("ssh-keys-path") + _ = cmd.MarkFlagRequired("cluster-id") //nolint:errcheck // flag exists + _ = cmd.MarkFlagRequired("cluster-name") //nolint:errcheck // flag exists + _ = cmd.MarkFlagRequired("ssh-keys-path") //nolint:errcheck // flag exists // Optional flags cmd.Flags().StringVar(&opts.cloudURL, "cloud-url", "", "Cloud Weka Home URL (default: from config)") @@ -76,7 +80,9 @@ Examples: cmd.Flags().StringVar(&opts.tmateServerHost, "tmate-server-host", "", "Override tmate SSH server hostname") cmd.Flags().StringVar(&opts.tmateServerPort, "tmate-server-port", "", "Override tmate SSH server port") cmd.Flags().StringVar(&opts.tmateServerRSA, "tmate-server-rsa-fingerprint", "", "Override RSA fingerprint") - cmd.Flags().StringVar(&opts.tmateServerEd25519, "tmate-server-ed25519-fingerprint", "", "Override Ed25519 fingerprint") + cmd.Flags().StringVar( + &opts.tmateServerEd25519, "tmate-server-ed25519-fingerprint", "", "Override Ed25519 fingerprint", + ) cmd.Flags().StringVar(&opts.tmateServerECDSA, "tmate-server-ecdsa-fingerprint", "", "Override ECDSA fingerprint") return cmd @@ -163,16 +169,14 @@ func buildWebhookBaseURLs(cloudURL, localAPIURL string) string { } func generateShortID() string { - bytes := make([]byte, 3) // 3 bytes = 6 hex chars - _, _ = rand.Read(bytes) - return hex.EncodeToString(bytes) -} + b := make([]byte, sessionIDBytes) + _, _ = rand.Read(b) //nolint:errcheck // crypto/rand.Read error indicates serious system issue -func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, image string, opts *startOptions) *corev1.Pod { - const sharedSocketPath = "/shared/tmate.socket" + return hex.EncodeToString(b) +} - // Environment variables for tmate container - tmateEnv := []corev1.EnvVar{ +func buildTmateEnv(webhookURLs, cloudURL string, opts *startOptions) []corev1.EnvVar { + envVars := []corev1.EnvVar{ {Name: "CLUSTER_ID", Value: opts.clusterID}, {Name: "CLUSTER_NAME", Value: opts.clusterName}, {Name: "SHARED_SOCKET", Value: sharedSocketPath}, @@ -180,61 +184,78 @@ func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, image {Name: "WEBHOOK_URLS", Value: webhookURLs}, } - // Add HOST_NAME only if explicitly provided (tmate.py uses socket.gethostname() as default) if opts.hostName != "" { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "HOST_NAME", Value: opts.hostName}) + envVars = append(envVars, corev1.EnvVar{Name: "HOST_NAME", Value: opts.hostName}) } - - // Add terminal dimensions only if explicitly provided (tmate.py defaults to 158x35) if opts.terminalCols > 0 { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TERMINAL_COLS", Value: fmt.Sprintf("%d", opts.terminalCols)}) + envVars = append(envVars, corev1.EnvVar{Name: "TERMINAL_COLS", Value: strconv.Itoa(opts.terminalCols)}) } if opts.terminalLines > 0 { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TERMINAL_LINES", Value: fmt.Sprintf("%d", opts.terminalLines)}) + envVars = append(envVars, corev1.EnvVar{Name: "TERMINAL_LINES", Value: strconv.Itoa(opts.terminalLines)}) } - - // Add tmate server overrides only if provided (tmate.py will use these if CLOUD_URL not in its CONFIG) if opts.tmateServerHost != "" { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_HOST", Value: opts.tmateServerHost}) + envVars = append(envVars, corev1.EnvVar{Name: "TMATE_SERVER_HOST", Value: opts.tmateServerHost}) } if opts.tmateServerPort != "" { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_PORT", Value: opts.tmateServerPort}) + envVars = append(envVars, corev1.EnvVar{Name: "TMATE_SERVER_PORT", Value: opts.tmateServerPort}) } if opts.tmateServerRSA != "" { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_RSA_FINGERPRINT", Value: opts.tmateServerRSA}) + envVars = append(envVars, corev1.EnvVar{Name: "TMATE_SERVER_RSA_FINGERPRINT", Value: opts.tmateServerRSA}) } if opts.tmateServerEd25519 != "" { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_ED25519_FINGERPRINT", Value: opts.tmateServerEd25519}) + envVars = append( + envVars, corev1.EnvVar{Name: "TMATE_SERVER_ED25519_FINGERPRINT", Value: opts.tmateServerEd25519}, + ) } if opts.tmateServerECDSA != "" { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "TMATE_SERVER_ECDSA_FINGERPRINT", Value: opts.tmateServerECDSA}) + envVars = append(envVars, corev1.EnvVar{Name: "TMATE_SERVER_ECDSA_FINGERPRINT", Value: opts.tmateServerECDSA}) } - if opts.debug { - tmateEnv = append(tmateEnv, corev1.EnvVar{Name: "DEBUG", Value: "1"}) + envVars = append(envVars, corev1.EnvVar{Name: "DEBUG", Value: "1"}) } - // Volume mounts for tmate container - tmateMounts := []corev1.VolumeMount{ - {Name: "shared-socket", MountPath: "/shared"}, - {Name: "ssh-keys", MountPath: "/root/.ssh", ReadOnly: true}, + return envVars +} + +func buildPodVolumes(recordingsPVCName, sshKeysPath string) []corev1.Volume { + return []corev1.Volume{ + {Name: "shared-socket", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}, + {Name: "recordings", VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: recordingsPVCName}, + }}, + {Name: "ssh-keys", VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{Path: sshKeysPath}, + }}, + {Name: "dev-pts", VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{Path: "/dev/pts"}, + }}, } +} - // Recorder environment +func buildSessionPod( + sessionID, webhookURLs, cloudURL, recordingsPVCName, image string, + opts *startOptions, +) *corev1.Pod { + tmateEnv := buildTmateEnv(webhookURLs, cloudURL, opts) recorderEnv := []corev1.EnvVar{ {Name: "SHARED_SOCKET", Value: sharedSocketPath}, {Name: "CLUSTER_ID", Value: opts.clusterID}, } - // Volume mounts for recorder container + tmateMounts := []corev1.VolumeMount{ + {Name: "shared-socket", MountPath: "/shared"}, + {Name: "ssh-keys", MountPath: "/root/.ssh", ReadOnly: true}, + {Name: "dev-pts", MountPath: "/dev/pts"}, + } recorderMounts := []corev1.VolumeMount{ {Name: "shared-socket", MountPath: "/shared"}, {Name: "recordings", MountPath: "/recordings"}, + {Name: "dev-pts", MountPath: "/dev/pts"}, } return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("remote-session-%s", sessionID), + Name: "remote-session-" + sessionID, Namespace: chart.ReleaseNamespace, Labels: map[string]string{ remoteAccessLabel: remoteAccessValue, @@ -244,7 +265,8 @@ func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, image }, }, Spec: corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyAlways, + ShareProcessNamespace: ptr.To(true), + RestartPolicy: corev1.RestartPolicyAlways, Containers: []corev1.Container{ { Name: "tmate", @@ -261,30 +283,7 @@ func buildSessionPod(sessionID, webhookURLs, cloudURL, recordingsPVCName, image VolumeMounts: recorderMounts, }, }, - Volumes: []corev1.Volume{ - { - Name: "shared-socket", - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, - }, - { - Name: "recordings", - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: recordingsPVCName, - }, - }, - }, - { - Name: "ssh-keys", - VolumeSource: corev1.VolumeSource{ - HostPath: &corev1.HostPathVolumeSource{ - Path: opts.sshKeysPath, - }, - }, - }, - }, + Volumes: buildPodVolumes(recordingsPVCName, opts.sshKeysPath), }, } } diff --git a/internal/cli/local/remote/stop.go b/internal/cli/local/remote/stop.go index 1d54225..3645b2c 100644 --- a/internal/cli/local/remote/stop.go +++ b/internal/cli/local/remote/stop.go @@ -1,6 +1,7 @@ package remote import ( + "errors" "fmt" "github.com/spf13/cobra" @@ -10,6 +11,9 @@ import ( "github.com/weka/gohomecli/internal/utils" ) +// ErrMissingStopFilter is returned when no filter is specified for stop command +var ErrMissingStopFilter = errors.New("must specify one of: --session-id, --cluster-id, or --all") + type stopOptions struct { sessionID string clusterID string @@ -30,13 +34,14 @@ Examples: homecli remote-access stop --cluster-id 550e8400-e29b-41d4-a716-446655440000 # Stop all sessions for a cluster homecli remote-access stop --all # Stop all active sessions `, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { return stopRun(cmd, opts) }, - PreRunE: func(cmd *cobra.Command, args []string) error { + PreRunE: func(_ *cobra.Command, _ []string) error { if opts.sessionID == "" && opts.clusterID == "" && !opts.all { - return fmt.Errorf("must specify one of: --session-id, --cluster-id, or --all") + return ErrMissingStopFilter } + return nil }, } @@ -77,12 +82,14 @@ func stopRun(cmd *cobra.Command, opts *stopOptions) error { if len(pods.Items) == 0 { utils.UserNote("No matching sessions found") + return nil } // Delete each pod stoppedCount := 0 - for _, pod := range pods.Items { + for i := range pods.Items { + pod := &pods.Items[i] sessionID := pod.Labels["session-id"] logger.Info().Str("pod", pod.Name).Str("sessionID", sessionID).Msg("Stopping session pod") @@ -95,5 +102,6 @@ func stopRun(cmd *cobra.Command, opts *stopOptions) error { } utils.UserNote("Stopped %d session(s)", stoppedCount) + return nil } diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index 06b024b..1db8343 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "context" + "errors" "fmt" "io" "net" @@ -169,6 +170,7 @@ func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath s if extractErr != nil { logger.Debug().Err(extractErr).Str("stderr", stderr.String()).Msg("CopyFromPod extract failed") + return extractErr } if execErr != nil { @@ -178,11 +180,13 @@ func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath s return nil } // Check for context timeout - if copyCtx.Err() == context.DeadlineExceeded { - return fmt.Errorf("copy timed out after 60s") + if errors.Is(copyCtx.Err(), context.DeadlineExceeded) { + return errors.New("copy timed out after 60s") } + return fmt.Errorf("tar exec failed: %s - %w", stderr.String(), execErr) } + return nil } @@ -192,6 +196,7 @@ func isClosedPipeError(err error) bool { if err == nil { return false } + return strings.Contains(err.Error(), "closed pipe") } @@ -217,6 +222,7 @@ func extractTarFile(reader io.Reader, destPath string) error { } _, err = io.Copy(outFile, tr) outFile.Close() + return err } } @@ -453,7 +459,11 @@ func GetConfigMapData(ctx context.Context, labelSelector, key string) (string, e } if len(configMaps.Items) == 0 { - return "", fmt.Errorf("no ConfigMap found with label selector %q in namespace %s", labelSelector, ReleaseNamespace) + return "", fmt.Errorf( + "no ConfigMap found with label selector %q in namespace %s", + labelSelector, + ReleaseNamespace, + ) } value, ok := configMaps.Items[0].Data[key] From e9b381833ae0d04cfe9ac3a8e875b6e6abbcb25d Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Tue, 27 Jan 2026 23:01:35 +0200 Subject: [PATCH 08/16] fixes --- internal/cli/local/remote/copy_recording.go | 3 +- internal/cli/local/remote/list.go | 12 ++- internal/cli/local/remote/list_recordings.go | 11 ++- internal/cli/local/remote/remote.go | 20 +++++ internal/cli/local/remote/start.go | 80 +++++++++++++++++++- internal/local/chart/kube.go | 76 +++++-------------- 6 files changed, 137 insertions(+), 65 deletions(-) diff --git a/internal/cli/local/remote/copy_recording.go b/internal/cli/local/remote/copy_recording.go index f6e4969..2837290 100644 --- a/internal/cli/local/remote/copy_recording.go +++ b/internal/cli/local/remote/copy_recording.go @@ -73,8 +73,7 @@ Examples: cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Copy all recordings for a cluster") cmd.Flags().BoolVar(&opts.all, "all", false, "Copy all recordings") cmd.Flags().StringVarP(&opts.output, "output", "o", "", "Local destination directory (required)") - //nolint:errcheck,gosec // cobra returns error only for unknown flags, which won't happen with our flag - cmd.MarkFlagRequired("output") + _ = cmd.MarkFlagRequired("output") //nolint:errcheck // flag exists return cmd } diff --git a/internal/cli/local/remote/list.go b/internal/cli/local/remote/list.go index 1d912f7..5ec1628 100644 --- a/internal/cli/local/remote/list.go +++ b/internal/cli/local/remote/list.go @@ -14,8 +14,9 @@ import ( ) const ( - minutesPerHour = 60 - hoursPerDay = 24 + minutesPerHour = 60 + hoursPerDay = 24 + uuidStringLength = 36 ) type ( @@ -45,6 +46,9 @@ Examples: homecli remote-access list --output json # List sessions in JSON format homecli remote-access list --output yaml # List sessions in YAML format `, + PreRunE: func(_ *cobra.Command, _ []string) error { + return validateOutputFormat(opts.outputFormat) + }, RunE: func(cmd *cobra.Command, _ []string) error { return listRun(cmd, opts) }, @@ -152,8 +156,8 @@ func outputSessionsAsTable(sessions []SessionInfo) error { index++ // Truncate cluster ID for display clusterIDDisplay := s.ClusterID - if len(clusterIDDisplay) > 36 { //nolint:mnd // UUID length - clusterIDDisplay = clusterIDDisplay[:36] + if len(clusterIDDisplay) > uuidStringLength { + clusterIDDisplay = clusterIDDisplay[:uuidStringLength] } return []string{ diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go index a9baf5c..ad4c60d 100644 --- a/internal/cli/local/remote/list_recordings.go +++ b/internal/cli/local/remote/list_recordings.go @@ -57,6 +57,9 @@ Examples: homecli remote-access list-recordings --cluster-id 550e8400-e29b-41d4-a716-446655440000 homecli remote-access list-recordings --output json `, + PreRunE: func(_ *cobra.Command, _ []string) error { + return validateOutputFormat(opts.outputFormat) + }, RunE: func(cmd *cobra.Command, _ []string) error { return listRecordingsRun(cmd, opts) }, @@ -119,8 +122,12 @@ func listRecordings(ctx context.Context, client *chart.K8sExecClient, clusterID // Find all .cast files recursively with stat info // Output format: /recordings/[clusterID/]filename.cast|size|mtime - output, err := client.Exec(ctx, "sh", "-c", - fmt.Sprintf(`find %s -name '*.cast' -type f -exec stat -c '%%n|%%s|%%Y' {} \;`, searchPath)) + output, err := client.Exec(ctx, + "find", searchPath, + "-name", "*.cast", + "-type", "f", + "-exec", "stat", "-c", "%n|%s|%Y", "{}", ";", + ) if err != nil { // Directory exists but find failed - this is a real error return nil, fmt.Errorf("failed to list recordings: %w", err) diff --git a/internal/cli/local/remote/remote.go b/internal/cli/local/remote/remote.go index 76e2672..69bdade 100644 --- a/internal/cli/local/remote/remote.go +++ b/internal/cli/local/remote/remote.go @@ -2,6 +2,9 @@ package remote import ( + "errors" + "fmt" + "github.com/spf13/cobra" "github.com/weka/gohomecli/internal/cli/app/hooks" @@ -19,6 +22,12 @@ var ( // Cli is the hooks.Cli instance for remote access commands Cli hooks.Cli //nolint:gochecknoglobals // cobra pattern + + // validOutputFormats defines the allowed output format values + validOutputFormats = []string{"table", "json", "yaml"} //nolint:gochecknoglobals // used by multiple commands + + // ErrInvalidOutputFormat is returned when an invalid output format is specified + ErrInvalidOutputFormat = errors.New("invalid output format") ) //nolint:gochecknoinits // cobra CLI pattern @@ -42,3 +51,14 @@ func init() { appCmd.AddCommand(remoteCmd) }) } + +// validateOutputFormat checks if the output format is valid +func validateOutputFormat(format string) error { + for _, valid := range validOutputFormats { + if format == valid { + return nil + } + } + + return fmt.Errorf("%w: %q (valid: table, json, yaml)", ErrInvalidOutputFormat, format) +} diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go index 324915f..7dd5acc 100644 --- a/internal/cli/local/remote/start.go +++ b/internal/cli/local/remote/start.go @@ -3,7 +3,10 @@ package remote import ( "crypto/rand" "encoding/hex" + "errors" "fmt" + "os" + "regexp" "strconv" "strings" @@ -17,6 +20,27 @@ import ( "github.com/weka/gohomecli/internal/utils" ) +// Kubernetes label value constraints. +const maxLabelValueLength = 63 + +// labelValuePattern matches valid Kubernetes label values. +// Must start and end with alphanumeric, can contain alphanumerics, dashes, underscores, and dots. +var labelValuePattern = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$`) + +var ( + // ErrSSHKeysPathNotDirectory is returned when the ssh-keys-path exists but is not a directory. + ErrSSHKeysPathNotDirectory = errors.New("ssh-keys-path must be a directory") + + // ErrClusterNameTooLong is returned when the cluster name exceeds the Kubernetes label value limit. + ErrClusterNameTooLong = errors.New("cluster-name exceeds maximum length of 63 characters") + + // ErrClusterNameInvalid is returned when the cluster name contains invalid characters for a Kubernetes label. + ErrClusterNameInvalid = errors.New( + "cluster-name must start and end with alphanumeric characters, " + + "and contain only alphanumerics, dashes, underscores, or dots", + ) +) + const ( remoteAccessLabel = "app" remoteAccessValue = "remote-access" @@ -63,8 +87,8 @@ Examples: // Required flags cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Cluster GUID (required)") - cmd.Flags().StringVar(&opts.clusterName, "cluster-name", "", "Human-readable cluster name (required)") - cmd.Flags().StringVar(&opts.sshKeysPath, "ssh-keys-path", "", "Host path to SSH keys directory (required)") + cmd.Flags().StringVar(&opts.clusterName, "cluster-name", "", "Human-readable cluster name, max 63 chars, alphanumeric with dashes/underscores/dots (required)") + cmd.Flags().StringVar(&opts.sshKeysPath, "ssh-keys-path", "", "Host path to existing SSH keys directory, mounted as HostPath volume (required)") _ = cmd.MarkFlagRequired("cluster-id") //nolint:errcheck // flag exists _ = cmd.MarkFlagRequired("cluster-name") //nolint:errcheck // flag exists _ = cmd.MarkFlagRequired("ssh-keys-path") //nolint:errcheck // flag exists @@ -91,6 +115,16 @@ Examples: func startRun(cmd *cobra.Command, opts *startOptions) error { ctx := cmd.Context() + // Validate SSH keys path exists and is a directory + if err := validateSSHKeysPath(opts.sshKeysPath); err != nil { + return err + } + + // Validate cluster name for Kubernetes label compatibility + if err := validateClusterName(opts.clusterName); err != nil { + return err + } + // Resolve cloud URL from flag, config, or default cloudURL := opts.cloudURL if cloudURL == "" && env.CurrentSiteConfig != nil { @@ -168,6 +202,48 @@ func buildWebhookBaseURLs(cloudURL, localAPIURL string) string { return strings.Join(urls, ",") } +// validateSSHKeysPath validates that the SSH keys path exists and is a directory. +func validateSSHKeysPath(path string) error { + info, err := os.Stat(path) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("ssh-keys-path does not exist: %s", path) + } + + return fmt.Errorf("failed to access ssh-keys-path: %w", err) + } + + if !info.IsDir() { + return fmt.Errorf("%w: %s", ErrSSHKeysPathNotDirectory, path) + } + + return nil +} + +// validateClusterName validates that the cluster name is valid for use as a Kubernetes label value. +func validateClusterName(name string) error { + if len(name) > maxLabelValueLength { + return fmt.Errorf("%w (got %d characters)", ErrClusterNameTooLong, len(name)) + } + + // Single character names are valid if alphanumeric + if len(name) == 1 { + if !((name[0] >= 'a' && name[0] <= 'z') || + (name[0] >= 'A' && name[0] <= 'Z') || + (name[0] >= '0' && name[0] <= '9')) { + return fmt.Errorf("%w: %q", ErrClusterNameInvalid, name) + } + + return nil + } + + if !labelValuePattern.MatchString(name) { + return fmt.Errorf("%w: %q", ErrClusterNameInvalid, name) + } + + return nil +} + func generateShortID() string { b := make([]byte, sessionIDBytes) _, _ = rand.Read(b) //nolint:errcheck // crypto/rand.Read error indicates serious system issue diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index 1db8343..a618d0e 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -27,6 +27,7 @@ import ( const ( KubeConfigPath = "/etc/rancher/k3s/k3s.yaml" + copyFromPodTimeout = 10 * time.Minute ) // K8sClient holds a Kubernetes clientset and REST config @@ -136,12 +137,13 @@ func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath s return fmt.Errorf("failed to create executor: %w", err) } - // Add timeout to prevent indefinite hangs - copyCtx, cancel := context.WithTimeout(ctx, 60*time.Second) + // Add timeout to prevent indefinite hangs on large files or slow connections + copyCtx, cancel := context.WithTimeout(ctx, copyFromPodTimeout) defer cancel() // Use pipe for streaming (memory efficient for large files) reader, writer := io.Pipe() + defer reader.Close() // Ensure cleanup even on panic var stderr bytes.Buffer execErrCh := make(chan error, 1) @@ -180,8 +182,8 @@ func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath s return nil } // Check for context timeout - if errors.Is(copyCtx.Err(), context.DeadlineExceeded) { - return errors.New("copy timed out after 60s") + if errors.Is(execErr, context.DeadlineExceeded) { + return fmt.Errorf("copy timed out after %v: %w", copyFromPodTimeout, execErr) } return fmt.Errorf("tar exec failed: %s - %w", stderr.String(), execErr) @@ -220,10 +222,14 @@ func extractTarFile(reader io.Reader, destPath string) error { if err != nil { return fmt.Errorf("failed to create file: %w", err) } - _, err = io.Copy(outFile, tr) - outFile.Close() + defer outFile.Close() //nolint:errcheck // error on close after successful write is acceptable - return err + _, copyErr := io.Copy(outFile, tr) + if copyErr != nil { + return fmt.Errorf("failed to copy file: %w", copyErr) + } + + return nil } } } @@ -341,22 +347,12 @@ type PodInfo struct { // GetNonRuninngPods returns a list of non-running pods in the ReleaseNamespace namespace func GetNonRuninngPods(ctx context.Context) ([]PodInfo, error) { - kubeconfig, err := ReadKubeConfig(KubeConfigPath) - if err != nil { - return nil, err - } - - config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) - if err != nil { - return nil, err - } - - clientset, err := kubernetes.NewForConfig(config) + k8s, err := NewKubernetesClient() if err != nil { return nil, err } - pods, err := clientset.CoreV1().Pods(ReleaseNamespace).List(ctx, v1.ListOptions{}) + pods, err := k8s.Clientset.CoreV1().Pods(ReleaseNamespace).List(ctx, v1.ListOptions{}) if err != nil { return nil, err } @@ -405,22 +401,12 @@ func getContainerStatusReason(containerStatus corev1.ContainerStatus, podReason // GetPVCName returns the name of a PVC found by label selector func GetPVCName(ctx context.Context, labelSelector string) (string, error) { - kubeconfig, err := ReadKubeConfig(KubeConfigPath) - if err != nil { - return "", err - } - - config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) - if err != nil { - return "", err - } - - clientset, err := kubernetes.NewForConfig(config) + k8s, err := NewKubernetesClient() if err != nil { return "", err } - pvcs, err := clientset.CoreV1().PersistentVolumeClaims(ReleaseNamespace).List(ctx, v1.ListOptions{ + pvcs, err := k8s.Clientset.CoreV1().PersistentVolumeClaims(ReleaseNamespace).List(ctx, v1.ListOptions{ LabelSelector: labelSelector, }) if err != nil { @@ -436,22 +422,12 @@ func GetPVCName(ctx context.Context, labelSelector string) (string, error) { // GetConfigMapData returns a specific key's value from a ConfigMap found by label selector func GetConfigMapData(ctx context.Context, labelSelector, key string) (string, error) { - kubeconfig, err := ReadKubeConfig(KubeConfigPath) - if err != nil { - return "", err - } - - config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) - if err != nil { - return "", err - } - - clientset, err := kubernetes.NewForConfig(config) + k8s, err := NewKubernetesClient() if err != nil { return "", err } - configMaps, err := clientset.CoreV1().ConfigMaps(ReleaseNamespace).List(ctx, v1.ListOptions{ + configMaps, err := k8s.Clientset.CoreV1().ConfigMaps(ReleaseNamespace).List(ctx, v1.ListOptions{ LabelSelector: labelSelector, }) if err != nil { @@ -476,22 +452,12 @@ func GetConfigMapData(ctx context.Context, labelSelector, key string) (string, e // GetIngressAddress returns the address of the ingress in ReleaseNamespace namespace func GetIngressAddress(ctx context.Context) (string, error) { - kubeconfig, err := ReadKubeConfig(KubeConfigPath) - if err != nil { - return "", err - } - - config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) - if err != nil { - return "", err - } - - clientset, err := kubernetes.NewForConfig(config) + k8s, err := NewKubernetesClient() if err != nil { return "", err } - ingress, err := clientset.NetworkingV1(). + ingress, err := k8s.Clientset.NetworkingV1(). Ingresses(ReleaseNamespace). Get(ctx, "wekahome", v1.GetOptions{}) if err != nil { From 139eb6c8febacddda5064a135a810b9a673360e1 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Tue, 27 Jan 2026 23:08:28 +0200 Subject: [PATCH 09/16] pre-commit --- internal/cli/local/remote/start.go | 6 ++++-- internal/local/chart/kube.go | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go index 7dd5acc..f432294 100644 --- a/internal/cli/local/remote/start.go +++ b/internal/cli/local/remote/start.go @@ -87,8 +87,10 @@ Examples: // Required flags cmd.Flags().StringVar(&opts.clusterID, "cluster-id", "", "Cluster GUID (required)") - cmd.Flags().StringVar(&opts.clusterName, "cluster-name", "", "Human-readable cluster name, max 63 chars, alphanumeric with dashes/underscores/dots (required)") - cmd.Flags().StringVar(&opts.sshKeysPath, "ssh-keys-path", "", "Host path to existing SSH keys directory, mounted as HostPath volume (required)") + cmd.Flags(). + StringVar(&opts.clusterName, "cluster-name", "", "Human-readable cluster name, max 63 chars, alphanumeric with dashes/underscores/dots (required)") + cmd.Flags(). + StringVar(&opts.sshKeysPath, "ssh-keys-path", "", "Host path to existing SSH keys directory, mounted as HostPath volume (required)") _ = cmd.MarkFlagRequired("cluster-id") //nolint:errcheck // flag exists _ = cmd.MarkFlagRequired("cluster-name") //nolint:errcheck // flag exists _ = cmd.MarkFlagRequired("ssh-keys-path") //nolint:errcheck // flag exists diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index a618d0e..2249e54 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -26,7 +26,7 @@ import ( ) const ( - KubeConfigPath = "/etc/rancher/k3s/k3s.yaml" + KubeConfigPath = "/etc/rancher/k3s/k3s.yaml" copyFromPodTimeout = 10 * time.Minute ) From 6e143f5d16301e94176c69a5acdd753d9ee1359c Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Tue, 27 Jan 2026 23:38:30 +0200 Subject: [PATCH 10/16] fixes 2 --- internal/cli/local/remote/copy_recording.go | 56 ++++++++++---------- internal/cli/local/remote/list.go | 2 +- internal/cli/local/remote/list_recordings.go | 19 ++++--- internal/cli/local/remote/start.go | 32 +++++++---- internal/cli/local/remote/stop.go | 17 ++++++ internal/local/chart/kube.go | 8 ++- 6 files changed, 83 insertions(+), 51 deletions(-) diff --git a/internal/cli/local/remote/copy_recording.go b/internal/cli/local/remote/copy_recording.go index 2837290..f9d91b6 100644 --- a/internal/cli/local/remote/copy_recording.go +++ b/internal/cli/local/remote/copy_recording.go @@ -26,22 +26,18 @@ var ( // ErrInvalidFilename is returned when a recording filename contains unsafe characters ErrInvalidFilename = errors.New("invalid recording filename: must be a .cast file with safe characters") -) - -type ( - copyRecordingOptions struct { - recording string - clusterID string - output string - all bool - } - // recordingNotFoundError is returned when a specific recording cannot be found. - recordingNotFoundError struct { - name string - } + // ErrRecordingNotFound is returned when a specific recording cannot be found + ErrRecordingNotFound = errors.New("recording not found") ) +type copyRecordingOptions struct { + recording string + clusterID string + output string + all bool +} + func newCopyRecordingCmd() *cobra.Command { opts := ©RecordingOptions{} @@ -65,6 +61,18 @@ Examples: return ErrMissingCopyFilter } + // Validate recording filename to prevent path traversal + if opts.recording != "" && !safeFilenamePattern.MatchString(opts.recording) { + return ErrInvalidFilename + } + + // Validate cluster ID is a valid UUID + if opts.clusterID != "" { + if _, err := uuid.Parse(opts.clusterID); err != nil { + return ErrInvalidClusterID + } + } + return nil }, } @@ -156,28 +164,22 @@ func resolveSpecificRecording( execClient *chart.K8sExecClient, opts *copyRecordingOptions, ) ([]RecordingInfo, error) { - // Validate inputs to prevent path traversal - if !safeFilenamePattern.MatchString(opts.recording) { - return nil, ErrInvalidFilename - } + // Note: filename and clusterID validation is done in PreRunE if opts.clusterID != "" { - return resolveWithClusterID(opts) + return resolveWithClusterID(opts), nil } return searchForRecording(ctx, execClient, opts.recording) } // resolveWithClusterID returns recording info when cluster ID is explicitly provided. -func resolveWithClusterID(opts *copyRecordingOptions) ([]RecordingInfo, error) { - if _, parseErr := uuid.Parse(opts.clusterID); parseErr != nil { - return nil, ErrInvalidClusterID - } - +// Note: clusterID validation is done in PreRunE. +func resolveWithClusterID(opts *copyRecordingOptions) []RecordingInfo { return []RecordingInfo{{ Filename: opts.recording, ClusterID: opts.clusterID, - }}, nil + }} } // searchForRecording searches all recordings to find the one matching the filename. @@ -197,9 +199,5 @@ func searchForRecording( } } - return nil, fmt.Errorf("recording not found: %w", &recordingNotFoundError{name: recordingName}) -} - -func (e *recordingNotFoundError) Error() string { - return e.name + return nil, fmt.Errorf("%w: %s", ErrRecordingNotFound, recordingName) } diff --git a/internal/cli/local/remote/list.go b/internal/cli/local/remote/list.go index 5ec1628..fac7337 100644 --- a/internal/cli/local/remote/list.go +++ b/internal/cli/local/remote/list.go @@ -154,7 +154,7 @@ func outputSessionsAsTable(sessions []SessionInfo) error { if index < len(sessions) { s := sessions[index] index++ - // Truncate cluster ID for display + // Truncate cluster ID to 36 chars if longer, for table consistency with invalid pod labels. clusterIDDisplay := s.ClusterID if len(clusterIDDisplay) > uuidStringLength { clusterIDDisplay = clusterIDDisplay[:uuidStringLength] diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go index ad4c60d..83493da 100644 --- a/internal/cli/local/remote/list_recordings.go +++ b/internal/cli/local/remote/list_recordings.go @@ -58,7 +58,18 @@ Examples: homecli remote-access list-recordings --output json `, PreRunE: func(_ *cobra.Command, _ []string) error { - return validateOutputFormat(opts.outputFormat) + if err := validateOutputFormat(opts.outputFormat); err != nil { + return err + } + + // Validate cluster ID is a valid UUID + if opts.clusterID != "" { + if _, err := uuid.Parse(opts.clusterID); err != nil { + return ErrInvalidClusterID + } + } + + return nil }, RunE: func(cmd *cobra.Command, _ []string) error { return listRecordingsRun(cmd, opts) @@ -102,14 +113,10 @@ func listRecordingsRun(cmd *cobra.Command, opts *listRecordingsOptions) error { } } -// listRecordings lists recordings, optionally filtered by cluster ID +// listRecordings lists recordings, optionally filtered by cluster ID. func listRecordings(ctx context.Context, client *chart.K8sExecClient, clusterID string) ([]RecordingInfo, error) { searchPath := recordingsPath if clusterID != "" { - // Validate clusterID is a valid UUID to prevent shell injection - if _, err := uuid.Parse(clusterID); err != nil { - return nil, ErrInvalidClusterID - } searchPath = fmt.Sprintf("%s/%s", recordingsPath, clusterID) } diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go index f432294..c8938da 100644 --- a/internal/cli/local/remote/start.go +++ b/internal/cli/local/remote/start.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + "github.com/google/uuid" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -39,6 +40,9 @@ var ( "cluster-name must start and end with alphanumeric characters, " + "and contain only alphanumerics, dashes, underscores, or dots", ) + + // ErrClusterIDInvalid is returned when the cluster ID is not a valid UUID. + ErrClusterIDInvalid = errors.New("cluster-id must be a valid UUID") ) const ( @@ -80,6 +84,24 @@ Examples: homecli remote-access start --cluster-id "550e8400-..." --cluster-name "prod" --ssh-keys-path "/root/.ssh" # Start a session with cloud URL from config homecli remote-access start --cluster-id "550e8400-..." --cluster-name "prod" --ssh-keys-path "/root/.ssh" --tmate-server-host "tmate.example.com" --tmate-server-port "22" --tmate-server-rsa-fingerprint "1234567890" --tmate-server-ed25519-fingerprint "1234567890" --tmate-server-ecdsa-fingerprint "1234567890" # Start a session with custom tmate server `, + PreRunE: func(_ *cobra.Command, _ []string) error { + // Validate SSH keys path exists and is a directory + if err := validateSSHKeysPath(opts.sshKeysPath); err != nil { + return err + } + + // Validate cluster name for Kubernetes label compatibility + if err := validateClusterName(opts.clusterName); err != nil { + return err + } + + // Validate cluster ID is a valid UUID to prevent label injection attacks + if _, err := uuid.Parse(opts.clusterID); err != nil { + return fmt.Errorf("%w: %s", ErrClusterIDInvalid, opts.clusterID) + } + + return nil + }, RunE: func(cmd *cobra.Command, _ []string) error { return startRun(cmd, opts) }, @@ -117,16 +139,6 @@ Examples: func startRun(cmd *cobra.Command, opts *startOptions) error { ctx := cmd.Context() - // Validate SSH keys path exists and is a directory - if err := validateSSHKeysPath(opts.sshKeysPath); err != nil { - return err - } - - // Validate cluster name for Kubernetes label compatibility - if err := validateClusterName(opts.clusterName); err != nil { - return err - } - // Resolve cloud URL from flag, config, or default cloudURL := opts.cloudURL if cloudURL == "" && env.CurrentSiteConfig != nil { diff --git a/internal/cli/local/remote/stop.go b/internal/cli/local/remote/stop.go index 3645b2c..285a7e5 100644 --- a/internal/cli/local/remote/stop.go +++ b/internal/cli/local/remote/stop.go @@ -3,7 +3,9 @@ package remote import ( "errors" "fmt" + "regexp" + "github.com/google/uuid" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -11,6 +13,9 @@ import ( "github.com/weka/gohomecli/internal/utils" ) +// sessionIDPattern matches 6 hex characters (format from generateShortID) +var sessionIDPattern = regexp.MustCompile(`^[0-9a-fA-F]{6}$`) + // ErrMissingStopFilter is returned when no filter is specified for stop command var ErrMissingStopFilter = errors.New("must specify one of: --session-id, --cluster-id, or --all") @@ -42,6 +47,18 @@ Examples: return ErrMissingStopFilter } + // Validate session-id format to prevent label selector injection + if opts.sessionID != "" && !sessionIDPattern.MatchString(opts.sessionID) { + return errors.New("invalid session-id format: must be 6 hex characters (e.g., a1b2c3)") + } + + // Validate cluster-id format to prevent label selector injection + if opts.clusterID != "" { + if _, err := uuid.Parse(opts.clusterID); err != nil { + return errors.New("invalid cluster-id format: must be a valid UUID") + } + } + return nil }, } diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index 2249e54..d4fc25b 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -129,10 +129,8 @@ func (c *K8sExecClient) Exec(ctx context.Context, command ...string) (string, er // CopyFromPod copies a file from the pod to the local filesystem using streaming func (c *K8sExecClient) CopyFromPod(ctx context.Context, remotePath, localPath string) error { - // Use sh -c to cd first, then tar - more portable across tar implementations (busybox, gnu) - tarCmd := fmt.Sprintf("cd %q && tar cf - %q", filepath.Dir(remotePath), filepath.Base(remotePath)) - - executor, err := c.newExecutor([]string{"sh", "-c", tarCmd}) + // Run tar directly without using a shell to avoid command injection risks + executor, err := c.newExecutor([]string{"tar", "cf", "-", remotePath}) if err != nil { return fmt.Errorf("failed to create executor: %w", err) } @@ -222,7 +220,7 @@ func extractTarFile(reader io.Reader, destPath string) error { if err != nil { return fmt.Errorf("failed to create file: %w", err) } - defer outFile.Close() //nolint:errcheck // error on close after successful write is acceptable + defer outFile.Close() //nolint:errcheck // safe: function returns immediately after first file extraction _, copyErr := io.Copy(outFile, tr) if copyErr != nil { From b064b04b987a1c4539e0b135ef41314448e54b6a Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Wed, 28 Jan 2026 09:24:23 +0200 Subject: [PATCH 11/16] sort recordings list --- internal/cli/local/remote/list_recordings.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go index 83493da..17f5652 100644 --- a/internal/cli/local/remote/list_recordings.go +++ b/internal/cli/local/remote/list_recordings.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "sort" "strconv" "strings" "time" @@ -140,7 +141,14 @@ func listRecordings(ctx context.Context, client *chart.K8sExecClient, clusterID return nil, fmt.Errorf("failed to list recordings: %w", err) } - return parseStatOutput(output), nil + recordings := parseStatOutput(output) + + // Sort by modification time descending (latest first) + sort.Slice(recordings, func(i, j int) bool { + return recordings[i].ModTime > recordings[j].ModTime + }) + + return recordings, nil } // parseStatOutput parses the output of stat command into RecordingInfo structs From 27636aa55466e26abcb6c00d0cdb976e7bba8143 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Wed, 28 Jan 2026 09:56:34 +0200 Subject: [PATCH 12/16] fix error handling --- internal/local/chart/kube.go | 57 ++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index d4fc25b..cf18a52 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -28,8 +28,12 @@ import ( const ( KubeConfigPath = "/etc/rancher/k3s/k3s.yaml" copyFromPodTimeout = 10 * time.Minute + + tarTypeRegV7 = '\x00' // V7 Unix tar regular file typeflag (null byte) ) +var errFileNotFoundInTar = errors.New("file not found in tar archive") + // K8sClient holds a Kubernetes clientset and REST config type K8sClient struct { Clientset *kubernetes.Clientset @@ -200,38 +204,55 @@ func isClosedPipeError(err error) bool { return strings.Contains(err.Error(), "closed pipe") } -// extractTarFile extracts a single file from a tar stream -func extractTarFile(reader io.Reader, destPath string) error { +// extractTarFile extracts the first regular file from a tar stream and drains remaining data. +// Draining prevents "closed pipe" errors from the streaming goroutine. +func extractTarFile(reader io.Reader, destPath string) error { //nolint:gocognit // simple loop tr := tar.NewReader(reader) - entryCount := 0 + extracted := false + for { header, err := tr.Next() - if err == io.EOF { - return fmt.Errorf("file not found in tar archive (found %d entries)", entryCount) + if errors.Is(err, io.EOF) { + if !extracted { + return errFileNotFoundInTar + } + + return nil } + if err != nil { + if extracted { + return nil // ignore stream errors after successful extraction + } + return fmt.Errorf("failed to read tar: %w", err) } - entryCount++ - // Accept both TypeReg ('0') and TypeRegA ('\x00') for compatibility - if header.Typeflag == tar.TypeReg || header.Typeflag == tar.TypeRegA { - outFile, err := os.Create(destPath) - if err != nil { - return fmt.Errorf("failed to create file: %w", err) + // Extract first regular file, then continue draining + if !extracted && (header.Typeflag == tar.TypeReg || header.Typeflag == tarTypeRegV7) { + if err := writeFile(destPath, tr); err != nil { + return err } - defer outFile.Close() //nolint:errcheck // safe: function returns immediately after first file extraction - _, copyErr := io.Copy(outFile, tr) - if copyErr != nil { - return fmt.Errorf("failed to copy file: %w", copyErr) - } - - return nil + extracted = true } } } +func writeFile(path string, r io.Reader) error { + f, err := os.Create(path) + if err != nil { + return fmt.Errorf("failed to create file: %w", err) + } + defer f.Close() //nolint:errcheck // best effort + + if _, err := io.Copy(f, r); err != nil { + return fmt.Errorf("failed to write file: %w", err) + } + + return nil +} + // ReadKubeConfig reads the kubeconfig from the given path with fallback to ~/.kube/config func ReadKubeConfig(kubeConfigPath string) ([]byte, error) { if kubeConfigPath == "" { From 0825548d0f8b1fab7d66b9e1a01fc7a2a5fb3dd3 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Wed, 28 Jan 2026 10:52:54 +0200 Subject: [PATCH 13/16] validate tmte server flags --- internal/cli/local/remote/list_recordings.go | 7 +++- internal/cli/local/remote/start.go | 41 +++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go index 17f5652..f538c7f 100644 --- a/internal/cli/local/remote/list_recordings.go +++ b/internal/cli/local/remote/list_recordings.go @@ -187,7 +187,12 @@ func parseStatOutput(output string) []RecordingInfo { size, sizeErr := strconv.ParseInt(parts[1], 10, 64) mtime, mtimeErr := strconv.ParseInt(parts[2], 10, 64) if sizeErr != nil || mtimeErr != nil { - // Skip malformed entries - log at debug level + logger.Debug(). + Err(sizeErr). + AnErr("mtimeErr", mtimeErr). + Str("line", line). + Msg("Skipping malformed stat output entry") + continue } diff --git a/internal/cli/local/remote/start.go b/internal/cli/local/remote/start.go index c8938da..8cef690 100644 --- a/internal/cli/local/remote/start.go +++ b/internal/cli/local/remote/start.go @@ -43,6 +43,13 @@ var ( // ErrClusterIDInvalid is returned when the cluster ID is not a valid UUID. ErrClusterIDInvalid = errors.New("cluster-id must be a valid UUID") + + // ErrTmateServerFlagsIncomplete is returned when some but not all tmate server flags are provided. + ErrTmateServerFlagsIncomplete = errors.New( + "when using custom tmate server, all flags must be provided: " + + "--tmate-server-host, --tmate-server-port, --tmate-server-rsa-fingerprint, " + + "--tmate-server-ed25519-fingerprint, --tmate-server-ecdsa-fingerprint", + ) ) const ( @@ -100,6 +107,11 @@ Examples: return fmt.Errorf("%w: %s", ErrClusterIDInvalid, opts.clusterID) } + // Validate tmate server flags: if any are provided, all must be provided + if err := validateTmateServerFlags(opts); err != nil { + return err + } + return nil }, RunE: func(cmd *cobra.Command, _ []string) error { @@ -122,7 +134,7 @@ Examples: cmd.Flags().StringVar(&opts.hostName, "host-name", "", "Override hostname (default: system hostname)") cmd.Flags().IntVar(&opts.terminalCols, "terminal-cols", 0, "Terminal width (default: 158)") cmd.Flags().IntVar(&opts.terminalLines, "terminal-lines", 0, "Terminal height (default: 35)") - cmd.Flags().BoolVar(&opts.debug, "debug", false, "Enable debug logging") + cmd.Flags().BoolVar(&opts.debug, "debug", false, "Enable debug logging in tmate container (default: false)") // Tmate server override flags (optional - tmate.py has built-in config for known cloud URLs) cmd.Flags().StringVar(&opts.tmateServerHost, "tmate-server-host", "", "Override tmate SSH server hostname") @@ -258,6 +270,33 @@ func validateClusterName(name string) error { return nil } +// validateTmateServerFlags validates that if ONE of these flags is provided, ALL must be provided. +func validateTmateServerFlags(opts *startOptions) error { + flags := map[string]string{ + "--tmate-server-host": opts.tmateServerHost, + "--tmate-server-port": opts.tmateServerPort, + "--tmate-server-rsa-fingerprint": opts.tmateServerRSA, + "--tmate-server-ed25519-fingerprint": opts.tmateServerEd25519, + "--tmate-server-ecdsa-fingerprint": opts.tmateServerECDSA, + } + + var provided, missing []string + for name, value := range flags { + if value != "" { + provided = append(provided, name) + } else { + missing = append(missing, name) + } + } + + // If ONE is provided but not ALL, return error + if len(provided) > 0 && len(missing) > 0 { + return ErrTmateServerFlagsIncomplete + } + + return nil +} + func generateShortID() string { b := make([]byte, sessionIDBytes) _, _ = rand.Read(b) //nolint:errcheck // crypto/rand.Read error indicates serious system issue From a28d2b45eca3e2fcec0dd02d2e7f47890bbd7f1d Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Wed, 28 Jan 2026 11:57:46 +0200 Subject: [PATCH 14/16] fixes 4 --- internal/cli/local/remote/list_recordings.go | 97 +++++++++++++------- internal/local/chart/kube.go | 2 + 2 files changed, 64 insertions(+), 35 deletions(-) diff --git a/internal/cli/local/remote/list_recordings.go b/internal/cli/local/remote/list_recordings.go index f538c7f..4bc1a9e 100644 --- a/internal/cli/local/remote/list_recordings.go +++ b/internal/cli/local/remote/list_recordings.go @@ -156,55 +156,82 @@ func listRecordings(ctx context.Context, client *chart.K8sExecClient, clusterID func parseStatOutput(output string) []RecordingInfo { lines := strings.Split(strings.TrimSpace(output), "\n") recordings := make([]RecordingInfo, 0, len(lines)) + skipped := 0 + for _, line := range lines { line = strings.TrimSpace(line) if line == "" { continue } - parts := strings.Split(line, "|") - if len(parts) != statOutputParts { + info, ok := parseStatLine(line) + if !ok { + skipped++ + continue } - // Extract filename and cluster ID from path - // /recordings/file.cast -> clusterID="", filename="file.cast" - // /recordings/abc-123/file.cast -> clusterID="abc-123", filename="file.cast" - // /recordings/abc-123/subdir/file.cast -> clusterID="abc-123", filename="subdir/file.cast" - fullPath := parts[0] - relPath := strings.TrimPrefix(fullPath, recordingsPath+"/") - pathParts := strings.Split(relPath, "/") - - var filename, clusterID string - if len(pathParts) == 1 { - filename = pathParts[0] - } else { - clusterID = pathParts[0] - // Keep the full relative path after cluster ID (handles nested dirs) - filename = strings.Join(pathParts[1:], "/") - } + recordings = append(recordings, info) + } - size, sizeErr := strconv.ParseInt(parts[1], 10, 64) - mtime, mtimeErr := strconv.ParseInt(parts[2], 10, 64) - if sizeErr != nil || mtimeErr != nil { - logger.Debug(). - Err(sizeErr). - AnErr("mtimeErr", mtimeErr). - Str("line", line). - Msg("Skipping malformed stat output entry") + if skipped > 0 { + logger.Warn(). + Int("skipped", skipped). + Int("parsed", len(recordings)). + Msg("Some stat output entries could not be parsed") + } - continue - } + return recordings +} - recordings = append(recordings, RecordingInfo{ - Filename: filename, - Size: size, - ModTime: mtime, - ClusterID: clusterID, - }) +// parseStatLine parses a single stat output line into a RecordingInfo. +// Returns false if the line is malformed. +func parseStatLine(line string) (RecordingInfo, bool) { + parts := strings.Split(line, "|") + if len(parts) != statOutputParts { + logger.Debug(). + Str("line", line). + Int("parts", len(parts)). + Int("expected", statOutputParts). + Msg("Skipping stat output entry with wrong number of parts") + + return RecordingInfo{}, false } - return recordings + // Extract filename and cluster ID from path + // /recordings/file.cast -> clusterID="", filename="file.cast" + // /recordings/abc-123/file.cast -> clusterID="abc-123", filename="file.cast" + // /recordings/abc-123/subdir/file.cast -> clusterID="abc-123", filename="subdir/file.cast" + fullPath := parts[0] + relPath := strings.TrimPrefix(fullPath, recordingsPath+"/") + pathParts := strings.Split(relPath, "/") + + var filename, clusterID string + if len(pathParts) == 1 { + filename = pathParts[0] + } else { + clusterID = pathParts[0] + filename = strings.Join(pathParts[1:], "/") // handles nested dirs + } + + size, sizeErr := strconv.ParseInt(parts[1], 10, 64) + mtime, mtimeErr := strconv.ParseInt(parts[2], 10, 64) + if sizeErr != nil || mtimeErr != nil { + logger.Debug(). + Err(sizeErr). + AnErr("mtimeErr", mtimeErr). + Str("line", line). + Msg("Skipping stat output entry with invalid size or mtime") + + return RecordingInfo{}, false + } + + return RecordingInfo{ + Filename: filename, + Size: size, + ModTime: mtime, + ClusterID: clusterID, + }, true } func outputRecordingsAsJSON(recordings []RecordingInfo) error { diff --git a/internal/local/chart/kube.go b/internal/local/chart/kube.go index cf18a52..6fd2498 100644 --- a/internal/local/chart/kube.go +++ b/internal/local/chart/kube.go @@ -247,6 +247,8 @@ func writeFile(path string, r io.Reader) error { defer f.Close() //nolint:errcheck // best effort if _, err := io.Copy(f, r); err != nil { + os.Remove(path) //nolint:errcheck // best effort cleanup of partial file + return fmt.Errorf("failed to write file: %w", err) } From c5c9dd62e72d3771ef37089b5e8e534c50f98417 Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Sun, 1 Feb 2026 10:04:28 +0200 Subject: [PATCH 15/16] Squashed commit of the following: commit c4606e92045d381e7c93d0e8a27795d3cc93cdd4 Merge: a335aa3 511a9db Author: vyeveweka Date: Fri Jan 30 16:39:32 2026 +0200 Merge pull request #112 from weka/chore/v.yevenko/WH-3694-disable-redis-fix chore: disable redis fix commit 511a9db0cd900a24bea48426ed8aab576d5e7ff4 Author: Vitalii Yevenko Date: Fri Jan 30 12:23:14 2026 +0200 disabled redis for LWH commit 249a960fea2078f381f1bc9a15a7a58857e77beb Author: Vitalii Yevenko Date: Thu Jan 29 22:00:15 2026 +0200 fixed redis deployment commit a335aa389d1c921c760ae00fb4015495be870a89 Merge: b3e913b 53deac1 Author: vyeveweka Date: Thu Jan 29 17:02:26 2026 +0200 Merge pull request #111 from weka/chore/v.yevenko/WH-3694-disable-redis chore: disable redis commit 53deac1a57eab5ded7e1ba99f7caa62dd4ffdfd5 Author: Vitalii Yevenko Date: Thu Jan 29 14:15:40 2026 +0200 disable redis --- internal/local/chart/v3.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/local/chart/v3.go b/internal/local/chart/v3.go index 0fec757..8e228d3 100644 --- a/internal/local/chart/v3.go +++ b/internal/local/chart/v3.go @@ -278,6 +278,9 @@ func configureLWH(*config_v1.Configuration) (yamlMap, error) { "resourcesPreset": "micro", }, }), + // redis - disable at all. + writeMapEntry(cfg, "storage.redis.useInternal", false), + writeMapEntry(cfg, "storage.redis.useExternal", false), ) return cfg, err From abadb937d8654ed314808db5cdcb1b0721cf411b Mon Sep 17 00:00:00 2001 From: Daniel Binyamin Date: Sun, 1 Feb 2026 10:08:48 +0200 Subject: [PATCH 16/16] fix merge conflict --- internal/local/chart/v3.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/internal/local/chart/v3.go b/internal/local/chart/v3.go index 945175d..992699a 100644 --- a/internal/local/chart/v3.go +++ b/internal/local/chart/v3.go @@ -264,20 +264,6 @@ func configureLWH(*config_v1.Configuration) (yamlMap, error) { writeMapEntry(cfg, "grafana.enabledDashboards.events-insights", false), // enable remote session client for LWH writeMapEntry(cfg, "remoteSessionClient.enabled", true), - // redis - writeMapEntry(cfg, "redis-cluster", yamlMap{ - "cluster": yamlMap{ - "nodes": 3, - "replicas": 0, - "update": yamlMap{ - "currentNumberOfNodes": 3, - "currentNumberOfReplicas": 0, - }, - }, - "redis": yamlMap{ - "resourcesPreset": "micro", - }, - }), // redis - disable at all. writeMapEntry(cfg, "storage.redis.useInternal", false), writeMapEntry(cfg, "storage.redis.useExternal", false),