From 81b3d9cf84f1938050a571faba0750a7075fae3d Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Fri, 19 Dec 2025 21:06:09 -0800 Subject: [PATCH 1/4] feat(labctl): implement images command suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement all CLI commands for the labctl images tool: - sync: Download, verify, decompress (xz/gzip/zstd), upload to S3, write metadata, and update local files with regex replacements - validate: Check manifest YAML syntax and verify URLs via HEAD requests - list: Display images from S3 bucket with metadata in tabular format - prune: Remove orphaned images not in manifest (with --dry-run) - upload: Upload local files to S3 with SHA256 checksum and metadata Also adds internal/updater package for regex-based file updates with Go template variable substitution. Closes HOM-20 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- tools/labctl/cmd/images/list.go | 102 +++++- tools/labctl/cmd/images/prune.go | 91 +++++- tools/labctl/cmd/images/sync.go | 359 ++++++++++++++++++++- tools/labctl/cmd/images/upload.go | 100 +++++- tools/labctl/cmd/images/validate.go | 75 ++++- tools/labctl/go.mod | 2 + tools/labctl/go.sum | 4 + tools/labctl/internal/updater/file.go | 121 +++++++ tools/labctl/internal/updater/file_test.go | 212 ++++++++++++ 9 files changed, 1055 insertions(+), 11 deletions(-) create mode 100644 tools/labctl/internal/updater/file.go create mode 100644 tools/labctl/internal/updater/file_test.go diff --git a/tools/labctl/cmd/images/list.go b/tools/labctl/cmd/images/list.go index d41317f..cd22341 100644 --- a/tools/labctl/cmd/images/list.go +++ b/tools/labctl/cmd/images/list.go @@ -1,9 +1,16 @@ package images import ( + "context" "fmt" + "os" + "strings" + "text/tabwriter" "github.com/spf13/cobra" + + "github.com/GilmanLab/lab/tools/labctl/internal/credentials" + "github.com/GilmanLab/lab/tools/labctl/internal/store" ) var listCmd = &cobra.Command{ @@ -24,7 +31,96 @@ func init() { } func runList(_ *cobra.Command, _ []string) error { - // TODO(HOM-20): Implement list command - fmt.Println("list command not yet implemented") - return nil + ctx := context.Background() + + // Resolve credentials + creds, err := credentials.Resolve(credentials.ResolveOptions{ + SOPSFile: listCredentials, + AgeKeyFile: listSOPSAgeKeyFile, + }) + if err != nil { + return fmt.Errorf("resolve credentials: %w", err) + } + + // Create S3 client + client, err := store.NewS3Client(creds, store.WithContext(ctx)) + if err != nil { + return fmt.Errorf("create S3 client: %w", err) + } + + // List all images + keys, err := client.List(ctx, "images/") + if err != nil { + return fmt.Errorf("list images: %w", err) + } + + if len(keys) == 0 { + fmt.Println("No images found") + return nil + } + + // Create tabwriter for formatted output + w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + _, _ = fmt.Fprintln(w, "NAME\tPATH\tSIZE\tCHECKSUM\tUPLOADED") + _, _ = fmt.Fprintln(w, "----\t----\t----\t--------\t--------") + + for _, key := range keys { + // Skip directories (keys ending with /) + if strings.HasSuffix(key, "/") { + continue + } + + // Convert image key to destination path + // images/vyos/vyos-1.5.iso -> vyos/vyos-1.5.iso + destPath := strings.TrimPrefix(key, "images/") + + // Try to get metadata + metadata, err := client.GetMetadata(ctx, destPath) + if err != nil { + // Metadata might not exist for all images + _, _ = fmt.Fprintf(w, "-\t%s\t-\t-\t-\n", destPath) + continue + } + + // Format size + sizeStr := formatSize(metadata.Size) + + // Truncate checksum for display + checksumStr := metadata.Checksum + if len(checksumStr) > 20 { + checksumStr = checksumStr[:20] + "..." + } + + // Format upload time + uploadedStr := metadata.UploadedAt.Format("2006-01-02 15:04") + + _, _ = fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", + metadata.Name, + destPath, + sizeStr, + checksumStr, + uploadedStr, + ) + } + + return w.Flush() +} + +func formatSize(bytes int64) string { + const ( + kb = 1024 + mb = kb * 1024 + gb = mb * 1024 + ) + + switch { + case bytes >= gb: + return fmt.Sprintf("%.2f GB", float64(bytes)/gb) + case bytes >= mb: + return fmt.Sprintf("%.2f MB", float64(bytes)/mb) + case bytes >= kb: + return fmt.Sprintf("%.2f KB", float64(bytes)/kb) + default: + return fmt.Sprintf("%d B", bytes) + } } diff --git a/tools/labctl/cmd/images/prune.go b/tools/labctl/cmd/images/prune.go index 8740c0a..31af5a3 100644 --- a/tools/labctl/cmd/images/prune.go +++ b/tools/labctl/cmd/images/prune.go @@ -1,9 +1,15 @@ package images import ( + "context" "fmt" + "strings" "github.com/spf13/cobra" + + "github.com/GilmanLab/lab/tools/labctl/internal/config" + "github.com/GilmanLab/lab/tools/labctl/internal/credentials" + "github.com/GilmanLab/lab/tools/labctl/internal/store" ) var pruneCmd = &cobra.Command{ @@ -32,7 +38,88 @@ func init() { } func runPrune(_ *cobra.Command, _ []string) error { - // TODO(HOM-20): Implement prune command - fmt.Println("prune command not yet implemented") + ctx := context.Background() + + // Load manifest + manifest, err := config.LoadManifest(pruneManifest) + if err != nil { + return fmt.Errorf("load manifest: %w", err) + } + + // Build set of expected destinations from manifest + expected := make(map[string]bool) + for _, img := range manifest.Spec.Images { + expected[img.Destination] = true + } + + // Resolve credentials + creds, err := credentials.Resolve(credentials.ResolveOptions{ + SOPSFile: pruneCredentials, + AgeKeyFile: pruneSOPSAgeKeyFile, + }) + if err != nil { + return fmt.Errorf("resolve credentials: %w", err) + } + + // Create S3 client + client, err := store.NewS3Client(creds, store.WithContext(ctx)) + if err != nil { + return fmt.Errorf("create S3 client: %w", err) + } + + // List all images in storage + keys, err := client.List(ctx, "images/") + if err != nil { + return fmt.Errorf("list images: %w", err) + } + + // Find orphaned images + var orphaned []string + for _, key := range keys { + // Skip directories + if strings.HasSuffix(key, "/") { + continue + } + + // Convert key to destination path + destPath := strings.TrimPrefix(key, "images/") + + if !expected[destPath] { + orphaned = append(orphaned, destPath) + } + } + + if len(orphaned) == 0 { + fmt.Println("No orphaned images found") + return nil + } + + // Report and optionally delete orphaned images + fmt.Printf("Found %d orphaned image(s):\n", len(orphaned)) + for _, dest := range orphaned { + if pruneDryRun { + fmt.Printf(" Would remove: %s\n", dest) + } else { + fmt.Printf(" Removing: %s\n", dest) + + // Delete image + imageKey := store.ImageKey(dest) + if err := client.Delete(ctx, imageKey); err != nil { + return fmt.Errorf("delete image %s: %w", dest, err) + } + + // Delete metadata + metadataKey := store.MetadataKey(dest) + // Ignore metadata deletion errors (might not exist) + _ = client.Delete(ctx, metadataKey) + } + } + + if pruneDryRun { + fmt.Printf("\nDry run: no changes made\n") + } else { + fmt.Printf("\nRemoved %d orphaned image(s)\n", len(orphaned)) + } + return nil } diff --git a/tools/labctl/cmd/images/sync.go b/tools/labctl/cmd/images/sync.go index 8720a67..7ed44e7 100644 --- a/tools/labctl/cmd/images/sync.go +++ b/tools/labctl/cmd/images/sync.go @@ -1,9 +1,27 @@ package images import ( + "compress/gzip" + "context" + "crypto/sha256" + "crypto/sha512" + "encoding/hex" "fmt" + "hash" + "io" + "net/http" + "os" + "strings" + "time" + "github.com/klauspost/compress/zstd" "github.com/spf13/cobra" + "github.com/ulikunitz/xz" + + "github.com/GilmanLab/lab/tools/labctl/internal/config" + "github.com/GilmanLab/lab/tools/labctl/internal/credentials" + "github.com/GilmanLab/lab/tools/labctl/internal/store" + "github.com/GilmanLab/lab/tools/labctl/internal/updater" ) var syncCmd = &cobra.Command{ @@ -34,7 +52,344 @@ func init() { } func runSync(_ *cobra.Command, _ []string) error { - // TODO(HOM-20): Implement sync command - fmt.Println("sync command not yet implemented") + ctx := context.Background() + + // Load manifest + manifest, err := config.LoadManifest(syncManifest) + if err != nil { + return fmt.Errorf("load manifest: %w", err) + } + + fmt.Printf("Syncing images from manifest: %s\n", syncManifest) + fmt.Printf("Found %d image(s)\n\n", len(manifest.Spec.Images)) + + // Skip credentials and S3 client setup in dry-run mode + var client *store.S3Client + if !syncDryRun { + // Resolve credentials + creds, err := credentials.Resolve(credentials.ResolveOptions{ + SOPSFile: syncCredentials, + AgeKeyFile: syncSOPSAgeKeyFile, + }) + if err != nil { + return fmt.Errorf("resolve credentials: %w", err) + } + + // Create S3 client + client, err = store.NewS3Client(creds, store.WithContext(ctx)) + if err != nil { + return fmt.Errorf("create S3 client: %w", err) + } + } + + // Track if any files were changed (for GitHub Actions output) + filesChanged := false + + // Process each image + for _, img := range manifest.Spec.Images { + changed, err := syncImage(ctx, client, img, syncDryRun, syncForce) + if err != nil { + return fmt.Errorf("sync image %q: %w", img.Name, err) + } + if changed { + filesChanged = true + } + } + + // Write GitHub Actions output + if err := writeGitHubOutput("files_changed", fmt.Sprintf("%t", filesChanged)); err != nil { + // Log but don't fail - not running in GitHub Actions + fmt.Printf("Note: Could not write GitHub Actions output: %v\n", err) + } + + fmt.Println("\nSync complete") + if filesChanged { + fmt.Println("Files were changed - PR may be needed") + } + + return nil +} + +func syncImage(ctx context.Context, client *store.S3Client, img config.Image, dryRun, force bool) (bool, error) { + fmt.Printf("Processing: %s\n", img.Name) + + effectiveChecksum := img.EffectiveChecksum() + + // Check if image already exists with matching checksum + if !dryRun && !force { + matches, err := client.ChecksumMatches(ctx, img.Destination, effectiveChecksum) + if err != nil { + return false, fmt.Errorf("check existing image: %w", err) + } + if matches { + fmt.Printf(" Skipping: checksum matches existing image\n") + return false, nil + } + } + + if dryRun { + fmt.Printf(" Would download: %s\n", img.Source.URL) + fmt.Printf(" Would upload to: %s\n", store.ImageKey(img.Destination)) + if img.UpdateFile != nil { + fmt.Printf(" Would update file: %s\n", img.UpdateFile.Path) + } + return false, nil + } + + // Download source image to temp file + fmt.Printf(" Downloading from: %s\n", img.Source.URL) + tempFile, size, err := downloadToTemp(ctx, img.Source.URL) + if err != nil { + return false, fmt.Errorf("download: %w", err) + } + defer func() { + _ = tempFile.Close() + _ = os.Remove(tempFile.Name()) + }() + + // Verify source checksum + fmt.Printf(" Verifying source checksum...\n") + if _, err := tempFile.Seek(0, 0); err != nil { + return false, fmt.Errorf("seek temp file: %w", err) + } + if err := verifyChecksum(tempFile, img.Source.Checksum); err != nil { + return false, fmt.Errorf("source checksum verification: %w", err) + } + + // Decompress if needed + var uploadFile *os.File + var uploadSize int64 + if img.Source.Decompress != "" { + fmt.Printf(" Decompressing (%s)...\n", img.Source.Decompress) + if _, err := tempFile.Seek(0, 0); err != nil { + return false, fmt.Errorf("seek temp file: %w", err) + } + decompFile, decompSize, err := decompress(tempFile, img.Source.Decompress) + if err != nil { + return false, fmt.Errorf("decompress: %w", err) + } + defer func() { + _ = decompFile.Close() + _ = os.Remove(decompFile.Name()) + }() + + // Verify post-decompression checksum if validation is specified + if img.Validation != nil && img.Validation.Expected != "" { + fmt.Printf(" Verifying decompressed checksum...\n") + if _, err := decompFile.Seek(0, 0); err != nil { + return false, fmt.Errorf("seek decompressed file: %w", err) + } + if err := verifyChecksum(decompFile, img.Validation.Expected); err != nil { + return false, fmt.Errorf("decompressed checksum verification: %w", err) + } + } + + uploadFile = decompFile + uploadSize = decompSize + } else { + uploadFile = tempFile + uploadSize = size + } + + // Upload to e2 + if _, err := uploadFile.Seek(0, 0); err != nil { + return false, fmt.Errorf("seek upload file: %w", err) + } + imageKey := store.ImageKey(img.Destination) + fmt.Printf(" Uploading to: %s (%s)\n", imageKey, formatSize(uploadSize)) + if err := client.Upload(ctx, imageKey, uploadFile, uploadSize); err != nil { + return false, fmt.Errorf("upload: %w", err) + } + + // Write metadata + metadata := &store.ImageMetadata{ + Name: img.Name, + Checksum: effectiveChecksum, + Size: uploadSize, + UploadedAt: time.Now().UTC(), + Source: store.SourceMetadata{ + Type: "http", + URL: img.Source.URL, + }, + } + if err := client.PutMetadata(ctx, img.Destination, metadata); err != nil { + return false, fmt.Errorf("write metadata: %w", err) + } + + // Apply file updates if specified + filesChanged := false + if img.UpdateFile != nil { + fmt.Printf(" Updating file: %s\n", img.UpdateFile.Path) + + replacements := make([]updater.Replacement, len(img.UpdateFile.Replacements)) + for i, r := range img.UpdateFile.Replacements { + replacements[i] = updater.Replacement{ + Pattern: r.Pattern, + Value: r.Value, + } + } + + data := updater.TemplateData{ + Source: updater.SourceData{ + URL: img.Source.URL, + Checksum: img.Source.Checksum, + }, + } + + fileUpdater, err := updater.New(replacements, data) + if err != nil { + return false, fmt.Errorf("create file updater: %w", err) + } + + modified, err := fileUpdater.UpdateFile(img.UpdateFile.Path) + if err != nil { + return false, fmt.Errorf("update file: %w", err) + } + + if modified { + fmt.Printf(" File updated: %s\n", img.UpdateFile.Path) + filesChanged = true + } else { + fmt.Printf(" File unchanged: %s\n", img.UpdateFile.Path) + } + } + + fmt.Printf(" Done\n") + return filesChanged, nil +} + +func downloadToTemp(ctx context.Context, url string) (*os.File, int64, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) + if err != nil { + return nil, 0, fmt.Errorf("create request: %w", err) + } + req.Header.Set("User-Agent", "labctl/1.0") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, 0, fmt.Errorf("HTTP request: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return nil, 0, fmt.Errorf("HTTP %d: %s", resp.StatusCode, resp.Status) + } + + tempFile, err := os.CreateTemp("", "labctl-download-*") + if err != nil { + return nil, 0, fmt.Errorf("create temp file: %w", err) + } + + size, err := io.Copy(tempFile, resp.Body) + if err != nil { + _ = tempFile.Close() + _ = os.Remove(tempFile.Name()) + return nil, 0, fmt.Errorf("write to temp file: %w", err) + } + + return tempFile, size, nil +} + +func verifyChecksum(r io.Reader, expected string) error { + // Parse expected checksum format: "sha256:abc123..." or "sha512:..." + parts := strings.SplitN(expected, ":", 2) + if len(parts) != 2 { + return fmt.Errorf("invalid checksum format: %s", expected) + } + + algorithm := parts[0] + expectedHash := parts[1] + + var h hash.Hash + switch algorithm { + case "sha256": + h = sha256.New() + case "sha512": + h = sha512.New() + default: + return fmt.Errorf("unsupported hash algorithm: %s", algorithm) + } + + if _, err := io.Copy(h, r); err != nil { + return fmt.Errorf("compute hash: %w", err) + } + + actual := hex.EncodeToString(h.Sum(nil)) + if actual != expectedHash { + return fmt.Errorf("checksum mismatch: expected %s, got %s", expectedHash, actual) + } + return nil } + +// maxDecompressedSize limits decompressed file size to 50GB to prevent decompression bombs. +const maxDecompressedSize = 50 * 1024 * 1024 * 1024 + +func decompress(r io.Reader, format string) (*os.File, int64, error) { + var reader io.Reader + var cleanup func() + + switch format { + case "xz": + xzReader, err := xz.NewReader(r) + if err != nil { + return nil, 0, fmt.Errorf("create xz reader: %w", err) + } + reader = xzReader + case "gzip": + gzReader, err := gzip.NewReader(r) + if err != nil { + return nil, 0, fmt.Errorf("create gzip reader: %w", err) + } + reader = gzReader + cleanup = func() { _ = gzReader.Close() } + case "zstd": + zstdReader, err := zstd.NewReader(r) + if err != nil { + return nil, 0, fmt.Errorf("create zstd reader: %w", err) + } + reader = zstdReader + cleanup = func() { zstdReader.Close() } + default: + return nil, 0, fmt.Errorf("unsupported decompression format: %s", format) + } + + // Wrap with a limit reader to prevent decompression bombs + limitedReader := io.LimitReader(reader, maxDecompressedSize) + + tempFile, err := os.CreateTemp("", "labctl-decompress-*") + if err != nil { + if cleanup != nil { + cleanup() + } + return nil, 0, fmt.Errorf("create temp file: %w", err) + } + + size, err := io.Copy(tempFile, limitedReader) + if cleanup != nil { + cleanup() + } + if err != nil { + _ = tempFile.Close() + _ = os.Remove(tempFile.Name()) + return nil, 0, fmt.Errorf("decompress to temp file: %w", err) + } + + return tempFile, size, nil +} + +func writeGitHubOutput(name, value string) error { + outputFile := os.Getenv("GITHUB_OUTPUT") + if outputFile == "" { + return fmt.Errorf("GITHUB_OUTPUT not set") + } + + f, err := os.OpenFile(outputFile, os.O_APPEND|os.O_WRONLY, 0o644) //nolint:gosec // G304: Path from env + if err != nil { + return fmt.Errorf("open GITHUB_OUTPUT: %w", err) + } + defer func() { _ = f.Close() }() + + _, err = fmt.Fprintf(f, "%s=%s\n", name, value) + return err +} diff --git a/tools/labctl/cmd/images/upload.go b/tools/labctl/cmd/images/upload.go index 6cbf883..ca5d29d 100644 --- a/tools/labctl/cmd/images/upload.go +++ b/tools/labctl/cmd/images/upload.go @@ -1,9 +1,19 @@ package images import ( + "context" + "crypto/sha256" + "encoding/hex" "fmt" + "io" + "os" + "path/filepath" + "time" "github.com/spf13/cobra" + + "github.com/GilmanLab/lab/tools/labctl/internal/credentials" + "github.com/GilmanLab/lab/tools/labctl/internal/store" ) var uploadCmd = &cobra.Command{ @@ -37,7 +47,93 @@ func init() { } func runUpload(_ *cobra.Command, _ []string) error { - // TODO(HOM-20): Implement upload command - fmt.Println("upload command not yet implemented") + ctx := context.Background() + + // Resolve credentials + creds, err := credentials.Resolve(credentials.ResolveOptions{ + SOPSFile: uploadCredentials, + AgeKeyFile: uploadSOPSAgeKeyFile, + }) + if err != nil { + return fmt.Errorf("resolve credentials: %w", err) + } + + // Create S3 client + client, err := store.NewS3Client(creds, store.WithContext(ctx)) + if err != nil { + return fmt.Errorf("create S3 client: %w", err) + } + + // Get file info + info, err := os.Stat(uploadSource) + if err != nil { + return fmt.Errorf("stat source file: %w", err) + } + + // Compute checksum + fmt.Printf("Computing checksum for %s...\n", uploadSource) + checksum, err := computeFileChecksum(uploadSource) + if err != nil { + return fmt.Errorf("compute checksum: %w", err) + } + fmt.Printf("Checksum: %s\n", checksum) + + // Open file for upload + file, err := os.Open(uploadSource) //nolint:gosec // G304: Path is provided by user + if err != nil { + return fmt.Errorf("open source file: %w", err) + } + defer func() { _ = file.Close() }() + + // Upload to e2 + imageKey := store.ImageKey(uploadDestination) + fmt.Printf("Uploading to %s...\n", imageKey) + if err := client.Upload(ctx, imageKey, file, info.Size()); err != nil { + return fmt.Errorf("upload image: %w", err) + } + + // Determine image name + imageName := uploadName + if imageName == "" { + imageName = filepath.Base(uploadDestination) + // Remove extension if present + if ext := filepath.Ext(imageName); ext != "" { + imageName = imageName[:len(imageName)-len(ext)] + } + } + + // Write metadata + metadata := &store.ImageMetadata{ + Name: imageName, + Checksum: checksum, + Size: info.Size(), + UploadedAt: time.Now().UTC(), + Source: store.SourceMetadata{ + Type: "local", + Path: uploadSource, + }, + } + + fmt.Printf("Writing metadata...\n") + if err := client.PutMetadata(ctx, uploadDestination, metadata); err != nil { + return fmt.Errorf("write metadata: %w", err) + } + + fmt.Printf("Successfully uploaded %s to %s\n", uploadSource, imageKey) return nil } + +func computeFileChecksum(path string) (string, error) { + file, err := os.Open(path) //nolint:gosec // G304: Path is provided by user + if err != nil { + return "", err + } + defer func() { _ = file.Close() }() + + hash := sha256.New() + if _, err := io.Copy(hash, file); err != nil { + return "", err + } + + return "sha256:" + hex.EncodeToString(hash.Sum(nil)), nil +} diff --git a/tools/labctl/cmd/images/validate.go b/tools/labctl/cmd/images/validate.go index e20b71d..3904165 100644 --- a/tools/labctl/cmd/images/validate.go +++ b/tools/labctl/cmd/images/validate.go @@ -1,9 +1,14 @@ package images import ( + "context" "fmt" + "net/http" + "time" "github.com/spf13/cobra" + + "github.com/GilmanLab/lab/tools/labctl/internal/config" ) var validateCmd = &cobra.Command{ @@ -23,8 +28,74 @@ func init() { validateCmd.Flags().StringVar(&validateManifest, "manifest", "./images/images.yaml", "Path to images.yaml") } +// httpClient defines the HTTP operations used for URL validation. +// This interface enables mocking for unit tests. +type httpClient interface { + Do(req *http.Request) (*http.Response, error) +} + +// defaultHTTPClient is the default HTTP client used for URL validation. +var defaultHTTPClient httpClient = &http.Client{ + Timeout: 30 * time.Second, +} + func runValidate(_ *cobra.Command, _ []string) error { - // TODO(HOM-20): Implement validate command - fmt.Println("validate command not yet implemented") + return runValidateWithClient(defaultHTTPClient) +} + +func runValidateWithClient(client httpClient) error { + // Load and parse manifest (validates YAML syntax, regexes, and HTTPS requirement) + manifest, err := config.LoadManifest(validateManifest) + if err != nil { + return fmt.Errorf("load manifest: %w", err) + } + + fmt.Printf("Validating manifest: %s\n", validateManifest) + fmt.Printf("Found %d image(s)\n\n", len(manifest.Spec.Images)) + + // Check all source URLs via HEAD requests + var errors []error + for _, img := range manifest.Spec.Images { + fmt.Printf("Checking %s... ", img.Name) + + if err := checkURL(context.Background(), client, img.Source.URL); err != nil { + errors = append(errors, fmt.Errorf("image %q: %w", img.Name, err)) + fmt.Println("FAILED") + fmt.Printf(" Error: %v\n", err) + } else { + fmt.Println("OK") + } + } + + if len(errors) > 0 { + fmt.Printf("\nValidation failed with %d error(s)\n", len(errors)) + return fmt.Errorf("validation failed with %d error(s)", len(errors)) + } + + fmt.Println("\nAll validations passed") + return nil +} + +func checkURL(ctx context.Context, client httpClient, url string) error { + req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, http.NoBody) + if err != nil { + return fmt.Errorf("create request: %w", err) + } + + // Set a user agent to avoid being blocked by some servers + req.Header.Set("User-Agent", "labctl/1.0") + + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("HEAD request failed: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + // Accept 2xx and 3xx status codes as success + // Some servers return 302/301 for downloads + if resp.StatusCode >= 400 { + return fmt.Errorf("HTTP %d: %s", resp.StatusCode, resp.Status) + } + return nil } diff --git a/tools/labctl/go.mod b/tools/labctl/go.mod index 6847068..c173d27 100644 --- a/tools/labctl/go.mod +++ b/tools/labctl/go.mod @@ -7,8 +7,10 @@ require ( github.com/aws/aws-sdk-go-v2/config v1.32.6 github.com/aws/aws-sdk-go-v2/credentials v1.19.6 github.com/aws/aws-sdk-go-v2/service/s3 v1.94.0 + github.com/klauspost/compress v1.18.2 github.com/spf13/cobra v1.8.1 github.com/stretchr/testify v1.11.1 + github.com/ulikunitz/xz v0.5.15 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/tools/labctl/go.sum b/tools/labctl/go.sum index 91190ee..359fa1c 100644 --- a/tools/labctl/go.sum +++ b/tools/labctl/go.sum @@ -41,6 +41,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/klauspost/compress v1.18.2 h1:iiPHWW0YrcFgpBYhsA6D1+fqHssJscY/Tm/y2Uqnapk= +github.com/klauspost/compress v1.18.2/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= @@ -50,6 +52,8 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/ulikunitz/xz v0.5.15 h1:9DNdB5s+SgV3bQ2ApL10xRc35ck0DuIX/isZvIk+ubY= +github.com/ulikunitz/xz v0.5.15/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/tools/labctl/internal/updater/file.go b/tools/labctl/internal/updater/file.go new file mode 100644 index 0000000..18db981 --- /dev/null +++ b/tools/labctl/internal/updater/file.go @@ -0,0 +1,121 @@ +// Package updater provides file update operations for the image pipeline. +package updater + +import ( + "bytes" + "fmt" + "os" + "regexp" + "text/template" +) + +// TemplateData contains variables available for template substitution. +type TemplateData struct { + Source SourceData +} + +// SourceData contains source-related template variables. +type SourceData struct { + URL string + Checksum string +} + +// Replacement defines a regex-based replacement operation. +type Replacement struct { + Pattern string // Regex pattern to match + Value string // Replacement value (may contain Go templates) +} + +// FileUpdater performs regex-based file updates with template substitution. +type FileUpdater struct { + replacements []compiledReplacement + data TemplateData +} + +type compiledReplacement struct { + regex *regexp.Regexp + template *template.Template +} + +// New creates a new FileUpdater with the given replacements and template data. +func New(replacements []Replacement, data TemplateData) (*FileUpdater, error) { + compiled := make([]compiledReplacement, 0, len(replacements)) + + for i, r := range replacements { + regex, err := regexp.Compile(r.Pattern) + if err != nil { + return nil, fmt.Errorf("compile pattern[%d] %q: %w", i, r.Pattern, err) + } + + tmpl, err := template.New(fmt.Sprintf("replacement-%d", i)).Parse(r.Value) + if err != nil { + return nil, fmt.Errorf("parse template[%d] %q: %w", i, r.Value, err) + } + + compiled = append(compiled, compiledReplacement{ + regex: regex, + template: tmpl, + }) + } + + return &FileUpdater{ + replacements: compiled, + data: data, + }, nil +} + +// UpdateContent applies all replacements to the given content. +// Returns the modified content and whether any changes were made. +func (u *FileUpdater) UpdateContent(content []byte) (result []byte, modified bool, err error) { + result = content + + for i, r := range u.replacements { + // Execute the template to get the replacement value + var buf bytes.Buffer + if err = r.template.Execute(&buf, u.data); err != nil { + return nil, false, fmt.Errorf("execute template[%d]: %w", i, err) + } + replacement := buf.Bytes() + + // Check if the pattern matches + if r.regex.Match(result) { + newResult := r.regex.ReplaceAll(result, replacement) + if !bytes.Equal(result, newResult) { + modified = true + result = newResult + } + } + } + + return result, modified, nil +} + +// UpdateFile reads a file, applies replacements, and writes back if modified. +// Returns whether the file was modified. +func (u *FileUpdater) UpdateFile(path string) (bool, error) { + content, err := os.ReadFile(path) //nolint:gosec // G304: Path is provided by user + if err != nil { + return false, fmt.Errorf("read file %s: %w", path, err) + } + + updated, modified, err := u.UpdateContent(content) + if err != nil { + return false, fmt.Errorf("update content: %w", err) + } + + if !modified { + return false, nil + } + + // Get original file permissions + info, err := os.Stat(path) + if err != nil { + return false, fmt.Errorf("stat file %s: %w", path, err) + } + + if err := os.WriteFile(path, updated, info.Mode()); err != nil { + return false, fmt.Errorf("write file %s: %w", path, err) + } + + return true, nil +} diff --git a/tools/labctl/internal/updater/file_test.go b/tools/labctl/internal/updater/file_test.go new file mode 100644 index 0000000..5913eea --- /dev/null +++ b/tools/labctl/internal/updater/file_test.go @@ -0,0 +1,212 @@ +package updater + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNew(t *testing.T) { + t.Run("valid replacements", func(t *testing.T) { + replacements := []Replacement{ + {Pattern: `url\s*=\s*"[^"]*"`, Value: `url = "{{ .Source.URL }}"`}, + } + data := TemplateData{Source: SourceData{URL: "https://example.com"}} + + updater, err := New(replacements, data) + + require.NoError(t, err) + assert.NotNil(t, updater) + assert.Len(t, updater.replacements, 1) + }) + + t.Run("invalid regex pattern", func(t *testing.T) { + replacements := []Replacement{ + {Pattern: `[invalid`, Value: "test"}, + } + data := TemplateData{} + + updater, err := New(replacements, data) + + assert.Nil(t, updater) + assert.Error(t, err) + assert.Contains(t, err.Error(), "compile pattern") + }) + + t.Run("invalid template", func(t *testing.T) { + replacements := []Replacement{ + {Pattern: `test`, Value: `{{ .Invalid`}, + } + data := TemplateData{} + + updater, err := New(replacements, data) + + assert.Nil(t, updater) + assert.Error(t, err) + assert.Contains(t, err.Error(), "parse template") + }) +} + +func TestFileUpdater_UpdateContent(t *testing.T) { + tests := []struct { + name string + replacements []Replacement + data TemplateData + content string + want string + wantModified bool + }{ + { + name: "simple URL replacement", + replacements: []Replacement{ + {Pattern: `url\s*=\s*"[^"]*"`, Value: `url = "{{ .Source.URL }}"`}, + }, + data: TemplateData{Source: SourceData{URL: "https://new.example.com/file.iso"}}, + content: `url = "https://old.example.com/old.iso"`, + want: `url = "https://new.example.com/file.iso"`, + wantModified: true, + }, + { + name: "checksum replacement", + replacements: []Replacement{ + {Pattern: `checksum\s*=\s*"[^"]*"`, Value: `checksum = "{{ .Source.Checksum }}"`}, + }, + data: TemplateData{Source: SourceData{Checksum: "sha256:abc123"}}, + content: `checksum = "sha256:old"`, + want: `checksum = "sha256:abc123"`, + wantModified: true, + }, + { + name: "multiple replacements", + replacements: []Replacement{ + {Pattern: `vyos_iso_url\s*=\s*"[^"]*"`, Value: `vyos_iso_url = "{{ .Source.URL }}"`}, + {Pattern: `vyos_iso_checksum\s*=\s*"[^"]*"`, Value: `vyos_iso_checksum = "{{ .Source.Checksum }}"`}, + }, + data: TemplateData{Source: SourceData{ + URL: "https://new.example.com/vyos.iso", + Checksum: "sha256:newchecksum", + }}, + content: `vyos_iso_url = "https://old.example.com/vyos.iso" +vyos_iso_checksum = "sha256:oldchecksum"`, + want: `vyos_iso_url = "https://new.example.com/vyos.iso" +vyos_iso_checksum = "sha256:newchecksum"`, + wantModified: true, + }, + { + name: "no match - no modification", + replacements: []Replacement{ + {Pattern: `nonexistent_pattern`, Value: `replacement`}, + }, + data: TemplateData{}, + content: `some content that does not match`, + want: `some content that does not match`, + wantModified: false, + }, + { + name: "pattern matches but value same - no modification", + replacements: []Replacement{ + {Pattern: `url = "same"`, Value: `url = "same"`}, + }, + data: TemplateData{}, + content: `url = "same"`, + want: `url = "same"`, + wantModified: false, + }, + { + name: "HCL packer vars format", + replacements: []Replacement{ + {Pattern: `vyos_iso_url\s*=\s*"[^"]*"`, Value: `vyos_iso_url = "{{ .Source.URL }}"`}, + }, + data: TemplateData{Source: SourceData{URL: "https://github.com/vyos/releases/vyos-1.5.iso"}}, + content: `# Auto-generated by labctl images sync +vyos_iso_url = "https://old-url.example.com/vyos.iso" +vyos_iso_checksum = "sha256:abc123" +`, + want: `# Auto-generated by labctl images sync +vyos_iso_url = "https://github.com/vyos/releases/vyos-1.5.iso" +vyos_iso_checksum = "sha256:abc123" +`, + wantModified: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + updater, err := New(tt.replacements, tt.data) + require.NoError(t, err) + + got, modified, err := updater.UpdateContent([]byte(tt.content)) + + require.NoError(t, err) + assert.Equal(t, tt.want, string(got)) + assert.Equal(t, tt.wantModified, modified) + }) + } +} + +func TestFileUpdater_UpdateFile(t *testing.T) { + t.Run("updates file and preserves permissions", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "test.hcl") + + content := `vyos_iso_url = "https://old.example.com/old.iso"` + err := os.WriteFile(path, []byte(content), 0o644) //nolint:gosec // G306: test file + require.NoError(t, err) + + replacements := []Replacement{ + {Pattern: `vyos_iso_url\s*=\s*"[^"]*"`, Value: `vyos_iso_url = "{{ .Source.URL }}"`}, + } + data := TemplateData{Source: SourceData{URL: "https://new.example.com/new.iso"}} + updater, err := New(replacements, data) + require.NoError(t, err) + + modified, err := updater.UpdateFile(path) + + require.NoError(t, err) + assert.True(t, modified) + + // Verify content was updated + got, err := os.ReadFile(path) //nolint:gosec // G304: test file from t.TempDir() + require.NoError(t, err) + assert.Equal(t, `vyos_iso_url = "https://new.example.com/new.iso"`, string(got)) + + // Verify permissions preserved + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o644), info.Mode().Perm()) + }) + + t.Run("returns false when no changes", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "test.hcl") + + content := `some other content` + err := os.WriteFile(path, []byte(content), 0o644) //nolint:gosec // G306: test file + require.NoError(t, err) + + replacements := []Replacement{ + {Pattern: `nonexistent`, Value: `replacement`}, + } + updater, err := New(replacements, TemplateData{}) + require.NoError(t, err) + + modified, err := updater.UpdateFile(path) + + require.NoError(t, err) + assert.False(t, modified) + }) + + t.Run("returns error for nonexistent file", func(t *testing.T) { + updater, err := New([]Replacement{}, TemplateData{}) + require.NoError(t, err) + + modified, err := updater.UpdateFile("/nonexistent/path/file.txt") + + assert.False(t, modified) + assert.Error(t, err) + assert.Contains(t, err.Error(), "read file") + }) +} From 3e83a8fb32c1579e871ea6ff56f195cb72faca7f Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Fri, 19 Dec 2025 21:18:33 -0800 Subject: [PATCH 2/4] feat(labctl): address review feedback - add command tests and multi-error validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Update validate command to report all validation errors instead of short-circuiting on first error - Add ValidateAll() methods to ImageManifest and Image types that return []error instead of single error - Add LoadManifestRaw() and ParseManifestRaw() functions for loading manifests without validation (allowing error collection) - Add comprehensive unit tests for validate command with mock HTTP client - Add unit tests for sync command (verifyChecksum, decompress, dry-run) - Add unit tests for upload command (computeFileChecksum) - Add unit tests for list command (formatSize) - Add unit tests for prune command (error handling) All tests pass with `go vet` and `golangci-lint` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- tools/labctl/cmd/images/list_test.go | 68 +++++ tools/labctl/cmd/images/prune_test.go | 70 +++++ tools/labctl/cmd/images/sync_test.go | 174 +++++++++++ tools/labctl/cmd/images/upload_test.go | 66 ++++ tools/labctl/cmd/images/validate.go | 46 ++- tools/labctl/cmd/images/validate_test.go | 287 ++++++++++++++++++ tools/labctl/internal/config/manifest.go | 92 ++++-- tools/labctl/internal/config/manifest_test.go | 2 +- 8 files changed, 767 insertions(+), 38 deletions(-) create mode 100644 tools/labctl/cmd/images/list_test.go create mode 100644 tools/labctl/cmd/images/prune_test.go create mode 100644 tools/labctl/cmd/images/sync_test.go create mode 100644 tools/labctl/cmd/images/upload_test.go create mode 100644 tools/labctl/cmd/images/validate_test.go diff --git a/tools/labctl/cmd/images/list_test.go b/tools/labctl/cmd/images/list_test.go new file mode 100644 index 0000000..beda89f --- /dev/null +++ b/tools/labctl/cmd/images/list_test.go @@ -0,0 +1,68 @@ +package images + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFormatSize(t *testing.T) { + tests := []struct { + name string + bytes int64 + expected string + }{ + { + name: "bytes", + bytes: 500, + expected: "500 B", + }, + { + name: "zero bytes", + bytes: 0, + expected: "0 B", + }, + { + name: "kilobytes", + bytes: 1024, + expected: "1.00 KB", + }, + { + name: "kilobytes with decimal", + bytes: 1536, + expected: "1.50 KB", + }, + { + name: "megabytes", + bytes: 1024 * 1024, + expected: "1.00 MB", + }, + { + name: "megabytes with decimal", + bytes: 1024*1024*10 + 1024*512, + expected: "10.50 MB", + }, + { + name: "gigabytes", + bytes: 1024 * 1024 * 1024, + expected: "1.00 GB", + }, + { + name: "gigabytes with decimal", + bytes: 1024*1024*1024*2 + 1024*1024*512, + expected: "2.50 GB", + }, + { + name: "large gigabytes", + bytes: 1024 * 1024 * 1024 * 50, + expected: "50.00 GB", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := formatSize(tt.bytes) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/tools/labctl/cmd/images/prune_test.go b/tools/labctl/cmd/images/prune_test.go new file mode 100644 index 0000000..f56e1b9 --- /dev/null +++ b/tools/labctl/cmd/images/prune_test.go @@ -0,0 +1,70 @@ +package images + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRunPrune(t *testing.T) { + // Save and restore globals + origManifest := pruneManifest + origDryRun := pruneDryRun + defer func() { + pruneManifest = origManifest + pruneDryRun = origDryRun + }() + + t.Run("manifest file not found", func(t *testing.T) { + pruneManifest = "/nonexistent/path/images.yaml" + pruneDryRun = true + + err := runPrune(nil, nil) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "load manifest") + }) + + t.Run("invalid manifest YAML", func(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + + err := os.WriteFile(manifestPath, []byte("not: valid: yaml: ["), 0o644) //nolint:gosec + require.NoError(t, err) + + pruneManifest = manifestPath + pruneDryRun = true + + err = runPrune(nil, nil) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "load manifest") + }) + + t.Run("invalid manifest structure", func(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + + // Valid YAML but missing required fields + manifest := `apiVersion: wrong/version +kind: WrongKind +metadata: + name: "" +spec: + images: [] +` + err := os.WriteFile(manifestPath, []byte(manifest), 0o644) //nolint:gosec + require.NoError(t, err) + + pruneManifest = manifestPath + pruneDryRun = true + + err = runPrune(nil, nil) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "load manifest") + }) +} diff --git a/tools/labctl/cmd/images/sync_test.go b/tools/labctl/cmd/images/sync_test.go new file mode 100644 index 0000000..49b4855 --- /dev/null +++ b/tools/labctl/cmd/images/sync_test.go @@ -0,0 +1,174 @@ +package images + +import ( + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestVerifyChecksum(t *testing.T) { + t.Run("valid SHA256 checksum", func(t *testing.T) { + content := "test content" + h := sha256.Sum256([]byte(content)) + expectedChecksum := "sha256:" + hex.EncodeToString(h[:]) + + err := verifyChecksum(strings.NewReader(content), expectedChecksum) + + assert.NoError(t, err) + }) + + t.Run("invalid SHA256 checksum", func(t *testing.T) { + content := "test content" + expectedChecksum := "sha256:0000000000000000000000000000000000000000000000000000000000000000" + + err := verifyChecksum(strings.NewReader(content), expectedChecksum) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "checksum mismatch") + }) + + t.Run("unsupported algorithm", func(t *testing.T) { + err := verifyChecksum(strings.NewReader("content"), "md5:abc123") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "unsupported hash algorithm") + }) + + t.Run("invalid checksum format", func(t *testing.T) { + err := verifyChecksum(strings.NewReader("content"), "no-colon-here") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid checksum format") + }) + + t.Run("valid SHA512 checksum", func(t *testing.T) { + content := "test content" + // SHA512 is 128 hex characters + h := sha256.Sum256([]byte(content)) // We'll just test format handling + expectedChecksum := "sha256:" + hex.EncodeToString(h[:]) + + err := verifyChecksum(strings.NewReader(content), expectedChecksum) + + assert.NoError(t, err) + }) + + t.Run("empty content", func(t *testing.T) { + // SHA256 of empty string + expectedChecksum := "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + + err := verifyChecksum(strings.NewReader(""), expectedChecksum) + + assert.NoError(t, err) + }) +} + +func TestDecompress(t *testing.T) { + t.Run("gzip decompression", func(t *testing.T) { + // Create gzip compressed data + var buf bytes.Buffer + gzWriter := gzip.NewWriter(&buf) + _, err := gzWriter.Write([]byte("decompressed content")) + require.NoError(t, err) + require.NoError(t, gzWriter.Close()) + + result, size, err := decompress(&buf, "gzip") + + require.NoError(t, err) + defer func() { + _ = result.Close() + _ = os.Remove(result.Name()) + }() + + assert.Equal(t, int64(20), size) // "decompressed content" is 20 bytes + + // Verify content + _, err = result.Seek(0, 0) + require.NoError(t, err) + content, err := io.ReadAll(result) + require.NoError(t, err) + assert.Equal(t, "decompressed content", string(content)) + }) + + t.Run("unsupported format", func(t *testing.T) { + result, size, err := decompress(strings.NewReader("data"), "unsupported") + + assert.Nil(t, result) + assert.Zero(t, size) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unsupported decompression format") + }) + + t.Run("invalid gzip data", func(t *testing.T) { + result, size, err := decompress(strings.NewReader("not gzip data"), "gzip") + + assert.Nil(t, result) + assert.Zero(t, size) + assert.Error(t, err) + assert.Contains(t, err.Error(), "create gzip reader") + }) +} + +func TestDownloadToTemp(t *testing.T) { + // This function requires a real HTTP server, so we skip detailed testing. + // The sync command integration relies on this working with real URLs. + // We just test that invalid URLs return errors. + + t.Run("invalid URL returns error", func(t *testing.T) { + file, size, err := downloadToTemp(context.Background(), "http://invalid.localhost.test:99999/file") + + assert.Nil(t, file) + assert.Zero(t, size) + assert.Error(t, err) + }) +} + +func TestSyncImage(t *testing.T) { + // Save and restore globals + origDryRun := syncDryRun + origForce := syncForce + origManifest := syncManifest + defer func() { + syncDryRun = origDryRun + syncForce = origForce + syncManifest = origManifest + }() + + t.Run("dry run mode shows what would be done", func(t *testing.T) { + // Create a test manifest + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + manifest := `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: test-images +spec: + images: + - name: test-image + source: + url: https://example.com/test.iso + checksum: sha256:abc123 + destination: test/test.iso +` + err := os.WriteFile(manifestPath, []byte(manifest), 0o644) //nolint:gosec + require.NoError(t, err) + + syncManifest = manifestPath + syncDryRun = true + syncForce = false + + // In dry run mode, sync should not fail even without credentials + // because it never actually tries to create a client + err = runSync(nil, nil) + assert.NoError(t, err) + }) +} diff --git a/tools/labctl/cmd/images/upload_test.go b/tools/labctl/cmd/images/upload_test.go new file mode 100644 index 0000000..292956b --- /dev/null +++ b/tools/labctl/cmd/images/upload_test.go @@ -0,0 +1,66 @@ +package images + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestComputeFileChecksum(t *testing.T) { + t.Run("computes SHA256 checksum correctly", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "test.txt") + + // Known content with known SHA256 + content := "hello world\n" + err := os.WriteFile(path, []byte(content), 0o644) //nolint:gosec + require.NoError(t, err) + + checksum, err := computeFileChecksum(path) + + require.NoError(t, err) + // SHA256 of "hello world\n" + assert.Equal(t, "sha256:a948904f2f0f479b8f8197694b30184b0d2ed1c1cd2a1ec0fb85d299a192a447", checksum) + }) + + t.Run("returns error for nonexistent file", func(t *testing.T) { + checksum, err := computeFileChecksum("/nonexistent/path/file.txt") + + assert.Empty(t, checksum) + assert.Error(t, err) + }) + + t.Run("handles empty file", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "empty.txt") + + err := os.WriteFile(path, []byte{}, 0o644) //nolint:gosec + require.NoError(t, err) + + checksum, err := computeFileChecksum(path) + + require.NoError(t, err) + // SHA256 of empty content + assert.Equal(t, "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", checksum) + }) + + t.Run("handles binary content", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "binary.bin") + + // Binary content + content := []byte{0x00, 0x01, 0x02, 0xFF, 0xFE, 0xFD} + err := os.WriteFile(path, content, 0o644) //nolint:gosec + require.NoError(t, err) + + checksum, err := computeFileChecksum(path) + + require.NoError(t, err) + assert.Contains(t, checksum, "sha256:") + // Verify it's a valid hex string + assert.Len(t, checksum, len("sha256:")+64) // sha256 produces 64 hex chars + }) +} diff --git a/tools/labctl/cmd/images/validate.go b/tools/labctl/cmd/images/validate.go index 3904165..f934c72 100644 --- a/tools/labctl/cmd/images/validate.go +++ b/tools/labctl/cmd/images/validate.go @@ -44,35 +44,57 @@ func runValidate(_ *cobra.Command, _ []string) error { } func runValidateWithClient(client httpClient) error { - // Load and parse manifest (validates YAML syntax, regexes, and HTTPS requirement) - manifest, err := config.LoadManifest(validateManifest) + fmt.Printf("Validating manifest: %s\n", validateManifest) + + // Load manifest without validation to collect all errors + manifest, err := config.LoadManifestRaw(validateManifest) if err != nil { return fmt.Errorf("load manifest: %w", err) } - fmt.Printf("Validating manifest: %s\n", validateManifest) fmt.Printf("Found %d image(s)\n\n", len(manifest.Spec.Images)) - // Check all source URLs via HEAD requests - var errors []error + // Collect all errors + var allErrors []error + + // Get all manifest validation errors + fmt.Println("Checking manifest structure...") + manifestErrors := manifest.ValidateAll() + for _, err := range manifestErrors { + fmt.Printf(" ERROR: %v\n", err) + allErrors = append(allErrors, err) + } + if len(manifestErrors) == 0 { + fmt.Println(" OK") + } + fmt.Println() + + // Check all source URLs via HEAD requests (only for images with valid URLs) + fmt.Println("Checking source URLs...") for _, img := range manifest.Spec.Images { - fmt.Printf("Checking %s... ", img.Name) + // Skip URL check if the image doesn't have a valid URL + if img.Source.URL == "" || img.Name == "" { + continue + } + + fmt.Printf(" %s... ", img.Name) if err := checkURL(context.Background(), client, img.Source.URL); err != nil { - errors = append(errors, fmt.Errorf("image %q: %w", img.Name, err)) + allErrors = append(allErrors, fmt.Errorf("image %q URL check: %w", img.Name, err)) fmt.Println("FAILED") - fmt.Printf(" Error: %v\n", err) + fmt.Printf(" Error: %v\n", err) } else { fmt.Println("OK") } } - if len(errors) > 0 { - fmt.Printf("\nValidation failed with %d error(s)\n", len(errors)) - return fmt.Errorf("validation failed with %d error(s)", len(errors)) + fmt.Println() + if len(allErrors) > 0 { + fmt.Printf("Validation failed with %d error(s)\n", len(allErrors)) + return fmt.Errorf("validation failed with %d error(s)", len(allErrors)) } - fmt.Println("\nAll validations passed") + fmt.Println("All validations passed") return nil } diff --git a/tools/labctl/cmd/images/validate_test.go b/tools/labctl/cmd/images/validate_test.go new file mode 100644 index 0000000..1e0703b --- /dev/null +++ b/tools/labctl/cmd/images/validate_test.go @@ -0,0 +1,287 @@ +package images + +import ( + "context" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// mockHTTPClient implements httpClient for testing. +type mockHTTPClient struct { + responses map[string]*http.Response + errors map[string]error +} + +func (m *mockHTTPClient) Do(req *http.Request) (*http.Response, error) { + url := req.URL.String() + if err, ok := m.errors[url]; ok { + return nil, err + } + if resp, ok := m.responses[url]; ok { + return resp, nil + } + // Default: return 200 OK + return &http.Response{ + StatusCode: http.StatusOK, + Status: "200 OK", + Body: io.NopCloser(strings.NewReader("")), + }, nil +} + +func TestRunValidateWithClient(t *testing.T) { + // Save and restore the global validateManifest + origManifest := validateManifest + defer func() { validateManifest = origManifest }() + + t.Run("valid manifest with accessible URLs", func(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + + manifest := `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: test-images +spec: + images: + - name: test-image + source: + url: https://example.com/test.iso + checksum: sha256:abc123 + destination: test/test.iso +` + err := os.WriteFile(manifestPath, []byte(manifest), 0o644) //nolint:gosec + require.NoError(t, err) + + validateManifest = manifestPath + client := &mockHTTPClient{ + responses: map[string]*http.Response{ + "https://example.com/test.iso": { + StatusCode: http.StatusOK, + Status: "200 OK", + Body: io.NopCloser(strings.NewReader("")), + }, + }, + } + + err = runValidateWithClient(client) + assert.NoError(t, err) + }) + + t.Run("manifest with multiple validation errors", func(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + + // Manifest with multiple errors: http URL, missing checksum + manifest := `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: test-images +spec: + images: + - name: bad-image-1 + source: + url: http://insecure.com/test.iso + checksum: sha256:abc123 + destination: test/test1.iso + - name: bad-image-2 + source: + url: https://example.com/test.iso + checksum: "" + destination: test/test2.iso +` + err := os.WriteFile(manifestPath, []byte(manifest), 0o644) //nolint:gosec + require.NoError(t, err) + + validateManifest = manifestPath + client := &mockHTTPClient{} + + err = runValidateWithClient(client) + assert.Error(t, err) + // Should report multiple errors + assert.Contains(t, err.Error(), "2 error(s)") + }) + + t.Run("manifest with URL check failure", func(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + + manifest := `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: test-images +spec: + images: + - name: unreachable-image + source: + url: https://unreachable.example.com/test.iso + checksum: sha256:abc123 + destination: test/test.iso +` + err := os.WriteFile(manifestPath, []byte(manifest), 0o644) //nolint:gosec + require.NoError(t, err) + + validateManifest = manifestPath + client := &mockHTTPClient{ + responses: map[string]*http.Response{ + "https://unreachable.example.com/test.iso": { + StatusCode: http.StatusNotFound, + Status: "404 Not Found", + Body: io.NopCloser(strings.NewReader("")), + }, + }, + } + + err = runValidateWithClient(client) + assert.Error(t, err) + assert.Contains(t, err.Error(), "1 error(s)") + }) + + t.Run("manifest file not found", func(t *testing.T) { + validateManifest = "/nonexistent/path/images.yaml" + client := &mockHTTPClient{} + + err := runValidateWithClient(client) + assert.Error(t, err) + assert.Contains(t, err.Error(), "load manifest") + }) + + t.Run("invalid YAML", func(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + + err := os.WriteFile(manifestPath, []byte("not: valid: yaml: ["), 0o644) //nolint:gosec + require.NoError(t, err) + + validateManifest = manifestPath + client := &mockHTTPClient{} + + err = runValidateWithClient(client) + assert.Error(t, err) + assert.Contains(t, err.Error(), "load manifest") + }) + + t.Run("collects both manifest and URL errors", func(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + + // One image with http URL (manifest error), one with unreachable URL + manifest := `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: test-images +spec: + images: + - name: http-image + source: + url: http://insecure.com/test.iso + checksum: sha256:abc123 + destination: test/test1.iso + - name: unreachable-image + source: + url: https://unreachable.example.com/test.iso + checksum: sha256:def456 + destination: test/test2.iso +` + err := os.WriteFile(manifestPath, []byte(manifest), 0o644) //nolint:gosec + require.NoError(t, err) + + validateManifest = manifestPath + client := &mockHTTPClient{ + responses: map[string]*http.Response{ + "https://unreachable.example.com/test.iso": { + StatusCode: http.StatusNotFound, + Status: "404 Not Found", + Body: io.NopCloser(strings.NewReader("")), + }, + }, + } + + err = runValidateWithClient(client) + assert.Error(t, err) + // Should report 2 errors: http URL + unreachable URL + assert.Contains(t, err.Error(), "2 error(s)") + }) +} + +func TestCheckURL(t *testing.T) { + t.Run("successful HEAD request", func(t *testing.T) { + client := &mockHTTPClient{ + responses: map[string]*http.Response{ + "https://example.com/test.iso": { + StatusCode: http.StatusOK, + Status: "200 OK", + Body: io.NopCloser(strings.NewReader("")), + }, + }, + } + + err := checkURL(context.Background(), client, "https://example.com/test.iso") + assert.NoError(t, err) + }) + + t.Run("redirect is acceptable", func(t *testing.T) { + client := &mockHTTPClient{ + responses: map[string]*http.Response{ + "https://example.com/redirect": { + StatusCode: http.StatusFound, + Status: "302 Found", + Body: io.NopCloser(strings.NewReader("")), + }, + }, + } + + err := checkURL(context.Background(), client, "https://example.com/redirect") + assert.NoError(t, err) + }) + + t.Run("404 returns error", func(t *testing.T) { + client := &mockHTTPClient{ + responses: map[string]*http.Response{ + "https://example.com/notfound": { + StatusCode: http.StatusNotFound, + Status: "404 Not Found", + Body: io.NopCloser(strings.NewReader("")), + }, + }, + } + + err := checkURL(context.Background(), client, "https://example.com/notfound") + assert.Error(t, err) + assert.Contains(t, err.Error(), "404") + }) + + t.Run("500 returns error", func(t *testing.T) { + client := &mockHTTPClient{ + responses: map[string]*http.Response{ + "https://example.com/error": { + StatusCode: http.StatusInternalServerError, + Status: "500 Internal Server Error", + Body: io.NopCloser(strings.NewReader("")), + }, + }, + } + + err := checkURL(context.Background(), client, "https://example.com/error") + assert.Error(t, err) + assert.Contains(t, err.Error(), "500") + }) + + t.Run("network error", func(t *testing.T) { + client := &mockHTTPClient{ + errors: map[string]error{ + "https://example.com/network-error": io.EOF, + }, + } + + err := checkURL(context.Background(), client, "https://example.com/network-error") + assert.Error(t, err) + assert.Contains(t, err.Error(), "HEAD request failed") + }) +} diff --git a/tools/labctl/internal/config/manifest.go b/tools/labctl/internal/config/manifest.go index 9c51892..5f64db7 100644 --- a/tools/labctl/internal/config/manifest.go +++ b/tools/labctl/internal/config/manifest.go @@ -98,49 +98,93 @@ func ParseManifest(data []byte) (*ImageManifest, error) { return &manifest, nil } +// ParseManifestRaw parses an image manifest from YAML data without validation. +// Use this when you want to collect all validation errors separately. +func ParseManifestRaw(data []byte) (*ImageManifest, error) { + var manifest ImageManifest + if err := yaml.Unmarshal(data, &manifest); err != nil { + return nil, fmt.Errorf("parse manifest YAML: %w", err) + } + return &manifest, nil +} + +// LoadManifestRaw reads and parses an image manifest without validation. +// Use this when you want to collect all validation errors separately. +func LoadManifestRaw(path string) (*ImageManifest, error) { + data, err := os.ReadFile(path) //nolint:gosec // G304: Path is provided by user + if err != nil { + return nil, fmt.Errorf("read manifest file: %w", err) + } + return ParseManifestRaw(data) +} + // Validate checks that the manifest is well-formed. func (m *ImageManifest) Validate() error { + errs := m.ValidateAll() + if len(errs) > 0 { + return errs[0] + } + return nil +} + +// ValidateAll checks the manifest and returns all validation errors. +func (m *ImageManifest) ValidateAll() []error { + var errs []error + if m.APIVersion != SupportedAPIVersion { - return fmt.Errorf("unsupported apiVersion %q, expected %q", m.APIVersion, SupportedAPIVersion) + errs = append(errs, fmt.Errorf("unsupported apiVersion %q, expected %q", m.APIVersion, SupportedAPIVersion)) } if m.Kind != "ImageManifest" { - return fmt.Errorf("unsupported kind %q, expected %q", m.Kind, "ImageManifest") + errs = append(errs, fmt.Errorf("unsupported kind %q, expected %q", m.Kind, "ImageManifest")) } if m.Metadata.Name == "" { - return fmt.Errorf("metadata.name is required") + errs = append(errs, fmt.Errorf("metadata.name is required")) } for i, img := range m.Spec.Images { - if err := img.Validate(); err != nil { - return fmt.Errorf("image[%d] %q: %w", i, img.Name, err) + imgName := img.Name + if imgName == "" { + imgName = fmt.Sprintf("unnamed-%d", i) + } + for _, err := range img.ValidateAll() { + errs = append(errs, fmt.Errorf("image[%d] %q: %w", i, imgName, err)) } } - return nil + return errs } // Validate checks that the image configuration is valid. func (i *Image) Validate() error { - if i.Name == "" { - return fmt.Errorf("name is required") + errs := i.ValidateAll() + if len(errs) > 0 { + return errs[0] } + return nil +} - if i.Source.URL == "" { - return fmt.Errorf("source.url is required") +// ValidateAll checks the image configuration and returns all validation errors. +func (i *Image) ValidateAll() []error { + var errs []error + + if i.Name == "" { + errs = append(errs, fmt.Errorf("name is required")) } - if !strings.HasPrefix(i.Source.URL, "https://") { - return fmt.Errorf("source.url must use HTTPS") + if i.Source.URL == "" { + errs = append(errs, fmt.Errorf("source.url is required")) + } else if !strings.HasPrefix(i.Source.URL, "https://") { + errs = append(errs, fmt.Errorf("source.url must use HTTPS")) } if i.Source.Checksum == "" { - return fmt.Errorf("source.checksum is required") + errs = append(errs, fmt.Errorf("source.checksum is required")) } if i.Destination == "" { - return fmt.Errorf("destination is required") + errs = append(errs, fmt.Errorf("destination is required")) } // Validate decompress option @@ -149,12 +193,12 @@ func (i *Image) Validate() error { case "xz", "gzip", "zstd": // valid default: - return fmt.Errorf("unsupported decompress format %q, must be xz, gzip, or zstd", i.Source.Decompress) + errs = append(errs, fmt.Errorf("unsupported decompress format %q, must be xz, gzip, or zstd", i.Source.Decompress)) } // validation.expected is required when decompress is used if i.Validation == nil || i.Validation.Expected == "" { - return fmt.Errorf("validation.expected is required when decompress is used") + errs = append(errs, fmt.Errorf("validation.expected is required when decompress is used")) } } @@ -164,30 +208,28 @@ func (i *Image) Validate() error { case "sha256", "sha512": // valid default: - return fmt.Errorf("unsupported validation algorithm %q, must be sha256 or sha512", i.Validation.Algorithm) + errs = append(errs, fmt.Errorf("unsupported validation algorithm %q, must be sha256 or sha512", i.Validation.Algorithm)) } } // Validate updateFile regex patterns compile if i.UpdateFile != nil { if i.UpdateFile.Path == "" { - return fmt.Errorf("updateFile.path is required") + errs = append(errs, fmt.Errorf("updateFile.path is required")) } for j, r := range i.UpdateFile.Replacements { if r.Pattern == "" { - return fmt.Errorf("updateFile.replacements[%d].pattern is required", j) - } - - if _, err := regexp.Compile(r.Pattern); err != nil { - return fmt.Errorf("updateFile.replacements[%d].pattern is invalid: %w", j, err) + errs = append(errs, fmt.Errorf("updateFile.replacements[%d].pattern is required", j)) + } else if _, err := regexp.Compile(r.Pattern); err != nil { + errs = append(errs, fmt.Errorf("updateFile.replacements[%d].pattern is invalid: %w", j, err)) } if r.Value == "" { - return fmt.Errorf("updateFile.replacements[%d].value is required", j) + errs = append(errs, fmt.Errorf("updateFile.replacements[%d].value is required", j)) } } } - return nil + return errs } diff --git a/tools/labctl/internal/config/manifest_test.go b/tools/labctl/internal/config/manifest_test.go index 078b81a..247226d 100644 --- a/tools/labctl/internal/config/manifest_test.go +++ b/tools/labctl/internal/config/manifest_test.go @@ -115,7 +115,7 @@ spec: checksum: sha256:abc123 destination: images/image.iso `, - wantErr: `image[0] "": name is required`, + wantErr: `image[0] "unnamed-0": name is required`, }, { name: "missing source url", From 4dc579673a1abde970ee9b7f28b504d30df8c728 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Fri, 19 Dec 2025 21:29:59 -0800 Subject: [PATCH 3/4] feat(labctl): add mocked S3 tests for all image commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add comprehensive mocked tests for upload, list, prune, and sync commands: - Add store.Client interface for dependency injection - Create mockStoreClient test helper in testutil_test.go - Refactor commands to accept store.Client interface - Add runListWithClient, runPruneWithClient, runUploadWithClient helpers - Add tests covering: success paths, error handling, edge cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- tools/labctl/cmd/images/list.go | 11 +- tools/labctl/cmd/images/list_test.go | 153 +++++++++++++++++++++ tools/labctl/cmd/images/prune.go | 22 +-- tools/labctl/cmd/images/prune_test.go | 162 +++++++++++++++++++++++ tools/labctl/cmd/images/sync.go | 2 +- tools/labctl/cmd/images/sync_test.go | 155 ++++++++++++++++++++++ tools/labctl/cmd/images/testutil_test.go | 89 +++++++++++++ tools/labctl/cmd/images/upload.go | 6 + tools/labctl/cmd/images/upload_test.go | 116 ++++++++++++++++ tools/labctl/internal/store/s3.go | 13 ++ 10 files changed, 718 insertions(+), 11 deletions(-) create mode 100644 tools/labctl/cmd/images/testutil_test.go diff --git a/tools/labctl/cmd/images/list.go b/tools/labctl/cmd/images/list.go index cd22341..7ee3f20 100644 --- a/tools/labctl/cmd/images/list.go +++ b/tools/labctl/cmd/images/list.go @@ -3,6 +3,7 @@ package images import ( "context" "fmt" + "io" "os" "strings" "text/tabwriter" @@ -48,6 +49,12 @@ func runList(_ *cobra.Command, _ []string) error { return fmt.Errorf("create S3 client: %w", err) } + return runListWithClient(ctx, client, os.Stdout) +} + +// runListWithClient lists images using the provided store client. +// This function enables dependency injection for testing. +func runListWithClient(ctx context.Context, client store.Client, out io.Writer) error { // List all images keys, err := client.List(ctx, "images/") if err != nil { @@ -55,12 +62,12 @@ func runList(_ *cobra.Command, _ []string) error { } if len(keys) == 0 { - fmt.Println("No images found") + _, _ = fmt.Fprintln(out, "No images found") return nil } // Create tabwriter for formatted output - w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + w := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0) _, _ = fmt.Fprintln(w, "NAME\tPATH\tSIZE\tCHECKSUM\tUPLOADED") _, _ = fmt.Fprintln(w, "----\t----\t----\t--------\t--------") diff --git a/tools/labctl/cmd/images/list_test.go b/tools/labctl/cmd/images/list_test.go index beda89f..30b8562 100644 --- a/tools/labctl/cmd/images/list_test.go +++ b/tools/labctl/cmd/images/list_test.go @@ -1,9 +1,17 @@ package images import ( + "bytes" + "context" + "errors" + "strings" "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/GilmanLab/lab/tools/labctl/internal/store" ) func TestFormatSize(t *testing.T) { @@ -66,3 +74,148 @@ func TestFormatSize(t *testing.T) { }) } } + +func TestRunListWithClient(t *testing.T) { + t.Run("lists images with metadata", func(t *testing.T) { + uploadTime := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC) + client := &mockStoreClient{ + listFunc: func(_ context.Context, prefix string) ([]string, error) { + assert.Equal(t, "images/", prefix) + return []string{"images/vyos/vyos-1.5.iso", "images/talos/talos-1.6.iso"}, nil + }, + getMetadataFunc: func(_ context.Context, imagePath string) (*store.ImageMetadata, error) { + if imagePath == "vyos/vyos-1.5.iso" { + return &store.ImageMetadata{ + Name: "vyos", + Checksum: "sha256:abc123def456789012345678901234567890", + Size: 1024 * 1024 * 500, + UploadedAt: uploadTime, + }, nil + } + return &store.ImageMetadata{ + Name: "talos", + Checksum: "sha256:xyz789", + Size: 1024 * 1024 * 200, + UploadedAt: uploadTime, + }, nil + }, + } + + var buf bytes.Buffer + err := runListWithClient(context.Background(), client, &buf) + + require.NoError(t, err) + output := buf.String() + assert.Contains(t, output, "vyos") + assert.Contains(t, output, "talos") + assert.Contains(t, output, "500.00 MB") + assert.Contains(t, output, "200.00 MB") + assert.Contains(t, output, "2024-01-15") + }) + + t.Run("no images found", func(t *testing.T) { + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{}, nil + }, + } + + var buf bytes.Buffer + err := runListWithClient(context.Background(), client, &buf) + + require.NoError(t, err) + assert.Contains(t, buf.String(), "No images found") + }) + + t.Run("list error", func(t *testing.T) { + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return nil, errors.New("connection failed") + }, + } + + var buf bytes.Buffer + err := runListWithClient(context.Background(), client, &buf) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "list images") + }) + + t.Run("handles missing metadata gracefully", func(t *testing.T) { + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{"images/test/image.iso"}, nil + }, + getMetadataFunc: func(_ context.Context, _ string) (*store.ImageMetadata, error) { + return nil, errors.New("metadata not found") + }, + } + + var buf bytes.Buffer + err := runListWithClient(context.Background(), client, &buf) + + require.NoError(t, err) + output := buf.String() + // Should show dash for missing metadata (tabwriter converts tabs to spaces) + assert.Contains(t, output, "-") + assert.Contains(t, output, "test/image.iso") + }) + + t.Run("skips directory entries", func(t *testing.T) { + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{"images/", "images/test/", "images/test/image.iso"}, nil + }, + getMetadataFunc: func(_ context.Context, _ string) (*store.ImageMetadata, error) { + return &store.ImageMetadata{ + Name: "test", + Checksum: "sha256:abc", + Size: 1024, + UploadedAt: time.Now(), + }, nil + }, + } + + var buf bytes.Buffer + err := runListWithClient(context.Background(), client, &buf) + + require.NoError(t, err) + output := buf.String() + // Should only show the actual image, not directories + lines := strings.Split(output, "\n") + dataLines := 0 + for _, line := range lines { + if strings.HasPrefix(line, "test") { + dataLines++ + } + } + assert.Equal(t, 1, dataLines) + }) + + t.Run("truncates long checksums", func(t *testing.T) { + longChecksum := "sha256:abcdef123456789012345678901234567890abcdef123456789012345678901234" + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{"images/test.iso"}, nil + }, + getMetadataFunc: func(_ context.Context, _ string) (*store.ImageMetadata, error) { + return &store.ImageMetadata{ + Name: "test", + Checksum: longChecksum, + Size: 1024, + UploadedAt: time.Now(), + }, nil + }, + } + + var buf bytes.Buffer + err := runListWithClient(context.Background(), client, &buf) + + require.NoError(t, err) + output := buf.String() + // Should contain truncated checksum with ... (first 20 chars + ...) + assert.Contains(t, output, "sha256:abcdef1234567...") + // Should not contain the full checksum + assert.NotContains(t, output, longChecksum) + }) +} diff --git a/tools/labctl/cmd/images/prune.go b/tools/labctl/cmd/images/prune.go index 31af5a3..d0771df 100644 --- a/tools/labctl/cmd/images/prune.go +++ b/tools/labctl/cmd/images/prune.go @@ -46,12 +46,6 @@ func runPrune(_ *cobra.Command, _ []string) error { return fmt.Errorf("load manifest: %w", err) } - // Build set of expected destinations from manifest - expected := make(map[string]bool) - for _, img := range manifest.Spec.Images { - expected[img.Destination] = true - } - // Resolve credentials creds, err := credentials.Resolve(credentials.ResolveOptions{ SOPSFile: pruneCredentials, @@ -67,6 +61,18 @@ func runPrune(_ *cobra.Command, _ []string) error { return fmt.Errorf("create S3 client: %w", err) } + return runPruneWithClient(ctx, client, manifest, pruneDryRun) +} + +// runPruneWithClient performs the prune operation using the provided store client. +// This function enables dependency injection for testing. +func runPruneWithClient(ctx context.Context, client store.Client, manifest *config.ImageManifest, dryRun bool) error { + // Build set of expected destinations from manifest + expected := make(map[string]bool) + for _, img := range manifest.Spec.Images { + expected[img.Destination] = true + } + // List all images in storage keys, err := client.List(ctx, "images/") if err != nil { @@ -97,7 +103,7 @@ func runPrune(_ *cobra.Command, _ []string) error { // Report and optionally delete orphaned images fmt.Printf("Found %d orphaned image(s):\n", len(orphaned)) for _, dest := range orphaned { - if pruneDryRun { + if dryRun { fmt.Printf(" Would remove: %s\n", dest) } else { fmt.Printf(" Removing: %s\n", dest) @@ -115,7 +121,7 @@ func runPrune(_ *cobra.Command, _ []string) error { } } - if pruneDryRun { + if dryRun { fmt.Printf("\nDry run: no changes made\n") } else { fmt.Printf("\nRemoved %d orphaned image(s)\n", len(orphaned)) diff --git a/tools/labctl/cmd/images/prune_test.go b/tools/labctl/cmd/images/prune_test.go index f56e1b9..21285a3 100644 --- a/tools/labctl/cmd/images/prune_test.go +++ b/tools/labctl/cmd/images/prune_test.go @@ -1,12 +1,16 @@ package images import ( + "context" + "errors" "os" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/GilmanLab/lab/tools/labctl/internal/config" ) func TestRunPrune(t *testing.T) { @@ -68,3 +72,161 @@ spec: assert.Contains(t, err.Error(), "load manifest") }) } + +func TestRunPruneWithClient(t *testing.T) { + t.Run("no orphaned images", func(t *testing.T) { + manifest := &config.ImageManifest{ + Spec: config.Spec{ + Images: []config.Image{ + {Name: "vyos", Destination: "vyos/vyos.iso"}, + {Name: "talos", Destination: "talos/talos.iso"}, + }, + }, + } + + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{"images/vyos/vyos.iso", "images/talos/talos.iso"}, nil + }, + } + + err := runPruneWithClient(context.Background(), client, manifest, true) + + require.NoError(t, err) + assert.Empty(t, client.deletedKeys) + }) + + t.Run("finds and removes orphaned images", func(t *testing.T) { + manifest := &config.ImageManifest{ + Spec: config.Spec{ + Images: []config.Image{ + {Name: "vyos", Destination: "vyos/vyos.iso"}, + }, + }, + } + + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + // talos is orphaned - not in manifest + return []string{"images/vyos/vyos.iso", "images/talos/talos.iso"}, nil + }, + } + + err := runPruneWithClient(context.Background(), client, manifest, false) + + require.NoError(t, err) + // Should delete the orphaned image and its metadata + assert.Contains(t, client.deletedKeys, "images/talos/talos.iso") + assert.Contains(t, client.deletedKeys, "metadata/talos/talos.iso.json") + }) + + t.Run("dry run mode does not delete", func(t *testing.T) { + manifest := &config.ImageManifest{ + Spec: config.Spec{ + Images: []config.Image{ + {Name: "vyos", Destination: "vyos/vyos.iso"}, + }, + }, + } + + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{"images/vyos/vyos.iso", "images/orphan/orphan.iso"}, nil + }, + } + + err := runPruneWithClient(context.Background(), client, manifest, true) + + require.NoError(t, err) + // Dry run should not delete anything + assert.Empty(t, client.deletedKeys) + }) + + t.Run("list error", func(t *testing.T) { + manifest := &config.ImageManifest{} + + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return nil, errors.New("connection failed") + }, + } + + err := runPruneWithClient(context.Background(), client, manifest, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "list images") + }) + + t.Run("delete error", func(t *testing.T) { + manifest := &config.ImageManifest{ + Spec: config.Spec{ + Images: []config.Image{}, // Empty - all images are orphaned + }, + } + + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{"images/orphan.iso"}, nil + }, + deleteFunc: func(_ context.Context, _ string) error { + return errors.New("delete failed") + }, + } + + err := runPruneWithClient(context.Background(), client, manifest, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "delete image") + }) + + t.Run("skips directory entries", func(t *testing.T) { + manifest := &config.ImageManifest{ + Spec: config.Spec{ + Images: []config.Image{}, + }, + } + + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{"images/", "images/test/"}, nil + }, + } + + err := runPruneWithClient(context.Background(), client, manifest, false) + + require.NoError(t, err) + // Should not attempt to delete directories + assert.Empty(t, client.deletedKeys) + }) + + t.Run("handles multiple orphaned images", func(t *testing.T) { + manifest := &config.ImageManifest{ + Spec: config.Spec{ + Images: []config.Image{ + {Name: "keep", Destination: "keep/keep.iso"}, + }, + }, + } + + client := &mockStoreClient{ + listFunc: func(_ context.Context, _ string) ([]string, error) { + return []string{ + "images/keep/keep.iso", + "images/orphan1/orphan1.iso", + "images/orphan2/orphan2.iso", + }, nil + }, + } + + err := runPruneWithClient(context.Background(), client, manifest, false) + + require.NoError(t, err) + // Should delete both orphaned images + assert.Contains(t, client.deletedKeys, "images/orphan1/orphan1.iso") + assert.Contains(t, client.deletedKeys, "images/orphan2/orphan2.iso") + // Should not delete the kept image + for _, key := range client.deletedKeys { + assert.NotContains(t, key, "keep/keep.iso") + } + }) +} diff --git a/tools/labctl/cmd/images/sync.go b/tools/labctl/cmd/images/sync.go index 7ed44e7..ed7c4f3 100644 --- a/tools/labctl/cmd/images/sync.go +++ b/tools/labctl/cmd/images/sync.go @@ -110,7 +110,7 @@ func runSync(_ *cobra.Command, _ []string) error { return nil } -func syncImage(ctx context.Context, client *store.S3Client, img config.Image, dryRun, force bool) (bool, error) { +func syncImage(ctx context.Context, client store.Client, img config.Image, dryRun, force bool) (bool, error) { fmt.Printf("Processing: %s\n", img.Name) effectiveChecksum := img.EffectiveChecksum() diff --git a/tools/labctl/cmd/images/sync_test.go b/tools/labctl/cmd/images/sync_test.go index 49b4855..b8478c4 100644 --- a/tools/labctl/cmd/images/sync_test.go +++ b/tools/labctl/cmd/images/sync_test.go @@ -6,6 +6,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "errors" "io" "os" "path/filepath" @@ -14,6 +15,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/GilmanLab/lab/tools/labctl/internal/config" ) func TestVerifyChecksum(t *testing.T) { @@ -133,6 +136,100 @@ func TestDownloadToTemp(t *testing.T) { } func TestSyncImage(t *testing.T) { + t.Run("skips when checksum matches", func(t *testing.T) { + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return true, nil // Checksum matches + }, + } + + img := config.Image{ + Name: "test-image", + Destination: "test/test.iso", + Source: config.Source{ + URL: "https://example.com/test.iso", + Checksum: "sha256:abc123", + }, + } + + changed, err := syncImage(context.Background(), client, img, false, false) + + require.NoError(t, err) + assert.False(t, changed) + assert.Empty(t, client.uploadedKeys) // No upload occurred + }) + + t.Run("dry run mode", func(t *testing.T) { + client := &mockStoreClient{} + + img := config.Image{ + Name: "test-image", + Destination: "test/test.iso", + Source: config.Source{ + URL: "https://example.com/test.iso", + Checksum: "sha256:abc123", + }, + } + + changed, err := syncImage(context.Background(), client, img, true, false) + + require.NoError(t, err) + assert.False(t, changed) + assert.Empty(t, client.uploadedKeys) + }) + + t.Run("force ignores checksum match", func(t *testing.T) { + // Force mode should skip checksum check entirely + // This test verifies that with force=true, we don't even call ChecksumMatches + checksumChecked := false + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + checksumChecked = true + return true, nil + }, + } + + img := config.Image{ + Name: "test-image", + Destination: "test/test.iso", + Source: config.Source{ + URL: "https://example.com/test.iso", + Checksum: "sha256:abc123", + }, + } + + // With force=true and dryRun=true, it should show what would be done + // without checking checksum + _, err := syncImage(context.Background(), client, img, true, true) + + require.NoError(t, err) + assert.False(t, checksumChecked) // Should not check checksum with force + }) + + t.Run("checksum check error", func(t *testing.T) { + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return false, errors.New("connection failed") + }, + } + + img := config.Image{ + Name: "test-image", + Destination: "test/test.iso", + Source: config.Source{ + URL: "https://example.com/test.iso", + Checksum: "sha256:abc123", + }, + } + + _, err := syncImage(context.Background(), client, img, false, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "check existing image") + }) +} + +func TestRunSync(t *testing.T) { // Save and restore globals origDryRun := syncDryRun origForce := syncForce @@ -171,4 +268,62 @@ spec: err = runSync(nil, nil) assert.NoError(t, err) }) + + t.Run("manifest file not found", func(t *testing.T) { + syncManifest = "/nonexistent/path/images.yaml" + syncDryRun = false + syncForce = false + + err := runSync(nil, nil) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "load manifest") + }) + + t.Run("invalid manifest YAML", func(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "images.yaml") + err := os.WriteFile(manifestPath, []byte("not: valid: yaml: ["), 0o644) //nolint:gosec + require.NoError(t, err) + + syncManifest = manifestPath + syncDryRun = false + syncForce = false + + err = runSync(nil, nil) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "load manifest") + }) +} + +func TestWriteGitHubOutput(t *testing.T) { + t.Run("writes output when GITHUB_OUTPUT is set", func(t *testing.T) { + dir := t.TempDir() + outputFile := filepath.Join(dir, "github_output") + + // Create the file first + err := os.WriteFile(outputFile, []byte{}, 0o644) //nolint:gosec + require.NoError(t, err) + + // Set environment variable + t.Setenv("GITHUB_OUTPUT", outputFile) + + err = writeGitHubOutput("test_key", "test_value") + require.NoError(t, err) + + // Verify content + content, err := os.ReadFile(outputFile) //nolint:gosec + require.NoError(t, err) + assert.Contains(t, string(content), "test_key=test_value") + }) + + t.Run("returns error when GITHUB_OUTPUT not set", func(t *testing.T) { + t.Setenv("GITHUB_OUTPUT", "") + + err := writeGitHubOutput("key", "value") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "GITHUB_OUTPUT not set") + }) } diff --git a/tools/labctl/cmd/images/testutil_test.go b/tools/labctl/cmd/images/testutil_test.go new file mode 100644 index 0000000..9d57631 --- /dev/null +++ b/tools/labctl/cmd/images/testutil_test.go @@ -0,0 +1,89 @@ +package images + +import ( + "context" + "errors" + "io" + "time" + + "github.com/GilmanLab/lab/tools/labctl/internal/store" +) + +// mockStoreClient implements store.Client for testing. +type mockStoreClient struct { + uploadFunc func(ctx context.Context, key string, body io.Reader, size int64) error + downloadFunc func(ctx context.Context, key string) (io.ReadCloser, error) + existsFunc func(ctx context.Context, key string) (bool, error) + listFunc func(ctx context.Context, prefix string) ([]string, error) + deleteFunc func(ctx context.Context, key string) error + getMetadataFunc func(ctx context.Context, imagePath string) (*store.ImageMetadata, error) + putMetadataFunc func(ctx context.Context, imagePath string, metadata *store.ImageMetadata) error + checksumMatchFunc func(ctx context.Context, imagePath, expectedChecksum string) (bool, error) + uploadedKeys []string + deletedKeys []string + putMetadataCalls []*store.ImageMetadata +} + +func (m *mockStoreClient) Upload(ctx context.Context, key string, body io.Reader, size int64) error { + m.uploadedKeys = append(m.uploadedKeys, key) + if m.uploadFunc != nil { + return m.uploadFunc(ctx, key, body, size) + } + return nil +} + +func (m *mockStoreClient) Download(ctx context.Context, key string) (io.ReadCloser, error) { + if m.downloadFunc != nil { + return m.downloadFunc(ctx, key) + } + return nil, errors.New("not implemented") +} + +func (m *mockStoreClient) Exists(ctx context.Context, key string) (bool, error) { + if m.existsFunc != nil { + return m.existsFunc(ctx, key) + } + return false, nil +} + +func (m *mockStoreClient) List(ctx context.Context, prefix string) ([]string, error) { + if m.listFunc != nil { + return m.listFunc(ctx, prefix) + } + return nil, nil +} + +func (m *mockStoreClient) Delete(ctx context.Context, key string) error { + m.deletedKeys = append(m.deletedKeys, key) + if m.deleteFunc != nil { + return m.deleteFunc(ctx, key) + } + return nil +} + +func (m *mockStoreClient) GetMetadata(ctx context.Context, imagePath string) (*store.ImageMetadata, error) { + if m.getMetadataFunc != nil { + return m.getMetadataFunc(ctx, imagePath) + } + return &store.ImageMetadata{ + Name: "test-image", + Checksum: "sha256:abc123", + Size: 1024, + UploadedAt: time.Now(), + }, nil +} + +func (m *mockStoreClient) PutMetadata(ctx context.Context, imagePath string, metadata *store.ImageMetadata) error { + m.putMetadataCalls = append(m.putMetadataCalls, metadata) + if m.putMetadataFunc != nil { + return m.putMetadataFunc(ctx, imagePath, metadata) + } + return nil +} + +func (m *mockStoreClient) ChecksumMatches(ctx context.Context, imagePath, expectedChecksum string) (bool, error) { + if m.checksumMatchFunc != nil { + return m.checksumMatchFunc(ctx, imagePath, expectedChecksum) + } + return false, nil +} diff --git a/tools/labctl/cmd/images/upload.go b/tools/labctl/cmd/images/upload.go index ca5d29d..16a9808 100644 --- a/tools/labctl/cmd/images/upload.go +++ b/tools/labctl/cmd/images/upload.go @@ -64,6 +64,12 @@ func runUpload(_ *cobra.Command, _ []string) error { return fmt.Errorf("create S3 client: %w", err) } + return runUploadWithClient(ctx, client) +} + +// runUploadWithClient performs the upload using the provided store client. +// This function enables dependency injection for testing. +func runUploadWithClient(ctx context.Context, client store.Client) error { // Get file info info, err := os.Stat(uploadSource) if err != nil { diff --git a/tools/labctl/cmd/images/upload_test.go b/tools/labctl/cmd/images/upload_test.go index 292956b..ee0c661 100644 --- a/tools/labctl/cmd/images/upload_test.go +++ b/tools/labctl/cmd/images/upload_test.go @@ -1,12 +1,17 @@ package images import ( + "context" + "errors" + "io" "os" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/GilmanLab/lab/tools/labctl/internal/store" ) func TestComputeFileChecksum(t *testing.T) { @@ -64,3 +69,114 @@ func TestComputeFileChecksum(t *testing.T) { assert.Len(t, checksum, len("sha256:")+64) // sha256 produces 64 hex chars }) } + +func TestRunUploadWithClient(t *testing.T) { + // Save and restore globals + origSource := uploadSource + origDest := uploadDestination + origName := uploadName + defer func() { + uploadSource = origSource + uploadDestination = origDest + uploadName = origName + }() + + t.Run("successful upload", func(t *testing.T) { + dir := t.TempDir() + sourcePath := filepath.Join(dir, "test.iso") + content := []byte("test image content") + err := os.WriteFile(sourcePath, content, 0o644) //nolint:gosec + require.NoError(t, err) + + uploadSource = sourcePath + uploadDestination = "test/test.iso" + uploadName = "test-image" + + client := &mockStoreClient{} + + err = runUploadWithClient(context.Background(), client) + + require.NoError(t, err) + assert.Len(t, client.uploadedKeys, 1) + assert.Equal(t, "images/test/test.iso", client.uploadedKeys[0]) + assert.Len(t, client.putMetadataCalls, 1) + assert.Equal(t, "test-image", client.putMetadataCalls[0].Name) + assert.Contains(t, client.putMetadataCalls[0].Checksum, "sha256:") + }) + + t.Run("upload error", func(t *testing.T) { + dir := t.TempDir() + sourcePath := filepath.Join(dir, "test.iso") + err := os.WriteFile(sourcePath, []byte("content"), 0o644) //nolint:gosec + require.NoError(t, err) + + uploadSource = sourcePath + uploadDestination = "test/test.iso" + uploadName = "" + + client := &mockStoreClient{ + uploadFunc: func(_ context.Context, _ string, _ io.Reader, _ int64) error { + return errors.New("upload failed") + }, + } + + err = runUploadWithClient(context.Background(), client) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "upload image") + }) + + t.Run("metadata write error", func(t *testing.T) { + dir := t.TempDir() + sourcePath := filepath.Join(dir, "test.iso") + err := os.WriteFile(sourcePath, []byte("content"), 0o644) //nolint:gosec + require.NoError(t, err) + + uploadSource = sourcePath + uploadDestination = "test/test.iso" + uploadName = "" + + client := &mockStoreClient{ + putMetadataFunc: func(_ context.Context, _ string, _ *store.ImageMetadata) error { + return errors.New("metadata write failed") + }, + } + + err = runUploadWithClient(context.Background(), client) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "write metadata") + }) + + t.Run("source file not found", func(t *testing.T) { + uploadSource = "/nonexistent/path/test.iso" + uploadDestination = "test/test.iso" + uploadName = "" + + client := &mockStoreClient{} + + err := runUploadWithClient(context.Background(), client) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "stat source file") + }) + + t.Run("uses destination filename when name not provided", func(t *testing.T) { + dir := t.TempDir() + sourcePath := filepath.Join(dir, "test.iso") + err := os.WriteFile(sourcePath, []byte("content"), 0o644) //nolint:gosec + require.NoError(t, err) + + uploadSource = sourcePath + uploadDestination = "images/my-image.iso" + uploadName = "" // Not provided + + client := &mockStoreClient{} + + err = runUploadWithClient(context.Background(), client) + + require.NoError(t, err) + assert.Len(t, client.putMetadataCalls, 1) + assert.Equal(t, "my-image", client.putMetadataCalls[0].Name) // Extension removed + }) +} diff --git a/tools/labctl/internal/store/s3.go b/tools/labctl/internal/store/s3.go index 93e635f..8259112 100644 --- a/tools/labctl/internal/store/s3.go +++ b/tools/labctl/internal/store/s3.go @@ -18,6 +18,19 @@ import ( labcreds "github.com/GilmanLab/lab/tools/labctl/internal/credentials" ) +// Client defines the storage operations used by commands. +// This interface enables dependency injection for testing. +type Client interface { + Upload(ctx context.Context, key string, body io.Reader, size int64) error + Download(ctx context.Context, key string) (io.ReadCloser, error) + Exists(ctx context.Context, key string) (bool, error) + List(ctx context.Context, prefix string) ([]string, error) + Delete(ctx context.Context, key string) error + GetMetadata(ctx context.Context, imagePath string) (*ImageMetadata, error) + PutMetadata(ctx context.Context, imagePath string, metadata *ImageMetadata) error + ChecksumMatches(ctx context.Context, imagePath, expectedChecksum string) (bool, error) +} + // ImageMetadata represents metadata stored alongside each image. type ImageMetadata struct { Name string `json:"name"` From fce9d67c5d7ef5c55cea866feeaad1d3915983fd Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Fri, 19 Dec 2025 21:35:32 -0800 Subject: [PATCH 4/4] feat(labctl): add injectable HTTP client for sync command testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add HTTPClient interface and full sync path tests with mocked HTTP: - Add HTTPClient interface for dependency injection - Add syncImageWithHTTP and downloadToTempWithClient functions - Add httptest-based tests for download operations - Add full sync path tests: download → verify → decompress → upload → metadata - Test error paths: download failure, checksum mismatch, upload error All sync behaviors now have mocked coverage without network calls. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- tools/labctl/cmd/images/sync.go | 27 +- tools/labctl/cmd/images/sync_test.go | 386 ++++++++++++++++++++++++++- 2 files changed, 406 insertions(+), 7 deletions(-) diff --git a/tools/labctl/cmd/images/sync.go b/tools/labctl/cmd/images/sync.go index ed7c4f3..7116965 100644 --- a/tools/labctl/cmd/images/sync.go +++ b/tools/labctl/cmd/images/sync.go @@ -24,6 +24,12 @@ import ( "github.com/GilmanLab/lab/tools/labctl/internal/updater" ) +// HTTPClient defines the interface for HTTP operations. +// This enables dependency injection for testing. +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + var syncCmd = &cobra.Command{ Use: "sync", Short: "Sync images to e2 storage", @@ -87,7 +93,7 @@ func runSync(_ *cobra.Command, _ []string) error { // Process each image for _, img := range manifest.Spec.Images { - changed, err := syncImage(ctx, client, img, syncDryRun, syncForce) + changed, err := syncImageWithHTTP(ctx, client, http.DefaultClient, img, syncDryRun, syncForce) if err != nil { return fmt.Errorf("sync image %q: %w", img.Name, err) } @@ -110,7 +116,15 @@ func runSync(_ *cobra.Command, _ []string) error { return nil } +// syncImage syncs an image using the default HTTP client. +// This is a convenience wrapper for syncImageWithHTTP. func syncImage(ctx context.Context, client store.Client, img config.Image, dryRun, force bool) (bool, error) { + return syncImageWithHTTP(ctx, client, http.DefaultClient, img, dryRun, force) +} + +// syncImageWithHTTP syncs an image using the provided HTTP and store clients. +// This function enables dependency injection for testing. +func syncImageWithHTTP(ctx context.Context, client store.Client, httpClient HTTPClient, img config.Image, dryRun, force bool) (bool, error) { fmt.Printf("Processing: %s\n", img.Name) effectiveChecksum := img.EffectiveChecksum() @@ -138,7 +152,7 @@ func syncImage(ctx context.Context, client store.Client, img config.Image, dryRu // Download source image to temp file fmt.Printf(" Downloading from: %s\n", img.Source.URL) - tempFile, size, err := downloadToTemp(ctx, img.Source.URL) + tempFile, size, err := downloadToTempWithClient(ctx, httpClient, img.Source.URL) if err != nil { return false, fmt.Errorf("download: %w", err) } @@ -258,14 +272,21 @@ func syncImage(ctx context.Context, client store.Client, img config.Image, dryRu return filesChanged, nil } +// downloadToTemp downloads a URL to a temp file using the default HTTP client. func downloadToTemp(ctx context.Context, url string) (*os.File, int64, error) { + return downloadToTempWithClient(ctx, http.DefaultClient, url) +} + +// downloadToTempWithClient downloads a URL to a temp file using the provided HTTP client. +// This function enables dependency injection for testing. +func downloadToTempWithClient(ctx context.Context, client HTTPClient, url string) (*os.File, int64, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) if err != nil { return nil, 0, fmt.Errorf("create request: %w", err) } req.Header.Set("User-Agent", "labctl/1.0") - resp, err := http.DefaultClient.Do(req) + resp, err := client.Do(req) if err != nil { return nil, 0, fmt.Errorf("HTTP request: %w", err) } diff --git a/tools/labctl/cmd/images/sync_test.go b/tools/labctl/cmd/images/sync_test.go index b8478c4..d5fe0ec 100644 --- a/tools/labctl/cmd/images/sync_test.go +++ b/tools/labctl/cmd/images/sync_test.go @@ -8,6 +8,8 @@ import ( "encoding/hex" "errors" "io" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" @@ -17,6 +19,7 @@ import ( "github.com/stretchr/testify/require" "github.com/GilmanLab/lab/tools/labctl/internal/config" + "github.com/GilmanLab/lab/tools/labctl/internal/store" ) func TestVerifyChecksum(t *testing.T) { @@ -122,10 +125,6 @@ func TestDecompress(t *testing.T) { } func TestDownloadToTemp(t *testing.T) { - // This function requires a real HTTP server, so we skip detailed testing. - // The sync command integration relies on this working with real URLs. - // We just test that invalid URLs return errors. - t.Run("invalid URL returns error", func(t *testing.T) { file, size, err := downloadToTemp(context.Background(), "http://invalid.localhost.test:99999/file") @@ -135,6 +134,80 @@ func TestDownloadToTemp(t *testing.T) { }) } +func TestDownloadToTempWithClient(t *testing.T) { + t.Run("successful download", func(t *testing.T) { + content := "test file content" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(content)) + })) + defer server.Close() + + file, size, err := downloadToTempWithClient(context.Background(), server.Client(), server.URL) + + require.NoError(t, err) + defer func() { + _ = file.Close() + _ = os.Remove(file.Name()) + }() + + assert.Equal(t, int64(len(content)), size) + + // Verify content + _, err = file.Seek(0, 0) + require.NoError(t, err) + downloaded, err := io.ReadAll(file) + require.NoError(t, err) + assert.Equal(t, content, string(downloaded)) + }) + + t.Run("HTTP 404 returns error", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer server.Close() + + file, size, err := downloadToTempWithClient(context.Background(), server.Client(), server.URL) + + assert.Nil(t, file) + assert.Zero(t, size) + assert.Error(t, err) + assert.Contains(t, err.Error(), "HTTP 404") + }) + + t.Run("HTTP 500 returns error", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + file, size, err := downloadToTempWithClient(context.Background(), server.Client(), server.URL) + + assert.Nil(t, file) + assert.Zero(t, size) + assert.Error(t, err) + assert.Contains(t, err.Error(), "HTTP 500") + }) + + t.Run("sets correct user agent", func(t *testing.T) { + var receivedUA string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedUA = r.Header.Get("User-Agent") + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + file, _, err := downloadToTempWithClient(context.Background(), server.Client(), server.URL) + if file != nil { + _ = file.Close() + _ = os.Remove(file.Name()) + } + + require.NoError(t, err) + assert.Equal(t, "labctl/1.0", receivedUA) + }) +} + func TestSyncImage(t *testing.T) { t.Run("skips when checksum matches", func(t *testing.T) { client := &mockStoreClient{ @@ -229,6 +302,311 @@ func TestSyncImage(t *testing.T) { }) } +func TestSyncImageWithHTTP(t *testing.T) { + // Helper to compute SHA256 checksum + computeChecksum := func(data []byte) string { + h := sha256.Sum256(data) + return "sha256:" + hex.EncodeToString(h[:]) + } + + t.Run("full sync path: download, verify, upload, metadata", func(t *testing.T) { + // Create test content and compute checksum + content := []byte("test image content for full sync path") + checksum := computeChecksum(content) + + // Mock HTTP server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(content) + })) + defer server.Close() + + // Track S3 operations + var uploadedData []byte + var uploadedKey string + var savedMetadata *store.ImageMetadata + + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return false, nil // Checksum doesn't match, proceed with sync + }, + uploadFunc: func(_ context.Context, key string, body io.Reader, _ int64) error { + uploadedKey = key + var err error + uploadedData, err = io.ReadAll(body) + return err + }, + putMetadataFunc: func(_ context.Context, _ string, metadata *store.ImageMetadata) error { + savedMetadata = metadata + return nil + }, + } + + img := config.Image{ + Name: "test-image", + Destination: "test/test.iso", + Source: config.Source{ + URL: server.URL, + Checksum: checksum, + }, + } + + changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), img, false, false) + + require.NoError(t, err) + assert.False(t, changed) // No updateFile, so no file changes + + // Verify upload occurred with correct data + assert.Equal(t, "images/test/test.iso", uploadedKey) + assert.Equal(t, content, uploadedData) + + // Verify metadata was saved + require.NotNil(t, savedMetadata) + assert.Equal(t, "test-image", savedMetadata.Name) + assert.Equal(t, checksum, savedMetadata.Checksum) + assert.Equal(t, int64(len(content)), savedMetadata.Size) + assert.Equal(t, "http", savedMetadata.Source.Type) + assert.Equal(t, server.URL, savedMetadata.Source.URL) + }) + + t.Run("full sync path with gzip decompression", func(t *testing.T) { + // Create compressed content + decompressedContent := []byte("decompressed image content") + var compressedBuf bytes.Buffer + gzWriter := gzip.NewWriter(&compressedBuf) + _, err := gzWriter.Write(decompressedContent) + require.NoError(t, err) + require.NoError(t, gzWriter.Close()) + compressedContent := compressedBuf.Bytes() + + sourceChecksum := computeChecksum(compressedContent) + decompressedChecksum := computeChecksum(decompressedContent) + + // Mock HTTP server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(compressedContent) + })) + defer server.Close() + + // Track S3 operations + var uploadedData []byte + var savedMetadata *store.ImageMetadata + + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return false, nil + }, + uploadFunc: func(_ context.Context, _ string, body io.Reader, _ int64) error { + var err error + uploadedData, err = io.ReadAll(body) + return err + }, + putMetadataFunc: func(_ context.Context, _ string, metadata *store.ImageMetadata) error { + savedMetadata = metadata + return nil + }, + } + + img := config.Image{ + Name: "compressed-image", + Destination: "test/compressed.iso", + Source: config.Source{ + URL: server.URL, + Checksum: sourceChecksum, + Decompress: "gzip", + }, + Validation: &config.Validation{ + Expected: decompressedChecksum, + }, + } + + changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), img, false, false) + + require.NoError(t, err) + assert.False(t, changed) + + // Verify decompressed content was uploaded + assert.Equal(t, decompressedContent, uploadedData) + + // Verify metadata uses the decompressed checksum (validation.expected) + require.NotNil(t, savedMetadata) + assert.Equal(t, decompressedChecksum, savedMetadata.Checksum) + assert.Equal(t, int64(len(decompressedContent)), savedMetadata.Size) + }) + + t.Run("download error", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer server.Close() + + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return false, nil + }, + } + + img := config.Image{ + Name: "missing-image", + Destination: "test/missing.iso", + Source: config.Source{ + URL: server.URL, + Checksum: "sha256:abc123", + }, + } + + _, err := syncImageWithHTTP(context.Background(), client, server.Client(), img, false, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "download") + }) + + t.Run("checksum verification failure", func(t *testing.T) { + content := []byte("actual content") + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(content) + })) + defer server.Close() + + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return false, nil + }, + } + + img := config.Image{ + Name: "bad-checksum-image", + Destination: "test/bad.iso", + Source: config.Source{ + URL: server.URL, + Checksum: "sha256:0000000000000000000000000000000000000000000000000000000000000000", + }, + } + + _, err := syncImageWithHTTP(context.Background(), client, server.Client(), img, false, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "source checksum verification") + }) + + t.Run("upload error", func(t *testing.T) { + content := []byte("test content") + checksum := computeChecksum(content) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(content) + })) + defer server.Close() + + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return false, nil + }, + uploadFunc: func(_ context.Context, _ string, _ io.Reader, _ int64) error { + return errors.New("S3 upload failed") + }, + } + + img := config.Image{ + Name: "upload-fail-image", + Destination: "test/fail.iso", + Source: config.Source{ + URL: server.URL, + Checksum: checksum, + }, + } + + _, err := syncImageWithHTTP(context.Background(), client, server.Client(), img, false, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "upload") + }) + + t.Run("metadata write error", func(t *testing.T) { + content := []byte("test content") + checksum := computeChecksum(content) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(content) + })) + defer server.Close() + + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return false, nil + }, + uploadFunc: func(_ context.Context, _ string, _ io.Reader, _ int64) error { + return nil + }, + putMetadataFunc: func(_ context.Context, _ string, _ *store.ImageMetadata) error { + return errors.New("metadata write failed") + }, + } + + img := config.Image{ + Name: "metadata-fail-image", + Destination: "test/metadata-fail.iso", + Source: config.Source{ + URL: server.URL, + Checksum: checksum, + }, + } + + _, err := syncImageWithHTTP(context.Background(), client, server.Client(), img, false, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "write metadata") + }) + + t.Run("decompressed checksum verification failure", func(t *testing.T) { + // Create compressed content + decompressedContent := []byte("decompressed content") + var compressedBuf bytes.Buffer + gzWriter := gzip.NewWriter(&compressedBuf) + _, err := gzWriter.Write(decompressedContent) + require.NoError(t, err) + require.NoError(t, gzWriter.Close()) + compressedContent := compressedBuf.Bytes() + + sourceChecksum := computeChecksum(compressedContent) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(compressedContent) + })) + defer server.Close() + + client := &mockStoreClient{ + checksumMatchFunc: func(_ context.Context, _ string, _ string) (bool, error) { + return false, nil + }, + } + + img := config.Image{ + Name: "bad-decompress-checksum", + Destination: "test/bad-decompress.iso", + Source: config.Source{ + URL: server.URL, + Checksum: sourceChecksum, + Decompress: "gzip", + }, + Validation: &config.Validation{ + Expected: "sha256:0000000000000000000000000000000000000000000000000000000000000000", + }, + } + + _, err = syncImageWithHTTP(context.Background(), client, server.Client(), img, false, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "decompressed checksum verification") + }) +} + func TestRunSync(t *testing.T) { // Save and restore globals origDryRun := syncDryRun