From 6a58377115d91ae7e9e5c402d388dac981c999a0 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Sun, 28 Dec 2025 20:13:37 -0800 Subject: [PATCH] feat(labctl): add transformation hook support for image sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new `transform` hook type that allows modifying files before upload. Transform hooks receive a copy of the file and modify it in-place, with the transformed output becoming the upload source. Key changes: - Add Transform field to Hooks struct in manifest schema - Add RunTransformHooks() method to hook executor - Integrate transform hooks into sync pipeline (runs after decompression, before preUpload hooks) - Checksum verification uses input file (for caching), upload uses output Closes HOM-26 🤖 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 | 66 +++++ tools/labctl/internal/config/manifest.go | 10 + tools/labctl/internal/config/manifest_test.go | 80 ++++++ tools/labctl/internal/hooks/executor.go | 143 ++++++++++ tools/labctl/internal/hooks/executor_test.go | 248 ++++++++++++++++++ 6 files changed, 574 insertions(+) create mode 100644 tools/labctl/internal/hooks/executor_test.go diff --git a/tools/labctl/cmd/images/sync.go b/tools/labctl/cmd/images/sync.go index 635cffd..82baa92 100644 --- a/tools/labctl/cmd/images/sync.go +++ b/tools/labctl/cmd/images/sync.go @@ -298,6 +298,33 @@ func syncImageWithHTTP(ctx context.Context, client store.Client, httpClient HTTP uploadSize = size } + // Run transform hooks (before pre-upload hooks) + if hookExecutor != nil && img.Hooks != nil && len(img.Hooks.Transform) > 0 { + fmt.Printf(" Running transform hooks...\n") + result, err := hookExecutor.RunTransformHooks(ctx, img, uploadFile.Name()) + if err != nil { + return false, fmt.Errorf("transform hooks: %w", err) + } + defer result.Cleanup() + + if result.OutputPath != "" { + // Use transformed file for upload + transformedFile, err := os.Open(result.OutputPath) //nolint:gosec // G304: Path from trusted hook result + if err != nil { + return false, fmt.Errorf("open transformed file: %w", err) + } + defer func() { _ = transformedFile.Close() }() + + stat, err := transformedFile.Stat() + if err != nil { + return false, fmt.Errorf("stat transformed file: %w", err) + } + + uploadFile = transformedFile + uploadSize = stat.Size() + } + } + // Run pre-upload hooks if hookExecutor != nil && img.Hooks != nil && len(img.Hooks.PreUpload) > 0 { fmt.Printf(" Running pre-upload hooks...\n") diff --git a/tools/labctl/cmd/images/sync_test.go b/tools/labctl/cmd/images/sync_test.go index 809d6d0..6b44405 100644 --- a/tools/labctl/cmd/images/sync_test.go +++ b/tools/labctl/cmd/images/sync_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/require" "github.com/GilmanLab/lab/tools/labctl/internal/config" + "github.com/GilmanLab/lab/tools/labctl/internal/hooks" "github.com/GilmanLab/lab/tools/labctl/internal/store" ) @@ -683,6 +684,71 @@ func TestSyncImageWithHTTP(t *testing.T) { assert.False(t, uploadCalled, "Upload should not be called in no-upload mode") assert.False(t, metadataCalled, "PutMetadata should not be called in no-upload mode") }) + + t.Run("transform hook modifies uploaded content", func(t *testing.T) { + // Create test content + originalContent := []byte("original content") + checksum := computeChecksum(originalContent) + + // Mock HTTP server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(originalContent) + })) + defer server.Close() + + // Track uploaded content + var uploadedData []byte + + 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, _ *store.ImageMetadata) error { + return nil + }, + } + + // Create a transform script in a temp dir + dir := t.TempDir() + scriptFile := filepath.Join(dir, "transform.sh") + scriptContent := "#!/bin/sh\necho ' TRANSFORMED' >> \"$1\"\n" + err := os.WriteFile(scriptFile, []byte(scriptContent), 0o755) //nolint:gosec // G306: Script needs execute permission + require.NoError(t, err) + + // Create a hook executor for testing + hookExecutor := hooks.NewExecutor(nil, "") + + img := config.Image{ + Name: "test-image", + Destination: "test/test.iso", + Source: config.Source{ + URL: server.URL, + Checksum: checksum, + }, + Hooks: &config.Hooks{ + Transform: []config.Hook{ + { + Name: "append-transform", + Command: scriptFile, + }, + }, + }, + } + + changed, err := syncImageWithHTTP(context.Background(), client, server.Client(), hookExecutor, nil, img, false, false, false) + + require.NoError(t, err) + assert.False(t, changed) + + // Verify that the uploaded content was transformed + assert.Equal(t, "original content TRANSFORMED\n", string(uploadedData)) + }) } func TestRunSync(t *testing.T) { diff --git a/tools/labctl/internal/config/manifest.go b/tools/labctl/internal/config/manifest.go index efe3f76..b5ab467 100644 --- a/tools/labctl/internal/config/manifest.go +++ b/tools/labctl/internal/config/manifest.go @@ -47,6 +47,11 @@ type Hooks struct { // PreUpload runs after download/verification, before upload. // Hook must exit 0 for upload to proceed. PreUpload []Hook `yaml:"preUpload,omitempty"` + // Transform runs after download/verification, before upload. + // The hook receives a copy of the file and can modify it in-place. + // The modified file becomes the upload source. + // Transform hooks run before preUpload hooks. + Transform []Hook `yaml:"transform,omitempty"` } // Hook defines a hook to run during image processing. @@ -274,6 +279,11 @@ func (i *Image) ValidateAll() []error { errs = append(errs, fmt.Errorf("hooks.preUpload[%d]: %w", j, err)) } } + for j, h := range i.Hooks.Transform { + for _, err := range h.ValidateAll() { + errs = append(errs, fmt.Errorf("hooks.transform[%d]: %w", j, err)) + } + } } return errs diff --git a/tools/labctl/internal/config/manifest_test.go b/tools/labctl/internal/config/manifest_test.go index 3db7fae..1edfe5d 100644 --- a/tools/labctl/internal/config/manifest_test.go +++ b/tools/labctl/internal/config/manifest_test.go @@ -272,6 +272,86 @@ spec: `, wantErr: "updateFile.path is required", }, + { + name: "valid manifest with transform hooks", + yaml: `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: lab-images +spec: + images: + - name: talos-iso + source: + url: https://factory.talos.dev/image/talos-amd64.iso + checksum: sha256:abc123 + destination: talos/talos-amd64.iso + hooks: + transform: + - name: embed-config + command: ./scripts/embed-config.sh + args: ["--config", "machine.yaml"] + timeout: 10m +`, + }, + { + name: "invalid transform hook missing name", + yaml: `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: lab-images +spec: + images: + - name: test-image + source: + url: https://example.com/image.iso + checksum: sha256:abc123 + destination: images/image.iso + hooks: + transform: + - command: ./script.sh +`, + wantErr: "hooks.transform[0]: name is required", + }, + { + name: "invalid transform hook missing command", + yaml: `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: lab-images +spec: + images: + - name: test-image + source: + url: https://example.com/image.iso + checksum: sha256:abc123 + destination: images/image.iso + hooks: + transform: + - name: my-hook +`, + wantErr: "hooks.transform[0]: command is required", + }, + { + name: "invalid transform hook bad timeout", + yaml: `apiVersion: images.lab.gilman.io/v1alpha1 +kind: ImageManifest +metadata: + name: lab-images +spec: + images: + - name: test-image + source: + url: https://example.com/image.iso + checksum: sha256:abc123 + destination: images/image.iso + hooks: + transform: + - name: my-hook + command: ./script.sh + timeout: invalid +`, + wantErr: "hooks.transform[0]: invalid timeout", + }, } for _, tt := range tests { diff --git a/tools/labctl/internal/hooks/executor.go b/tools/labctl/internal/hooks/executor.go index 0e4e677..97b5b76 100644 --- a/tools/labctl/internal/hooks/executor.go +++ b/tools/labctl/internal/hooks/executor.go @@ -37,6 +37,149 @@ func NewExecutor(client store.Client, cacheDir string) *Executor { return &Executor{client: client, cacheDir: cacheDir} } +// TransformResult contains the result of running transform hooks. +type TransformResult struct { + // OutputPath is the path to the transformed file. + // If no transform hooks were run, this is empty. + OutputPath string + // Cleanup should be called to remove any temporary files created. + // Safe to call even if OutputPath is empty. + Cleanup func() +} + +// RunTransformHooks executes all transform hooks for an image. +// Each hook receives a copy of the file and can modify it in-place. +// The hooks are chained: each hook receives the output of the previous hook. +// Returns the path to the final transformed file, or empty string if no hooks. +// The caller must call Cleanup() when done with the transformed file. +func (e *Executor) RunTransformHooks(ctx context.Context, img config.Image, imagePath string) (*TransformResult, error) { + if img.Hooks == nil || len(img.Hooks.Transform) == 0 { + return &TransformResult{Cleanup: func() {}}, nil + } + + // Create a working copy of the file for transformations + workFile, err := copyToTemp(imagePath) + if err != nil { + return nil, fmt.Errorf("create working copy: %w", err) + } + + cleanup := func() { + _ = os.Remove(workFile) + } + + // Run each transform hook in sequence + for _, hook := range img.Hooks.Transform { + if err := e.runTransformHook(ctx, hook, workFile); err != nil { + cleanup() + return nil, fmt.Errorf("transform hook %q failed: %w", hook.Name, err) + } + } + + return &TransformResult{ + OutputPath: workFile, + Cleanup: cleanup, + }, nil +} + +// runTransformHook executes a single transform hook. +// The hook modifies the file at workPath in-place. +func (e *Executor) runTransformHook(ctx context.Context, hook config.Hook, workPath string) error { + // Parse timeout + timeout := DefaultTimeout + if hook.Timeout != "" { + var err error + timeout, err = time.ParseDuration(hook.Timeout) + if err != nil { + return fmt.Errorf("invalid timeout %q: %w", hook.Timeout, err) + } + } + + // Execute hook + fmt.Printf(" Running transform hook %q...\n", hook.Name) + hookCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + args := append([]string{workPath}, hook.Args...) + cmd := exec.CommandContext(hookCtx, hook.Command, args...) //nolint:gosec // G204: Command is from trusted manifest + if hook.WorkDir != "" { + cmd.Dir = hook.WorkDir + } + + // Set up hook cache directory if configured + if e.cacheDir != "" { + 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() + if err != nil { + return fmt.Errorf("create stdout pipe: %w", err) + } + stderr, err := cmd.StderrPipe() + if err != nil { + return fmt.Errorf("create stderr pipe: %w", err) + } + + start := time.Now() + if err := cmd.Start(); err != nil { + return fmt.Errorf("start hook: %w", err) + } + + // Stream output with prefix + var wg sync.WaitGroup + wg.Add(2) + go streamWithPrefix(&wg, stdout, &outputBuf, " │ ") + go streamWithPrefix(&wg, stderr, &outputBuf, " │ ") + wg.Wait() + + err = cmd.Wait() + duration := time.Since(start) + output := outputBuf.Bytes() + + if err != nil { + return fmt.Errorf("exit status %v:\n%s", err, truncateOutput(string(output), 1024)) + } + + fmt.Printf(" Transform hook %q: completed (%s)\n", hook.Name, duration.Round(time.Second)) + return nil +} + +// copyToTemp creates a temporary copy of the file at srcPath. +// Returns the path to the temporary file. +func copyToTemp(srcPath string) (string, error) { + src, err := os.Open(srcPath) //nolint:gosec // G304: Path is from trusted internal source + if err != nil { + return "", fmt.Errorf("open source: %w", err) + } + defer func() { _ = src.Close() }() + + dst, err := os.CreateTemp("", "labctl-transform-*") + if err != nil { + return "", fmt.Errorf("create temp file: %w", err) + } + dstPath := dst.Name() + + if _, err := io.Copy(dst, src); err != nil { + _ = dst.Close() + _ = os.Remove(dstPath) + return "", fmt.Errorf("copy file: %w", err) + } + + if err := dst.Close(); err != nil { + _ = os.Remove(dstPath) + return "", fmt.Errorf("close temp file: %w", err) + } + + return dstPath, nil +} + // RunPreUploadHooks executes all pre-upload hooks for an image. // Returns nil if all hooks pass, error if any hook fails. func (e *Executor) RunPreUploadHooks(ctx context.Context, img config.Image, imagePath, checksum string) error { diff --git a/tools/labctl/internal/hooks/executor_test.go b/tools/labctl/internal/hooks/executor_test.go new file mode 100644 index 0000000..26cde85 --- /dev/null +++ b/tools/labctl/internal/hooks/executor_test.go @@ -0,0 +1,248 @@ +package hooks + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/GilmanLab/lab/tools/labctl/internal/config" +) + +func TestRunTransformHooks(t *testing.T) { + t.Run("no hooks returns empty result", func(t *testing.T) { + executor := NewExecutor(nil, "") + img := config.Image{ + Name: "test-image", + } + + result, err := executor.RunTransformHooks(context.Background(), img, "/nonexistent") + + require.NoError(t, err) + assert.Empty(t, result.OutputPath) + assert.NotNil(t, result.Cleanup) + result.Cleanup() // Should not panic + }) + + t.Run("empty hooks returns empty result", func(t *testing.T) { + executor := NewExecutor(nil, "") + img := config.Image{ + Name: "test-image", + Hooks: &config.Hooks{}, + } + + result, err := executor.RunTransformHooks(context.Background(), img, "/nonexistent") + + require.NoError(t, err) + assert.Empty(t, result.OutputPath) + }) + + t.Run("transform hook modifies file in-place", func(t *testing.T) { + // Create a temp file with initial content + dir := t.TempDir() + inputFile := filepath.Join(dir, "input.txt") + err := os.WriteFile(inputFile, []byte("original content"), 0o600) + require.NoError(t, err) + + // Create a script that modifies the file passed as first argument + scriptFile := filepath.Join(dir, "transform.sh") + scriptContent := `#!/bin/sh +echo " transformed" >> "$1" +` + err = os.WriteFile(scriptFile, []byte(scriptContent), 0o755) //nolint:gosec // G306: Script needs execute permission + require.NoError(t, err) + + executor := NewExecutor(nil, "") + img := config.Image{ + Name: "test-image", + Hooks: &config.Hooks{ + Transform: []config.Hook{ + { + Name: "append-hook", + Command: scriptFile, + }, + }, + }, + } + + result, err := executor.RunTransformHooks(context.Background(), img, inputFile) + require.NoError(t, err) + require.NotEmpty(t, result.OutputPath) + defer result.Cleanup() + + // Verify the transformed file has the expected content + content, err := os.ReadFile(result.OutputPath) //nolint:gosec + require.NoError(t, err) + assert.Equal(t, "original content transformed\n", string(content)) + + // Original file should be unchanged + originalContent, err := os.ReadFile(inputFile) //nolint:gosec // G304: Test file path + require.NoError(t, err) + assert.Equal(t, "original content", string(originalContent)) + }) + + t.Run("multiple transform hooks chain correctly", func(t *testing.T) { + dir := t.TempDir() + inputFile := filepath.Join(dir, "input.txt") + err := os.WriteFile(inputFile, []byte("start"), 0o600) + require.NoError(t, err) + + // Create scripts for each hook + script1 := filepath.Join(dir, "first.sh") + err = os.WriteFile(script1, []byte("#!/bin/sh\necho '-first' >> \"$1\"\n"), 0o755) //nolint:gosec // G306: Script needs execute permission + require.NoError(t, err) + + script2 := filepath.Join(dir, "second.sh") + err = os.WriteFile(script2, []byte("#!/bin/sh\necho '-second' >> \"$1\"\n"), 0o755) //nolint:gosec // G306: Script needs execute permission + require.NoError(t, err) + + executor := NewExecutor(nil, "") + img := config.Image{ + Name: "test-image", + Hooks: &config.Hooks{ + Transform: []config.Hook{ + { + Name: "first-hook", + Command: script1, + }, + { + Name: "second-hook", + Command: script2, + }, + }, + }, + } + + result, err := executor.RunTransformHooks(context.Background(), img, inputFile) + require.NoError(t, err) + require.NotEmpty(t, result.OutputPath) + defer result.Cleanup() + + content, err := os.ReadFile(result.OutputPath) //nolint:gosec + require.NoError(t, err) + assert.Equal(t, "start-first\n-second\n", string(content)) + }) + + t.Run("hook failure returns error and cleans up", func(t *testing.T) { + dir := t.TempDir() + inputFile := filepath.Join(dir, "input.txt") + err := os.WriteFile(inputFile, []byte("content"), 0o600) + require.NoError(t, err) + + // Create a script that fails + failScript := filepath.Join(dir, "fail.sh") + err = os.WriteFile(failScript, []byte("#!/bin/sh\nexit 1\n"), 0o755) //nolint:gosec // G306: Script needs execute permission + require.NoError(t, err) + + executor := NewExecutor(nil, "") + img := config.Image{ + Name: "test-image", + Hooks: &config.Hooks{ + Transform: []config.Hook{ + { + Name: "failing-hook", + Command: failScript, + }, + }, + }, + } + + result, err := executor.RunTransformHooks(context.Background(), img, inputFile) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "failing-hook") + assert.Nil(t, result) + }) + + t.Run("hook receives LABCTL_HOOK_CACHE when cacheDir configured", func(t *testing.T) { + dir := t.TempDir() + cacheDir := filepath.Join(dir, "cache") + inputFile := filepath.Join(dir, "input.txt") + outputFile := filepath.Join(dir, "env.txt") + err := os.WriteFile(inputFile, []byte("content"), 0o600) + require.NoError(t, err) + + // Create a script that writes the cache env var to a file + checkScript := filepath.Join(dir, "check-env.sh") + scriptContent := fmt.Sprintf("#!/bin/sh\necho $LABCTL_HOOK_CACHE > %s\n", outputFile) + err = os.WriteFile(checkScript, []byte(scriptContent), 0o755) //nolint:gosec // G306: Script needs execute permission + require.NoError(t, err) + + executor := NewExecutor(nil, cacheDir) + img := config.Image{ + Name: "test-image", + Hooks: &config.Hooks{ + Transform: []config.Hook{ + { + Name: "env-check", + Command: checkScript, + }, + }, + }, + } + + result, err := executor.RunTransformHooks(context.Background(), img, inputFile) + require.NoError(t, err) + defer result.Cleanup() + + envContent, err := os.ReadFile(outputFile) //nolint:gosec // G304: Test file path + require.NoError(t, err) + assert.Contains(t, string(envContent), filepath.Join(cacheDir, "hooks", "env-check")) + }) +} + +func TestCopyToTemp(t *testing.T) { + t.Run("creates copy of file", func(t *testing.T) { + dir := t.TempDir() + srcFile := filepath.Join(dir, "source.txt") + content := "test content for copy" + err := os.WriteFile(srcFile, []byte(content), 0o600) + require.NoError(t, err) + + dstPath, err := copyToTemp(srcFile) + require.NoError(t, err) + defer func() { _ = os.Remove(dstPath) }() + + // Verify content matches + dstContent, err := os.ReadFile(dstPath) //nolint:gosec + require.NoError(t, err) + assert.Equal(t, content, string(dstContent)) + + // Verify it's a different file + assert.NotEqual(t, srcFile, dstPath) + }) + + t.Run("returns error for nonexistent file", func(t *testing.T) { + _, err := copyToTemp("/nonexistent/file.txt") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "open source") + }) +} + +func TestSanitizeHookName(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"simple", "simple"}, + {"with-dash", "with-dash"}, + {"with_underscore", "with_underscore"}, + {"with spaces", "with_spaces"}, + {"with/slashes", "with_slashes"}, + {"with:colons", "with_colons"}, + {"MixedCase123", "MixedCase123"}, + {"special!@#$chars", "special____chars"}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + result := sanitizeHookName(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +}