diff --git a/.github/workflows/images-sync.yml b/.github/workflows/images-sync.yml index d49d4b5..e94f347 100644 --- a/.github/workflows/images-sync.yml +++ b/.github/workflows/images-sync.yml @@ -89,11 +89,20 @@ jobs: run: | pip install -r infrastructure/network/vyos/tests/requirements.txt + # Cache for downloaded ISOs and hook artifacts (rootfs.tar) + - name: Setup labctl cache + uses: actions/cache@v4 + with: + path: ~/.cache/labctl + key: labctl-images-${{ hashFiles('images/images.yaml') }} + restore-keys: | + labctl-images- + # PR: run full sync without upload (tests hooks, no credentials needed) - name: Sync Images (PR - no upload) if: github.event_name == 'pull_request' run: | - ./labctl images sync --no-upload + ./labctl images sync --no-upload --cache-dir ~/.cache/labctl # Push/dispatch: full sync with credentials - name: Install SOPS @@ -120,6 +129,7 @@ jobs: ./labctl images sync \ --credentials images/e2.sops.yaml \ --sops-age-key-file /tmp/age-key.txt \ + --cache-dir ~/.cache/labctl \ $FLAGS - name: Create PR if files changed diff --git a/infrastructure/network/vyos/scripts/iso-to-container.sh b/infrastructure/network/vyos/scripts/iso-to-container.sh index d68315d..bef25f9 100755 --- a/infrastructure/network/vyos/scripts/iso-to-container.sh +++ b/infrastructure/network/vyos/scripts/iso-to-container.sh @@ -18,6 +18,9 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" DOCKERFILE="${SCRIPT_DIR}/../Dockerfile.containerlab" +# Cache directory for rootfs.tar (set by labctl via LABCTL_HOOK_CACHE) +CACHE_DIR="${LABCTL_HOOK_CACHE:-}" + usage() { echo "Usage: $0 [image-name:tag]" echo "" @@ -66,21 +69,43 @@ trap cleanup EXIT mkdir -p "${WORK_DIR}" -echo "Extracting squashfs from ISO..." -7z x -o"${WORK_DIR}" "${ISO_PATH}" "live/filesystem.squashfs" -y >/dev/null +# Check for cached rootfs.tar +CACHED_ROOTFS="" +if [[ -n "${CACHE_DIR}" ]]; then + # Use first 12 chars of ISO sha256 as cache key + ISO_HASH=$(sha256sum "${ISO_PATH}" | cut -d' ' -f1 | head -c 12) + CACHED_ROOTFS="${CACHE_DIR}/rootfs-${ISO_HASH}.tar" -SQUASHFS="${WORK_DIR}/live/filesystem.squashfs" -if [[ ! -f "${SQUASHFS}" ]]; then - echo "ERROR: filesystem.squashfs not found in ISO" - echo "Contents of ${WORK_DIR}:" - find "${WORK_DIR}" -type f - exit 1 + if [[ -f "${CACHED_ROOTFS}" ]]; then + echo "Using cached rootfs.tar: ${CACHED_ROOTFS}" + ROOTFS_TAR="${CACHED_ROOTFS}" + fi fi -echo "Converting squashfs to rootfs.tar..." -ROOTFS_TAR="${WORK_DIR}/rootfs.tar" -sqfs2tar "${SQUASHFS}" > "${ROOTFS_TAR}" -echo "rootfs.tar size: $(ls -lh "${ROOTFS_TAR}" | awk '{print $5}')" +# Extract and convert if not cached +if [[ -z "${ROOTFS_TAR:-}" ]]; then + echo "Extracting squashfs from ISO..." + 7z x -o"${WORK_DIR}" "${ISO_PATH}" "live/filesystem.squashfs" -y >/dev/null + + SQUASHFS="${WORK_DIR}/live/filesystem.squashfs" + if [[ ! -f "${SQUASHFS}" ]]; then + echo "ERROR: filesystem.squashfs not found in ISO" + echo "Contents of ${WORK_DIR}:" + find "${WORK_DIR}" -type f + exit 1 + fi + + echo "Converting squashfs to rootfs.tar..." + ROOTFS_TAR="${WORK_DIR}/rootfs.tar" + sqfs2tar "${SQUASHFS}" > "${ROOTFS_TAR}" + echo "rootfs.tar size: $(ls -lh "${ROOTFS_TAR}" | awk '{print $5}')" + + # Cache the rootfs.tar for future use + if [[ -n "${CACHED_ROOTFS}" ]]; then + echo "Caching rootfs.tar to: ${CACHED_ROOTFS}" + cp "${ROOTFS_TAR}" "${CACHED_ROOTFS}" + fi +fi echo "Building container image: ${IMAGE_TAG}..." BUILD_CONTEXT="${WORK_DIR}/build" diff --git a/tools/labctl/cmd/images/sync.go b/tools/labctl/cmd/images/sync.go index b467864..635cffd 100644 --- a/tools/labctl/cmd/images/sync.go +++ b/tools/labctl/cmd/images/sync.go @@ -18,6 +18,7 @@ import ( "github.com/spf13/cobra" "github.com/ulikunitz/xz" + "github.com/GilmanLab/lab/tools/labctl/internal/cache" "github.com/GilmanLab/lab/tools/labctl/internal/config" "github.com/GilmanLab/lab/tools/labctl/internal/credentials" "github.com/GilmanLab/lab/tools/labctl/internal/hooks" @@ -50,6 +51,7 @@ var ( syncForce bool syncSkipHooks bool syncNoUpload bool + syncCacheDir string ) func init() { @@ -60,6 +62,7 @@ func init() { syncCmd.Flags().BoolVar(&syncForce, "force", false, "Force re-upload even if checksums match") syncCmd.Flags().BoolVar(&syncSkipHooks, "skip-hooks", false, "Skip pre-upload hooks") syncCmd.Flags().BoolVar(&syncNoUpload, "no-upload", false, "Download and run hooks but skip upload (for testing)") + syncCmd.Flags().StringVar(&syncCacheDir, "cache-dir", "", "Local cache directory for downloads and hooks") } func runSync(_ *cobra.Command, _ []string) error { @@ -74,6 +77,17 @@ func runSync(_ *cobra.Command, _ []string) error { fmt.Printf("Syncing images from manifest: %s\n", syncManifest) fmt.Printf("Found %d image(s)\n\n", len(manifest.Spec.Images)) + // Set up local cache if configured + var cacheManager *cache.Manager + if syncCacheDir != "" { + var err error + cacheManager, err = cache.NewManager(syncCacheDir) + if err != nil { + return fmt.Errorf("create cache manager: %w", err) + } + fmt.Printf("Using cache directory: %s\n\n", syncCacheDir) + } + // Skip credentials and S3 client setup in dry-run or no-upload mode var client *store.S3Client var hookExecutor *hooks.Executor @@ -95,11 +109,11 @@ func runSync(_ *cobra.Command, _ []string) error { // Create hook executor with caching (unless skipped) if !syncSkipHooks { - hookExecutor = hooks.NewExecutor(client) + hookExecutor = hooks.NewExecutor(client, syncCacheDir) } } else if syncNoUpload && !syncSkipHooks { - // In no-upload mode, create hook executor without caching - hookExecutor = hooks.NewExecutor(nil) + // In no-upload mode, create hook executor without S3 caching + hookExecutor = hooks.NewExecutor(nil, syncCacheDir) } // Track if any files were changed (for GitHub Actions output) @@ -107,7 +121,7 @@ func runSync(_ *cobra.Command, _ []string) error { // Process each image for _, img := range manifest.Spec.Images { - changed, err := syncImageWithHTTP(ctx, client, http.DefaultClient, hookExecutor, img, syncDryRun, syncForce, syncNoUpload) + changed, err := syncImageWithHTTP(ctx, client, http.DefaultClient, hookExecutor, cacheManager, img, syncDryRun, syncForce, syncNoUpload) if err != nil { return fmt.Errorf("sync image %q: %w", img.Name, err) } @@ -132,13 +146,13 @@ func runSync(_ *cobra.Command, _ []string) error { // syncImage syncs an image using the default HTTP client. // This is a convenience wrapper for syncImageWithHTTP. -func syncImage(ctx context.Context, client store.Client, hookExecutor *hooks.Executor, img config.Image, dryRun, force, noUpload bool) (bool, error) { - return syncImageWithHTTP(ctx, client, http.DefaultClient, hookExecutor, img, dryRun, force, noUpload) +func syncImage(ctx context.Context, client store.Client, hookExecutor *hooks.Executor, cacheManager *cache.Manager, img config.Image, dryRun, force, noUpload bool) (bool, error) { + return syncImageWithHTTP(ctx, client, http.DefaultClient, hookExecutor, cacheManager, img, dryRun, force, noUpload) } // 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, hookExecutor *hooks.Executor, img config.Image, dryRun, force, noUpload bool) (bool, error) { +func syncImageWithHTTP(ctx context.Context, client store.Client, httpClient HTTPClient, hookExecutor *hooks.Executor, cacheManager *cache.Manager, img config.Image, dryRun, force, noUpload bool) (bool, error) { fmt.Printf("Processing: %s\n", img.Name) effectiveChecksum := img.EffectiveChecksum() @@ -164,24 +178,89 @@ func syncImageWithHTTP(ctx context.Context, client store.Client, httpClient HTTP return false, nil } - // Download source image to temp file - fmt.Printf(" Downloading from: %s\n", img.Source.URL) - tempFile, size, err := downloadToTempWithClient(ctx, httpClient, img.Source.URL) - if err != nil { - return false, fmt.Errorf("download: %w", err) + // Download source image (with cache check) + var tempFile *os.File + var size int64 + var fromCache bool + + if cacheManager != nil { + if cachePath, ok := cacheManager.Get(img.Source.Checksum); ok { + // Found in cache - verify checksum before using + fmt.Printf(" Using cached: %s\n", cachePath) + f, err := os.Open(cachePath) //nolint:gosec // G304: Path from trusted cache manager + if err != nil { + // Cache file not accessible, fall through to download + fmt.Printf(" Cache error, will download: %v\n", err) + } else { + // Verify cached file checksum + if err := verifyChecksum(f, img.Source.Checksum); err != nil { + fmt.Printf(" Cache checksum mismatch, will download: %v\n", err) + _ = f.Close() + _ = cacheManager.Remove(img.Source.Checksum) + } else { + if _, err := f.Seek(0, 0); err != nil { + _ = f.Close() + return false, fmt.Errorf("seek cached file: %w", err) + } + stat, _ := f.Stat() + tempFile = f + size = stat.Size() + fromCache = true + } + } + } + } + + if tempFile == nil { + // Not in cache or cache disabled - download + fmt.Printf(" Downloading from: %s\n", img.Source.URL) + var err error + tempFile, size, err = downloadToTempWithClient(ctx, httpClient, img.Source.URL) + if err != nil { + return false, fmt.Errorf("download: %w", err) + } + + // Verify source checksum + fmt.Printf(" Verifying source checksum...\n") + if _, err := tempFile.Seek(0, 0); err != nil { + _ = tempFile.Close() + _ = os.Remove(tempFile.Name()) + return false, fmt.Errorf("seek temp file: %w", err) + } + if err := verifyChecksum(tempFile, img.Source.Checksum); err != nil { + _ = tempFile.Close() + _ = os.Remove(tempFile.Name()) + return false, fmt.Errorf("source checksum verification: %w", err) + } + + // Store in cache for future use + if cacheManager != nil { + if _, err := tempFile.Seek(0, 0); err != nil { + _ = tempFile.Close() + _ = os.Remove(tempFile.Name()) + return false, fmt.Errorf("seek temp file for cache: %w", err) + } + cachePath, err := cacheManager.Put(img.Source.Checksum, tempFile) + if err != nil { + // Log but don't fail - caching is optional + fmt.Printf(" Warning: failed to cache: %v\n", err) + } else { + fmt.Printf(" Cached to: %s\n", cachePath) + } + } } + + // Set up cleanup - only remove temp files, not cached files defer func() { _ = tempFile.Close() - _ = os.Remove(tempFile.Name()) + if !fromCache { + _ = os.Remove(tempFile.Name()) + } }() - // Verify source checksum - fmt.Printf(" Verifying source checksum...\n") + // Reset file position after checksum verification 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) + return false, fmt.Errorf("seek file: %w", err) } // Decompress if needed diff --git a/tools/labctl/cmd/images/sync_test.go b/tools/labctl/cmd/images/sync_test.go index 87bba0d..809d6d0 100644 --- a/tools/labctl/cmd/images/sync_test.go +++ b/tools/labctl/cmd/images/sync_test.go @@ -225,7 +225,7 @@ func TestSyncImage(t *testing.T) { }, } - changed, err := syncImage(context.Background(), client, nil, img, false, false, false) + changed, err := syncImage(context.Background(), client, nil, nil, img, false, false, false) require.NoError(t, err) assert.False(t, changed) @@ -244,7 +244,7 @@ func TestSyncImage(t *testing.T) { }, } - changed, err := syncImage(context.Background(), client, nil, img, true, false, false) + changed, err := syncImage(context.Background(), client, nil, nil, img, true, false, false) require.NoError(t, err) assert.False(t, changed) @@ -273,7 +273,7 @@ func TestSyncImage(t *testing.T) { // With force=true and dryRun=true, it should show what would be done // without checking checksum - _, err := syncImage(context.Background(), client, nil, img, true, true, false) + _, err := syncImage(context.Background(), client, nil, nil, img, true, true, false) require.NoError(t, err) assert.False(t, checksumChecked) // Should not check checksum with force @@ -295,7 +295,7 @@ func TestSyncImage(t *testing.T) { }, } - _, err := syncImage(context.Background(), client, nil, img, false, false, false) + _, err := syncImage(context.Background(), client, nil, nil, img, false, false, false) assert.Error(t, err) assert.Contains(t, err.Error(), "check existing image") @@ -321,7 +321,7 @@ func TestSyncImage(t *testing.T) { // With noUpload=true, should skip checksum check (client is nil for noUpload) // and also skip upload - this test verifies the skip behavior - changed, err := syncImage(context.Background(), client, nil, img, true, false, false) + changed, err := syncImage(context.Background(), client, nil, nil, img, true, false, false) require.NoError(t, err) assert.False(t, changed) @@ -379,7 +379,7 @@ func TestSyncImageWithHTTP(t *testing.T) { }, } - changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, img, false, false, false) + changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, nil, img, false, false, false) require.NoError(t, err) assert.False(t, changed) // No updateFile, so no file changes @@ -449,7 +449,7 @@ func TestSyncImageWithHTTP(t *testing.T) { }, } - changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, img, false, false, false) + changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, nil, img, false, false, false) require.NoError(t, err) assert.False(t, changed) @@ -484,7 +484,7 @@ func TestSyncImageWithHTTP(t *testing.T) { }, } - _, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, img, false, false, false) + _, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, nil, img, false, false, false) assert.Error(t, err) assert.Contains(t, err.Error(), "download") @@ -514,7 +514,7 @@ func TestSyncImageWithHTTP(t *testing.T) { }, } - _, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, img, false, false, false) + _, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, nil, img, false, false, false) assert.Error(t, err) assert.Contains(t, err.Error(), "source checksum verification") @@ -548,7 +548,7 @@ func TestSyncImageWithHTTP(t *testing.T) { }, } - _, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, img, false, false, false) + _, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, nil, img, false, false, false) assert.Error(t, err) assert.Contains(t, err.Error(), "upload") @@ -585,7 +585,7 @@ func TestSyncImageWithHTTP(t *testing.T) { }, } - _, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, img, false, false, false) + _, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, nil, img, false, false, false) assert.Error(t, err) assert.Contains(t, err.Error(), "write metadata") @@ -628,7 +628,7 @@ func TestSyncImageWithHTTP(t *testing.T) { }, } - _, err = syncImageWithHTTP(context.Background(), client, server.Client(), nil, img, false, false, false) + _, err = syncImageWithHTTP(context.Background(), client, server.Client(), nil, nil, img, false, false, false) assert.Error(t, err) assert.Contains(t, err.Error(), "decompressed checksum verification") @@ -676,7 +676,7 @@ func TestSyncImageWithHTTP(t *testing.T) { } // noUpload=true should download, verify, but skip upload - changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, img, false, false, true) + changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), nil, nil, img, false, false, true) require.NoError(t, err) assert.False(t, changed) diff --git a/tools/labctl/internal/cache/cache.go b/tools/labctl/internal/cache/cache.go new file mode 100644 index 0000000..e919145 --- /dev/null +++ b/tools/labctl/internal/cache/cache.go @@ -0,0 +1,135 @@ +// Package cache provides local file caching for the image pipeline. +package cache + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "os" + "path/filepath" + "strings" +) + +// Manager handles local file caching by checksum. +// Files are stored in a directory structure under the base directory. +type Manager struct { + baseDir string +} + +// NewManager creates a new cache manager with the given base directory. +// Returns an error if the directory cannot be created. +func NewManager(baseDir string) (*Manager, error) { + if baseDir == "" { + return nil, fmt.Errorf("cache directory cannot be empty") + } + + // Create base directory and subdirectories + dirs := []string{ + filepath.Join(baseDir, "downloads"), + filepath.Join(baseDir, "hooks"), + } + for _, dir := range dirs { + if err := os.MkdirAll(dir, 0o750); err != nil { + return nil, fmt.Errorf("create cache directory %s: %w", dir, err) + } + } + + return &Manager{baseDir: baseDir}, nil +} + +// BaseDir returns the base cache directory. +func (m *Manager) BaseDir() string { + return m.baseDir +} + +// Get returns the path to a cached file for the given checksum. +// Returns the path and true if the file exists, empty string and false otherwise. +func (m *Manager) Get(checksum string) (string, bool) { + key := checksumKey(checksum) + cachePath := filepath.Join(m.baseDir, "downloads", key) + + if _, err := os.Stat(cachePath); err != nil { + return "", false + } + + return cachePath, true +} + +// Put stores content from the reader to the cache under the given checksum. +// Returns the path to the cached file. +func (m *Manager) Put(checksum string, src io.Reader) (string, error) { + key := checksumKey(checksum) + cachePath := filepath.Join(m.baseDir, "downloads", key) + + // Write to temp file first, then rename for atomicity + tempPath := cachePath + ".tmp" + f, err := os.Create(tempPath) //nolint:gosec // G304: Path is constructed from trusted cache directory + if err != nil { + return "", fmt.Errorf("create cache file: %w", err) + } + + _, err = io.Copy(f, src) + if closeErr := f.Close(); closeErr != nil && err == nil { + err = closeErr + } + if err != nil { + _ = os.Remove(tempPath) + return "", fmt.Errorf("write cache file: %w", err) + } + + if err := os.Rename(tempPath, cachePath); err != nil { + _ = os.Remove(tempPath) + return "", fmt.Errorf("rename cache file: %w", err) + } + + return cachePath, nil +} + +// Remove deletes a cached file for the given checksum. +func (m *Manager) Remove(checksum string) error { + key := checksumKey(checksum) + cachePath := filepath.Join(m.baseDir, "downloads", key) + return os.Remove(cachePath) +} + +// HookDir returns the cache directory for a specific hook. +// Creates the directory if it doesn't exist. +func (m *Manager) HookDir(hookName string) (string, error) { + // Sanitize hook name for filesystem safety + safeName := sanitizeName(hookName) + hookDir := filepath.Join(m.baseDir, "hooks", safeName) + + if err := os.MkdirAll(hookDir, 0o750); err != nil { + return "", fmt.Errorf("create hook cache directory: %w", err) + } + + return hookDir, nil +} + +// checksumKey converts a checksum string to a safe cache key. +// Uses the first 12 characters of sha256 hash for compact, unique names. +func checksumKey(checksum string) string { + // Remove algorithm prefix if present (e.g., "sha256:abc123...") + if idx := strings.Index(checksum, ":"); idx != -1 { + checksum = checksum[idx+1:] + } + + // Hash the checksum to get a consistent, safe filename + h := sha256.Sum256([]byte(checksum)) + return hex.EncodeToString(h[:])[:12] +} + +// sanitizeName makes a string safe for use as a filename. +func sanitizeName(name string) string { + // Replace any non-alphanumeric characters with underscore + var result strings.Builder + for _, r := range name { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '-' || r == '_' { + result.WriteRune(r) + } else { + result.WriteRune('_') + } + } + return result.String() +} diff --git a/tools/labctl/internal/cache/cache_test.go b/tools/labctl/internal/cache/cache_test.go new file mode 100644 index 0000000..7673b7a --- /dev/null +++ b/tools/labctl/internal/cache/cache_test.go @@ -0,0 +1,164 @@ +package cache + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestNewManager(t *testing.T) { + t.Run("creates directories", func(t *testing.T) { + dir := t.TempDir() + baseDir := filepath.Join(dir, "cache") + + m, err := NewManager(baseDir) + if err != nil { + t.Fatalf("NewManager() error = %v", err) + } + + // Verify directories were created + for _, subdir := range []string{"downloads", "hooks"} { + path := filepath.Join(baseDir, subdir) + if _, err := os.Stat(path); os.IsNotExist(err) { + t.Errorf("directory %s was not created", subdir) + } + } + + if m.BaseDir() != baseDir { + t.Errorf("BaseDir() = %q, want %q", m.BaseDir(), baseDir) + } + }) + + t.Run("returns error for empty path", func(t *testing.T) { + _, err := NewManager("") + if err == nil { + t.Error("NewManager(\"\") should return error") + } + }) +} + +func TestManager_GetPut(t *testing.T) { + dir := t.TempDir() + m, err := NewManager(filepath.Join(dir, "cache")) + if err != nil { + t.Fatalf("NewManager() error = %v", err) + } + + checksum := "sha256:abc123def456" + content := "test content" + + t.Run("Get returns false for missing file", func(t *testing.T) { + _, ok := m.Get(checksum) + if ok { + t.Error("Get() should return false for missing file") + } + }) + + t.Run("Put stores and Get retrieves", func(t *testing.T) { + path, err := m.Put(checksum, strings.NewReader(content)) + if err != nil { + t.Fatalf("Put() error = %v", err) + } + + // Verify file was created + data, err := os.ReadFile(path) //nolint:gosec // G304: Test file path + if err != nil { + t.Fatalf("ReadFile(%s) error = %v", path, err) + } + if string(data) != content { + t.Errorf("file content = %q, want %q", string(data), content) + } + + // Verify Get returns the path + gotPath, ok := m.Get(checksum) + if !ok { + t.Error("Get() should return true after Put()") + } + if gotPath != path { + t.Errorf("Get() path = %q, want %q", gotPath, path) + } + }) + + t.Run("Remove deletes cached file", func(t *testing.T) { + err := m.Remove(checksum) + if err != nil { + t.Fatalf("Remove() error = %v", err) + } + + _, ok := m.Get(checksum) + if ok { + t.Error("Get() should return false after Remove()") + } + }) +} + +func TestManager_HookDir(t *testing.T) { + dir := t.TempDir() + m, err := NewManager(filepath.Join(dir, "cache")) + if err != nil { + t.Fatalf("NewManager() error = %v", err) + } + + t.Run("creates hook directory", func(t *testing.T) { + hookDir, err := m.HookDir("vyos-integration-test") + if err != nil { + t.Fatalf("HookDir() error = %v", err) + } + + if _, err := os.Stat(hookDir); os.IsNotExist(err) { + t.Error("HookDir() did not create directory") + } + + // Verify it's under the hooks subdirectory + if !strings.Contains(hookDir, "hooks") { + t.Errorf("HookDir() = %q, expected to contain 'hooks'", hookDir) + } + }) + + t.Run("sanitizes hook name", func(t *testing.T) { + hookDir, err := m.HookDir("hook/with:special*chars") + if err != nil { + t.Fatalf("HookDir() error = %v", err) + } + + // Path should not contain special characters + base := filepath.Base(hookDir) + for _, char := range []string{"/", ":", "*"} { + if strings.Contains(base, char) { + t.Errorf("HookDir base %q contains special char %q", base, char) + } + } + }) +} + +func TestChecksumKey(t *testing.T) { + tests := []struct { + name string + checksum string + }{ + {"with prefix", "sha256:abc123def456"}, + {"without prefix", "abc123def456"}, + {"sha512 prefix", "sha512:abc123def456"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key := checksumKey(tt.checksum) + + // Key should be exactly 12 hex characters + if len(key) != 12 { + t.Errorf("checksumKey() length = %d, want 12", len(key)) + } + + // Key should only contain hex characters + for _, c := range key { + isDigit := c >= '0' && c <= '9' + isHexLetter := c >= 'a' && c <= 'f' + if !isDigit && !isHexLetter { + t.Errorf("checksumKey() contains non-hex char: %c", c) + } + } + }) + } +} diff --git a/tools/labctl/internal/hooks/executor.go b/tools/labctl/internal/hooks/executor.go index a0cb234..0e4e677 100644 --- a/tools/labctl/internal/hooks/executor.go +++ b/tools/labctl/internal/hooks/executor.go @@ -9,6 +9,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "sync" "time" @@ -25,13 +26,15 @@ const ( // Executor runs hooks and manages result caching. type Executor struct { - client store.Client + client store.Client + cacheDir string } // NewExecutor creates a new hook executor. -// If client is nil, caching is disabled. -func NewExecutor(client store.Client) *Executor { - return &Executor{client: client} +// If client is nil, S3 result caching is disabled. +// If cacheDir is non-empty, it will be passed to hooks as LABCTL_HOOK_CACHE. +func NewExecutor(client store.Client, cacheDir string) *Executor { + return &Executor{client: client, cacheDir: cacheDir} } // RunPreUploadHooks executes all pre-upload hooks for an image. @@ -83,6 +86,18 @@ func (e *Executor) runHook(ctx context.Context, destination string, hook config. cmd.Dir = hook.WorkDir } + // Set up hook cache directory if configured + if e.cacheDir != "" { + // Sanitize hook name for filesystem safety + safeName := sanitizeHookName(hook.Name) + hookCacheDir := filepath.Join(e.cacheDir, "hooks", safeName) + if err := os.MkdirAll(hookCacheDir, 0o750); err != nil { + fmt.Printf(" Warning: failed to create hook cache dir: %v\n", err) + } else { + cmd.Env = append(os.Environ(), "LABCTL_HOOK_CACHE="+hookCacheDir) + } + } + // Set up output streaming with prefix var outputBuf bytes.Buffer stdout, err := cmd.StdoutPipe() @@ -153,3 +168,17 @@ func streamWithPrefix(wg *sync.WaitGroup, r io.Reader, buf *bytes.Buffer, prefix _, _ = buf.WriteString(line + "\n") } } + +// sanitizeHookName makes a hook name safe for use as a directory name. +func sanitizeHookName(name string) string { + var result []byte + for i := 0; i < len(name); i++ { + c := name[i] + if (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '_' { + result = append(result, c) + } else { + result = append(result, '_') + } + } + return string(result) +}