From 0e598c005d9a38d231c57bc812ee19f0374600aa Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Mon, 13 Apr 2026 11:26:31 -0700 Subject: [PATCH 01/10] Add Config property bag to HookConfig and ExecutionContext (#7653) Add a generic Config map[string]any field to HookConfig, following the established property bag pattern used by ServiceConfig, RemoteConfig, and platform.Config. The Config field is threaded through ExecutionContext so hook executors can access executor-specific settings. Changes: - Add Config field to HookConfig (yaml: config,omitempty) - Add Config field to ExecutionContext - Thread Config from HookConfig to ExecutionContext in execHook() - Include Config in hook configuration signature calculation - Add comprehensive tests for YAML parsing, roundtrip, omitempty, signature inclusion, and executor threading Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/ext/hooks_config_test.go | 102 ++++++++++++++ cli/azd/pkg/ext/hooks_runner.go | 1 + cli/azd/pkg/ext/hooks_runner_test.go | 191 +++++++++++++++++++++++++++ cli/azd/pkg/ext/models.go | 14 ++ cli/azd/pkg/ext/models_test.go | 157 +++++++++++++++++++++- cli/azd/pkg/tools/script.go | 5 + 6 files changed, 467 insertions(+), 3 deletions(-) diff --git a/cli/azd/pkg/ext/hooks_config_test.go b/cli/azd/pkg/ext/hooks_config_test.go index 69829b0d075..bf473e84e58 100644 --- a/cli/azd/pkg/ext/hooks_config_test.go +++ b/cli/azd/pkg/ext/hooks_config_test.go @@ -246,6 +246,72 @@ predeploy: "script4.sh", hooks["predeploy"][0].Run, ) }) + + t.Run("hook with config block", func(t *testing.T) { + const doc = ` +postprovision: + run: ./hooks/seed-database.cs + kind: dotnet + config: + configuration: Release + framework: net10.0 +` + + var hooks HooksConfig + err := yaml.Unmarshal([]byte(doc), &hooks) + require.NoError(t, err) + + require.Len(t, hooks["postprovision"], 1) + hook := hooks["postprovision"][0] + assert.Equal(t, "./hooks/seed-database.cs", hook.Run) + assert.Equal(t, + language.HookKindDotNet, hook.Kind, + ) + require.NotNil(t, hook.Config) + assert.Equal(t, "Release", hook.Config["configuration"]) + assert.Equal(t, "net10.0", hook.Config["framework"]) + }) + + t.Run("multiple hooks with different configs", + func(t *testing.T) { + const doc = ` +postprovision: + - run: ./hooks/seed-database.cs + kind: dotnet + config: + configuration: Release + - run: ./hooks/setup.py + kind: python + config: + virtualEnvName: .venv +` + + var hooks HooksConfig + err := yaml.Unmarshal([]byte(doc), &hooks) + require.NoError(t, err) + + require.Len(t, hooks["postprovision"], 2) + + dotnetHook := hooks["postprovision"][0] + assert.Equal(t, + "./hooks/seed-database.cs", dotnetHook.Run, + ) + require.NotNil(t, dotnetHook.Config) + assert.Equal(t, + "Release", + dotnetHook.Config["configuration"], + ) + + pythonHook := hooks["postprovision"][1] + assert.Equal(t, + "./hooks/setup.py", pythonHook.Run, + ) + require.NotNil(t, pythonHook.Config) + assert.Equal(t, + ".venv", + pythonHook.Config["virtualEnvName"], + ) + }) } func TestHooksConfig_MarshalYAML(t *testing.T) { @@ -364,6 +430,42 @@ preprovision: got["preprovision"][1].Run, ) }) + + t.Run("roundtrip with config block", func(t *testing.T) { + hooks := HooksConfig{ + "postprovision": { + { + Run: "./hooks/seed.cs", + Kind: language.HookKindDotNet, + Config: map[string]any{ + "configuration": "Release", + "framework": "net10.0", + }, + }, + }, + } + + data, err := yaml.Marshal(hooks) + require.NoError(t, err) + + var got HooksConfig + err = yaml.Unmarshal(data, &got) + require.NoError(t, err) + + require.Len(t, got["postprovision"], 1) + hook := got["postprovision"][0] + assert.Equal(t, "./hooks/seed.cs", hook.Run) + assert.Equal(t, + language.HookKindDotNet, hook.Kind, + ) + require.NotNil(t, hook.Config) + assert.Equal(t, + "Release", hook.Config["configuration"], + ) + assert.Equal(t, + "net10.0", hook.Config["framework"], + ) + }) } func TestHooksConfig_RoundTrip(t *testing.T) { diff --git a/cli/azd/pkg/ext/hooks_runner.go b/cli/azd/pkg/ext/hooks_runner.go index 9e8c1066b41..c100c89e32d 100644 --- a/cli/azd/pkg/ext/hooks_runner.go +++ b/cli/azd/pkg/ext/hooks_runner.go @@ -178,6 +178,7 @@ func (h *HooksRunner) execHook( BoundaryDir: boundaryDir, InlineScript: hookConfig.inlineScript, HookName: hookConfig.Name, + Config: hookConfig.Config, } // Merge caller-provided overrides (e.g. forced interactive from 'azd hooks run'). diff --git a/cli/azd/pkg/ext/hooks_runner_test.go b/cli/azd/pkg/ext/hooks_runner_test.go index ff3625ca3c3..510c9c10144 100644 --- a/cli/azd/pkg/ext/hooks_runner_test.go +++ b/cli/azd/pkg/ext/hooks_runner_test.go @@ -14,6 +14,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/exec" "github.com/azure/azure-dev/cli/azd/pkg/osutil" + "github.com/azure/azure-dev/cli/azd/pkg/tools" "github.com/azure/azure-dev/cli/azd/pkg/tools/language" "github.com/azure/azure-dev/cli/azd/test/mocks" "github.com/azure/azure-dev/cli/azd/test/mocks/mockenv" @@ -1049,3 +1050,193 @@ func Test_ExecHook_DirRunResolution(t *testing.T) { ) }) } + +// configCaptureExecutor is a test-only [tools.HookExecutor] that +// captures the [tools.ExecutionContext] passed to Execute so tests +// can assert on it. +type configCaptureExecutor struct { + onExecute func(tools.ExecutionContext) +} + +func (e *configCaptureExecutor) Prepare( + _ context.Context, _ string, _ tools.ExecutionContext, +) error { + return nil +} + +func (e *configCaptureExecutor) Execute( + _ context.Context, _ string, execCtx tools.ExecutionContext, +) (exec.RunResult, error) { + if e.onExecute != nil { + e.onExecute(execCtx) + } + return exec.NewRunResult(0, "", ""), nil +} + +func (e *configCaptureExecutor) Cleanup( + _ context.Context, +) error { + return nil +} + +// Test_ExecHook_ConfigThreading verifies that HookConfig.Config is +// threaded through to ExecutionContext.Config when executing a hook. +func Test_ExecHook_ConfigThreading(t *testing.T) { + t.Run("ConfigPassedToExecutor", func(t *testing.T) { + cwd := t.TempDir() + ostest.Chdir(t, cwd) + + env := environment.NewWithValues( + "test", map[string]string{}, + ) + + // Create a script file so validate() resolves it. + require.NoError(t, os.MkdirAll( + filepath.Join(cwd, "scripts"), + osutil.PermissionDirectory, + )) + require.NoError(t, os.WriteFile( + filepath.Join(cwd, "scripts", "hook.sh"), + nil, osutil.PermissionExecutableFile, + )) + + expectedConfig := map[string]any{ + "key1": "value1", + "number": 42, + "nested": map[string]any{"a": "b"}, + } + + hooksMap := map[string][]*HookConfig{ + "predeploy": { + { + Name: "predeploy", + Shell: string(language.HookKindBash), + Run: "scripts/hook.sh", + Config: expectedConfig, + }, + }, + } + + envManager := &mockenv.MockEnvManager{} + envManager.On( + "Reload", mock.Anything, env, + ).Return(nil) + + var capturedCtx tools.ExecutionContext + mockContext := mocks.NewMockContext( + context.Background(), + ) + + // Register a capture executor instead of the real + // bash executor so we can inspect ExecutionContext. + mockContext.Container.MustRegisterNamedTransient( + string(language.HookKindBash), + func() tools.HookExecutor { + return &configCaptureExecutor{ + onExecute: func( + ctx tools.ExecutionContext, + ) { + capturedCtx = ctx + }, + } + }, + ) + + hooksManager := NewHooksManager( + HooksManagerOptions{Cwd: cwd}, mockContext.CommandRunner, + ) + runner := NewHooksRunner( + hooksManager, + mockContext.CommandRunner, + envManager, + mockContext.Console, + cwd, + hooksMap, + env, + mockContext.Container, + ) + + err := runner.RunHooks( + *mockContext.Context, HookTypePre, nil, + "deploy", + ) + require.NoError(t, err) + + require.Equal(t, expectedConfig, capturedCtx.Config) + require.Equal(t, "predeploy", capturedCtx.HookName) + }) + + t.Run("NilConfigWhenNotSet", func(t *testing.T) { + cwd := t.TempDir() + ostest.Chdir(t, cwd) + + env := environment.NewWithValues( + "test", map[string]string{}, + ) + + require.NoError(t, os.MkdirAll( + filepath.Join(cwd, "scripts"), + osutil.PermissionDirectory, + )) + require.NoError(t, os.WriteFile( + filepath.Join(cwd, "scripts", "hook.sh"), + nil, osutil.PermissionExecutableFile, + )) + + hooksMap := map[string][]*HookConfig{ + "predeploy": { + { + Name: "predeploy", + Shell: string(language.HookKindBash), + Run: "scripts/hook.sh", + // No Config set. + }, + }, + } + + envManager := &mockenv.MockEnvManager{} + envManager.On( + "Reload", mock.Anything, env, + ).Return(nil) + + var capturedCtx tools.ExecutionContext + mockContext := mocks.NewMockContext( + context.Background(), + ) + + mockContext.Container.MustRegisterNamedTransient( + string(language.HookKindBash), + func() tools.HookExecutor { + return &configCaptureExecutor{ + onExecute: func( + ctx tools.ExecutionContext, + ) { + capturedCtx = ctx + }, + } + }, + ) + + hooksManager := NewHooksManager( + HooksManagerOptions{Cwd: cwd}, mockContext.CommandRunner, + ) + runner := NewHooksRunner( + hooksManager, + mockContext.CommandRunner, + envManager, + mockContext.Console, + cwd, + hooksMap, + env, + mockContext.Container, + ) + + err := runner.RunHooks( + *mockContext.Context, HookTypePre, nil, + "deploy", + ) + require.NoError(t, err) + + require.Nil(t, capturedCtx.Config) + }) +} diff --git a/cli/azd/pkg/ext/models.go b/cli/azd/pkg/ext/models.go index c067020f156..8e8577a20d0 100644 --- a/cli/azd/pkg/ext/models.go +++ b/cli/azd/pkg/ext/models.go @@ -6,6 +6,7 @@ package ext import ( "crypto/sha256" "encoding/hex" + "encoding/json" "errors" "fmt" "maps" @@ -132,6 +133,10 @@ type HookConfig struct { // Environment variables in this list are added to the hook script and if the value is a akvs:// reference // it will be resolved to the secret value Secrets map[string]string `yaml:"secrets,omitempty"` + // Config is an optional property bag of executor-specific settings, + // discriminated by Kind. Each executor may unmarshal this into a + // strongly-typed struct for its own configuration needs. + Config map[string]any `yaml:"config,omitempty"` } // validate normalizes and validates the hook configuration. It resolves @@ -585,6 +590,15 @@ func appendHookConfigSignature(builder *strings.Builder, hookConfig *HookConfig) builder.WriteByte('\x00') } + // Config is a map[string]any — use JSON for deterministic key ordering. + if len(hookConfig.Config) > 0 { + configJSON, err := json.Marshal(hookConfig.Config) + if err == nil { + builder.Write(configJSON) + } + builder.WriteByte('\x00') + } + appendHookConfigSignature(builder, hookConfig.Windows) appendHookConfigSignature(builder, hookConfig.Posix) } diff --git a/cli/azd/pkg/ext/models_test.go b/cli/azd/pkg/ext/models_test.go index 378e47a52cb..e4cb3d3c94a 100644 --- a/cli/azd/pkg/ext/models_test.go +++ b/cli/azd/pkg/ext/models_test.go @@ -480,7 +480,7 @@ func TestHookConfig_ValidateDirRunResolution(t *testing.T) { Run: "main.py", Dir: filepath.Join("hooks", "preprovision"), }, - // Put file in dir but also at root — the + // Put file in dir but also at root ΓÇö the // absolute path case is tested next; here we // verify the dir path is used. createFiles: []string{ @@ -753,7 +753,7 @@ func TestHookConfig_ResolvedPaths(t *testing.T) { func TestHookConfig_ValidatePathTraversal(t *testing.T) { cwd := t.TempDir() - // File-based hook with Dir escaping project root — must fail. + // File-based hook with Dir escaping project root ΓÇö must fail. escapeDir := filepath.Join(cwd, "..", "..", "escape") require.NoError(t, os.MkdirAll(escapeDir, 0o755)) scriptPath := filepath.Join(escapeDir, "evil.sh") @@ -893,7 +893,7 @@ func TestHookConfig_ServiceHookEscapesProjectRoot( ) require.NoError(t, os.MkdirAll(serviceCwd, 0o755)) - // Script file outside the project root — the + // Script file outside the project root ΓÇö the // boundary check must reject this. outsideDir := t.TempDir() scriptPath := filepath.Join(outsideDir, "evil.sh") @@ -1326,3 +1326,154 @@ func TestHookConfig_DirRunKindInference(t *testing.T) { }) } } + +func TestHookConfig_ConfigField(t *testing.T) { + tests := []struct { + name string + yamlInput string + expectedConfig map[string]any + }{ + { + name: "StringValues", + yamlInput: "run: hooks/seed.py\nkind: python\nconfig:\n" + + " framework: net10.0\n configuration: Release\n", + expectedConfig: map[string]any{ + "framework": "net10.0", + "configuration": "Release", + }, + }, + { + name: "NumericValue", + yamlInput: "run: hooks/seed.py\nkind: python\nconfig:\n" + + " retries: 3\n timeout: 30.5\n", + expectedConfig: map[string]any{ + "retries": 3, + "timeout": 30.5, + }, + }, + { + name: "BooleanValue", + yamlInput: "run: hooks/seed.py\nkind: python\nconfig:\n" + + " verbose: true\n", + expectedConfig: map[string]any{ + "verbose": true, + }, + }, + { + name: "NestedMap", + yamlInput: "run: hooks/seed.py\nkind: python\nconfig:\n" + + " database:\n host: localhost\n port: 5432\n", + expectedConfig: map[string]any{ + "database": map[string]any{ + "host": "localhost", + "port": 5432, + }, + }, + }, + { + name: "ListValue", + yamlInput: "run: hooks/seed.py\nkind: python\nconfig:\n" + + " args:\n - --verbose\n - --dry-run\n", + expectedConfig: map[string]any{ + "args": []any{"--verbose", "--dry-run"}, + }, + }, + { + name: "NoConfig", + yamlInput: "run: hooks/seed.py\nkind: python\n", + expectedConfig: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var config HookConfig + err := yaml.Unmarshal([]byte(tt.yamlInput), &config) + require.NoError(t, err) + require.Equal(t, tt.expectedConfig, config.Config) + }) + } +} + +func TestHookConfig_ConfigRoundTrip(t *testing.T) { + original := HookConfig{ + Run: "hooks/deploy.py", + Kind: language.HookKindPython, + Config: map[string]any{ + "framework": "net10.0", + "configuration": "Release", + "nested": map[string]any{"key": "val"}, + }, + } + + data, err := yaml.Marshal(&original) + require.NoError(t, err) + + var decoded HookConfig + err = yaml.Unmarshal(data, &decoded) + require.NoError(t, err) + + require.Equal(t, original.Config, decoded.Config) + require.Equal(t, original.Kind, decoded.Kind) + require.Equal(t, original.Run, decoded.Run) +} + +func TestHookConfig_ConfigOmittedWhenEmpty(t *testing.T) { + config := HookConfig{ + Run: "hooks/deploy.py", + Kind: language.HookKindPython, + } + + data, err := yaml.Marshal(&config) + require.NoError(t, err) + + yamlStr := string(data) + require.NotContains(t, yamlStr, "config") +} + +func TestHooksConfigSignature_IncludesConfig(t *testing.T) { + base := map[string][]*HookConfig{ + "predeploy": { + { + Run: "hooks/deploy.py", + Kind: language.HookKindPython, + }, + }, + } + sigWithout := HooksConfigSignature(base) + require.NotEmpty(t, sigWithout) + + withConfig := map[string][]*HookConfig{ + "predeploy": { + { + Run: "hooks/deploy.py", + Kind: language.HookKindPython, + Config: map[string]any{ + "framework": "net10.0", + }, + }, + }, + } + sigWith := HooksConfigSignature(withConfig) + require.NotEmpty(t, sigWith) + require.NotEqual(t, sigWithout, sigWith, + "signature should change when Config is added", + ) + + // Different Config values should produce different signatures. + withDifferentConfig := map[string][]*HookConfig{ + "predeploy": { + { + Run: "hooks/deploy.py", + Kind: language.HookKindPython, + Config: map[string]any{ + "framework": "net9.0", + }, + }, + }, + } + sigDiff := HooksConfigSignature(withDifferentConfig) + require.NotEqual(t, sigWith, sigDiff, + "signature should differ for different Config values", + ) +} diff --git a/cli/azd/pkg/tools/script.go b/cli/azd/pkg/tools/script.go index 0740aa516c4..bf10fc56643 100644 --- a/cli/azd/pkg/tools/script.go +++ b/cli/azd/pkg/tools/script.go @@ -47,6 +47,11 @@ type ExecutionContext struct { // "preprovision"). Used by executors for temp file naming to // aid debuggability. HookName string + + // Config is the executor-specific property bag from HookConfig. + // Executors can unmarshal this into a typed struct for their + // configuration needs. May be nil if no config was specified. + Config map[string]any } // HookExecutor is the unified interface for all hook execution. From 9e1bfc788007d22635757bf2efe612e462a80383 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Mon, 13 Apr 2026 14:17:17 -0700 Subject: [PATCH 02/10] Add executor-specific config support with strongly-typed unmarshalling Implement config integration for all hook executors, building on the Config property bag added to HookConfig and ExecutionContext. Shared infrastructure: - Add generic UnmarshalHookConfig[T] helper for type-safe config parsing from map[string]any via JSON re-marshal JS/TS executors (#7692): - Add packageManager config (npm, pnpm, yarn) matching the existing ServiceConfig pattern from framework_service_node.go - Override auto-detection when config is specified - Shared validation in node_helpers.go used by both executors Python executor (#7693): - Add virtualEnvName config to override default {baseName}_env naming - Path traversal validation rejects separators, dot components .NET executor (#7694): - Add configuration config (Debug, Release, or custom MSBuild configs) - Add framework config for target framework moniker (net8.0, net10.0) - Flags applied in project mode only, single-file mode unchanged All changes include comprehensive table-driven tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/.vscode/cspell.yaml | 3 + cli/azd/docs/hook-config-integration/plan.md | 29 +++ cli/azd/docs/hook-config-integration/todos.md | 127 ++++++++++++ cli/azd/pkg/tools/hook_config.go | 31 +++ cli/azd/pkg/tools/hook_config_test.go | 144 ++++++++++++++ cli/azd/pkg/tools/language/dotnet_executor.go | 54 ++++- .../tools/language/dotnet_executor_test.go | 168 +++++++++++++++- cli/azd/pkg/tools/language/js_executor.go | 3 +- .../pkg/tools/language/js_executor_test.go | 53 +++++ cli/azd/pkg/tools/language/node_helpers.go | 73 ++++++- .../pkg/tools/language/node_helpers_test.go | 186 +++++++++++++++++- cli/azd/pkg/tools/language/python_executor.go | 60 +++++- .../tools/language/python_executor_test.go | 160 +++++++++++++++ cli/azd/pkg/tools/language/ts_executor.go | 3 +- .../pkg/tools/language/ts_executor_test.go | 53 +++++ 15 files changed, 1122 insertions(+), 25 deletions(-) create mode 100644 cli/azd/docs/hook-config-integration/plan.md create mode 100644 cli/azd/docs/hook-config-integration/todos.md create mode 100644 cli/azd/pkg/tools/hook_config.go create mode 100644 cli/azd/pkg/tools/hook_config_test.go diff --git a/cli/azd/.vscode/cspell.yaml b/cli/azd/.vscode/cspell.yaml index 386ee69820e..347739ea097 100644 --- a/cli/azd/.vscode/cspell.yaml +++ b/cli/azd/.vscode/cspell.yaml @@ -383,6 +383,9 @@ overrides: - unvalidated - venvs - workdir + - filename: extensions/azure.ai.agents/internal/cmd/init_locations.go + words: + - swedencentral - filename: docs/code-coverage-guide.md words: - covdata diff --git a/cli/azd/docs/hook-config-integration/plan.md b/cli/azd/docs/hook-config-integration/plan.md new file mode 100644 index 00000000000..9a517b874f4 --- /dev/null +++ b/cli/azd/docs/hook-config-integration/plan.md @@ -0,0 +1,29 @@ +# Hook Executor Config Integration — Development Plan + +## Overview + +Integrate the `Config map[string]any` property bag (PR #7690) into each hook executor, enabling users to configure executor-specific settings in `azure.yaml`. Properties mirror existing `ServiceConfig.Config` patterns where applicable. + +**Parent Issue:** #7653 (Add generic config property bag to HookConfig) +**Parent Epic:** #7435 (Multi-Language Hook Support) +**Prerequisite:** PR #7690 (core Config plumbing — in draft) + +## Scope Assessment + +3 issues, all parallelizable. Flat plan (no epics needed). + +## Sequencing + +All three issues can be implemented in parallel — they each modify a single executor with no cross-dependencies. All depend on PR #7690 (core Config bag) being merged first. + +``` +PR #7690 (core Config bag) ──┬── Issue 1 (JS/TS packageManager) + ├── Issue 2 (Python virtualEnvName) + └── Issue 3 (.NET configuration + framework) +``` + +## Cross-Cutting Concerns + +- **Shared validation pattern**: Each executor reads from `execCtx.Config` and validates its own keys. Consider extracting a shared `configString(config map[string]any, key string) (string, error)` helper if patterns converge. +- **JSON schema**: After all executors are integrated, update `azure.yaml.json` with `oneOf` discriminated on `kind` to validate per-executor config shapes. (Separate issue.) +- **Documentation**: After all executors are integrated, update `docs/language-hooks.md` with per-executor config options. (Separate issue.) diff --git a/cli/azd/docs/hook-config-integration/todos.md b/cli/azd/docs/hook-config-integration/todos.md new file mode 100644 index 00000000000..72478de2417 --- /dev/null +++ b/cli/azd/docs/hook-config-integration/todos.md @@ -0,0 +1,127 @@ +# Hook Executor Config Integration — Issues + +## Issue 1: Add `packageManager` config to JS/TS hook executors + +**Background & Motivation** +Users running JS/TS hooks need to use the same package manager as their project (pnpm, yarn). Today the hook executor auto-detects but provides no override, unlike the framework service which supports `ServiceConfig.Config["packageManager"]`. + +**User Story** +As a developer using pnpm/yarn, I want to specify `packageManager` in my hook config so that my JS/TS hooks use the correct package manager without relying on lock file detection. + +**Solution Approach** +- Read `Config["packageManager"]` in JS/TS executor `Prepare()` phase +- Reuse existing `packageManagerFromConfig()` validation pattern from `framework_service_node.go` +- Pass override to `node.NewCliWithPackageManager()` instead of auto-detecting +- Applies to both `js_executor.go` and `ts_executor.go` (shared via `node_helpers.go`) +- Accepted values: `npm`, `pnpm`, `yarn` (same as ServiceConfig) + +**Acceptance Criteria** +- [ ] `config.packageManager: pnpm` causes JS/TS hooks to use pnpm for install +- [ ] `config.packageManager: yarn` causes JS/TS hooks to use yarn for install +- [ ] Omitting `packageManager` preserves current auto-detection behavior (lock file → default npm) +- [ ] Invalid values (e.g., `bun`) produce a clear error message +- [ ] Unit tests covering override, fallback, and invalid value cases + +**Default when omitted:** Auto-detect from lock files (existing behavior) + +**Out of Scope** +- Custom install flags (e.g., `--no-audit`) — future P2 enhancement +- TypeScript runtime selection (`tsRuntime`) — separate P1 issue +- Bun or Deno support — not currently supported in azd + +**Testing Expectations** +- Unit tests: table-driven tests for config parsing, validation, and CLI selection +- Test that override beats auto-detection when both are present + +**Type:** backend +**Size:** M (4-7 files) — `js_executor.go`, `ts_executor.go`, `node_helpers.go`, tests +**Labels:** `area/hooks`, `enhancement` +**Depends on:** PR #7690 (core Config plumbing) +**Related:** #7653, #7435 + +--- + +## Issue 2: Add `virtualEnvName` config to Python hook executor + +**Background & Motivation** +Python hook scripts default to `{baseName}_env` for virtual environments, but many Python projects use `.venv` by convention. The issue #7653 explicitly calls out `virtualEnvName` as a key config property for Python hooks. + +**User Story** +As a Python developer, I want to specify `virtualEnvName` in my hook config so that my Python hooks use my preferred virtual environment directory. + +**Solution Approach** +- Read `Config["virtualEnvName"]` in `python_executor.go` `Prepare()` phase +- If set, use as the venv name instead of `VenvNameForDir()` default +- Validate it's a string, non-empty, and doesn't contain path separators (security: prevent path traversal) +- Pass to existing `EnsureVirtualEnv()` and `InstallDependencies()` methods + +**Acceptance Criteria** +- [ ] `config.virtualEnvName: .venv` creates/uses `.venv` directory for the hook +- [ ] `config.virtualEnvName: my_env` creates/uses `my_env` directory +- [ ] Omitting `virtualEnvName` preserves current behavior (search `.venv`/`venv`, fall back to `{baseName}_env`) +- [ ] Values with path separators (`/`, `\`) are rejected with a clear error +- [ ] Empty string values are rejected with a clear error +- [ ] Unit tests covering override, fallback, and validation + +**Default when omitted:** Existing behavior — search for `.venv`/`venv` in project dir, then fall back to `{baseName}_env` + +**Out of Scope** +- Python package manager selection (`uv`, `poetry`, `pdm`) — future P2 enhancement +- Custom requirements file name — future enhancement +- Python binary path override — future enhancement + +**Testing Expectations** +- Unit tests: table-driven tests for config parsing, validation, and venv name resolution +- Test path traversal rejection (e.g., `../evil`) + +**Type:** backend +**Size:** S (1-3 files) — `python_executor.go`, test file +**Labels:** `area/hooks`, `enhancement` +**Depends on:** PR #7690 (core Config plumbing) +**Related:** #7653, #7435 + +--- + +## Issue 3: Add `configuration` and `framework` config to .NET hook executor + +**Background & Motivation** +The .NET hook executor builds with an empty configuration (SDK default = Debug), while the framework service hardcodes `Release`. Users need control over the build configuration for hook scripts, especially for production seed/migration hooks. Multi-targeting users also need to specify a target framework moniker. + +**User Story** +As a .NET developer, I want to specify `configuration` and `framework` in my hook config so that my .NET hooks build with the correct settings. + +**Solution Approach** +- Read `Config["configuration"]` in `dotnet_executor.go` `Prepare()` phase +- Pass to `dotnetCli.Build()` instead of empty string +- Validate `configuration` is a string (no enum restriction — MSBuild allows custom configurations like `Staging`) +- Read `Config["framework"]` for target framework moniker +- Pass as `-f` / `--framework` flag to `dotnet run` in `Execute()` phase + +**Acceptance Criteria** +- [ ] `config.configuration: Release` builds hook script in Release mode +- [ ] `config.configuration: Debug` explicitly builds in Debug mode +- [ ] `config.framework: net10.0` targets specific framework for both build and run +- [ ] Omitting `configuration` preserves current behavior (empty string → SDK default) +- [ ] Omitting `framework` preserves current behavior (auto-detected from project) +- [ ] Both can be specified together +- [ ] Unit tests covering configuration override, framework override, combined, and fallback + +**Defaults when omitted:** +- `configuration`: empty string (SDK default, typically Debug) +- `framework`: not set (auto-detected from project file) + +**Out of Scope** +- Custom dotnet run arguments — future enhancement +- Suppressing DOTNET_NOLOGO — low priority +- Runtime identifier (`-r`) support — future enhancement + +**Testing Expectations** +- Unit tests: table-driven tests for config parsing and dotnet CLI argument construction +- Test that configuration is passed to both Build() and Run() commands +- Test that framework is passed to Run() command + +**Type:** backend +**Size:** S (1-3 files) — `dotnet_executor.go`, test file +**Labels:** `area/hooks`, `enhancement` +**Depends on:** PR #7690 (core Config plumbing) +**Related:** #7653, #7435 diff --git a/cli/azd/pkg/tools/hook_config.go b/cli/azd/pkg/tools/hook_config.go new file mode 100644 index 00000000000..e24889dab6e --- /dev/null +++ b/cli/azd/pkg/tools/hook_config.go @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tools + +import ( + "encoding/json" + "fmt" +) + +// UnmarshalHookConfig unmarshals the raw config map into a typed struct. +// Returns a zero-value T if config is nil or empty. +// Uses JSON re-marshal for deterministic conversion with proper type checking. +func UnmarshalHookConfig[T any](config map[string]any) (T, error) { + var result T + + if len(config) == 0 { + return result, nil + } + + data, err := json.Marshal(config) + if err != nil { + return result, fmt.Errorf("marshalling hook config to JSON: %w", err) + } + + if err := json.Unmarshal(data, &result); err != nil { + return result, fmt.Errorf("unmarshalling hook config: %w", err) + } + + return result, nil +} diff --git a/cli/azd/pkg/tools/hook_config_test.go b/cli/azd/pkg/tools/hook_config_test.go new file mode 100644 index 00000000000..2ceb31d405e --- /dev/null +++ b/cli/azd/pkg/tools/hook_config_test.go @@ -0,0 +1,144 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tools + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +type testNodeConfig struct { + PackageManager string `json:"packageManager"` +} + +type testPythonConfig struct { + VirtualEnvName string `json:"virtualEnvName"` +} + +type testDotnetConfig struct { + Configuration string `json:"configuration"` + Framework string `json:"framework"` +} + +type testNestedConfig struct { + Name string `json:"name"` + Count int `json:"count"` + Verbose bool `json:"verbose"` +} + +func TestUnmarshalHookConfig(t *testing.T) { + tests := []struct { + name string + run func(t *testing.T) + }{ + { + name: "NilConfig", + run: func(t *testing.T) { + result, err := UnmarshalHookConfig[testNodeConfig](nil) + require.NoError(t, err) + require.Equal(t, testNodeConfig{}, result) + }, + }, + { + name: "EmptyConfig", + run: func(t *testing.T) { + result, err := UnmarshalHookConfig[testNodeConfig](map[string]any{}) + require.NoError(t, err) + require.Equal(t, testNodeConfig{}, result) + }, + }, + { + name: "ValidStringField", + run: func(t *testing.T) { + config := map[string]any{ + "packageManager": "pnpm", + } + result, err := UnmarshalHookConfig[testNodeConfig](config) + require.NoError(t, err) + require.Equal(t, "pnpm", result.PackageManager) + }, + }, + { + name: "ValidPythonConfig", + run: func(t *testing.T) { + config := map[string]any{ + "virtualEnvName": ".myenv", + } + result, err := UnmarshalHookConfig[testPythonConfig](config) + require.NoError(t, err) + require.Equal(t, ".myenv", result.VirtualEnvName) + }, + }, + { + name: "ValidDotnetConfigMultipleFields", + run: func(t *testing.T) { + config := map[string]any{ + "configuration": "Release", + "framework": "net8.0", + } + result, err := UnmarshalHookConfig[testDotnetConfig](config) + require.NoError(t, err) + require.Equal(t, "Release", result.Configuration) + require.Equal(t, "net8.0", result.Framework) + }, + }, + { + name: "ValidNestedTypes", + run: func(t *testing.T) { + config := map[string]any{ + "name": "test-hook", + "count": 42, + "verbose": true, + } + result, err := UnmarshalHookConfig[testNestedConfig](config) + require.NoError(t, err) + require.Equal(t, "test-hook", result.Name) + require.Equal(t, 42, result.Count) + require.True(t, result.Verbose) + }, + }, + { + name: "TypeMismatchReturnsError", + run: func(t *testing.T) { + config := map[string]any{ + "packageManager": 123, + } + // JSON numbers unmarshal into strings as an error + _, err := UnmarshalHookConfig[testNodeConfig](config) + require.Error(t, err) + require.Contains(t, err.Error(), "unmarshalling hook config") + }, + }, + { + name: "UnknownKeysIgnored", + run: func(t *testing.T) { + config := map[string]any{ + "packageManager": "npm", + "unknownField": "some-value", + "anotherExtra": 42, + } + result, err := UnmarshalHookConfig[testNodeConfig](config) + require.NoError(t, err) + require.Equal(t, "npm", result.PackageManager) + }, + }, + { + name: "PartialConfigUsesZeroDefaults", + run: func(t *testing.T) { + config := map[string]any{ + "configuration": "Debug", + } + result, err := UnmarshalHookConfig[testDotnetConfig](config) + require.NoError(t, err) + require.Equal(t, "Debug", result.Configuration) + require.Empty(t, result.Framework) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, tt.run) + } +} diff --git a/cli/azd/pkg/tools/language/dotnet_executor.go b/cli/azd/pkg/tools/language/dotnet_executor.go index cec5cc48379..7cc6977fc5c 100644 --- a/cli/azd/pkg/tools/language/dotnet_executor.go +++ b/cli/azd/pkg/tools/language/dotnet_executor.go @@ -35,6 +35,20 @@ var minSingleFileVersion = semver.Version{ Major: 10, Minor: 0, Patch: 0, } +// dotnetHookConfig holds the typed configuration that users can +// specify in azure.yaml under a .NET hook's config section. +type dotnetHookConfig struct { + // Configuration is the MSBuild configuration (Debug, Release, + // or a custom name). Passed as `-c` to dotnet build and + // dotnet run. Empty means use the SDK default (Debug). + Configuration string `json:"configuration"` + + // Framework is the target framework moniker (e.g. net8.0, + // net10.0). Passed as `--framework` to dotnet run so the + // correct TFM is selected in multi-target projects. + Framework string `json:"framework"` +} + // dotnetExecutor implements [tools.HookExecutor] for .NET (C#) // scripts. It supports two execution modes: // - Project mode: when a .csproj/.fsproj/.vbproj is discovered @@ -48,6 +62,9 @@ type dotnetExecutor struct { // projectPath is set by Prepare when a .NET project file is // discovered. Empty means single-file mode. projectPath string + + // config holds parsed hook configuration from azure.yaml. + config dotnetHookConfig } // NewDotNetExecutor creates a .NET HookExecutor. @@ -97,7 +114,18 @@ func (e *dotnetExecutor) Prepare( } } - // 2. Discover .NET project context (.csproj/.fsproj/.vbproj). + // 2. Parse executor-specific config (configuration, framework). + cfg, err := tools.UnmarshalHookConfig[dotnetHookConfig]( + execCtx.Config, + ) + if err != nil { + return fmt.Errorf( + "parsing .NET hook config: %w", err, + ) + } + e.config = cfg + + // 3. Discover .NET project context (.csproj/.fsproj/.vbproj). // Uses DiscoverDotNetProject instead of the generic // DiscoverProjectFile to avoid Python/Node.js project files // shadowing the .NET project file in mixed-language directories. @@ -110,7 +138,7 @@ func (e *dotnetExecutor) Prepare( ) } - // 3a. Project mode: restore and build. + // 4a. Project mode: restore and build. if projCtx != nil { if err := e.dotnetCli.Restore( ctx, projCtx.DependencyFile, execCtx.EnvVars, @@ -122,7 +150,8 @@ func (e *dotnetExecutor) Prepare( } if err := e.dotnetCli.Build( - ctx, projCtx.DependencyFile, "", "", + ctx, projCtx.DependencyFile, + e.config.Configuration, "", execCtx.EnvVars, ); err != nil { return fmt.Errorf( @@ -135,7 +164,7 @@ func (e *dotnetExecutor) Prepare( return nil } - // 3b. Single-file mode: validate SDK version >= 10. + // 4b. Single-file mode: validate SDK version >= 10. sdkVer, err := e.dotnetCli.SdkVersion(ctx) if err != nil { return fmt.Errorf( @@ -188,11 +217,22 @@ func (e *dotnetExecutor) Execute( if e.projectPath != "" { // Project mode — skip restore/build since Prepare // already ran them. - runArgs = exec.NewRunArgs( - "dotnet", "run", + args := []string{ + "run", "--project", e.projectPath, "--no-build", - ) + } + + if e.config.Configuration != "" { + args = append(args, "-c", e.config.Configuration) + } + if e.config.Framework != "" { + args = append( + args, "--framework", e.config.Framework, + ) + } + + runArgs = exec.NewRunArgs("dotnet", args...) } else { // Single-file mode. runArgs = exec.NewRunArgs( diff --git a/cli/azd/pkg/tools/language/dotnet_executor_test.go b/cli/azd/pkg/tools/language/dotnet_executor_test.go index 89fb865df21..a444e645a28 100644 --- a/cli/azd/pkg/tools/language/dotnet_executor_test.go +++ b/cli/azd/pkg/tools/language/dotnet_executor_test.go @@ -29,10 +29,11 @@ type mockDotNetTools struct { restoreErr error buildErr error - restoreCalled bool - restoreProject string - buildCalled bool - buildProject string + restoreCalled bool + restoreProject string + buildCalled bool + buildProject string + buildConfiguration string } func (m *mockDotNetTools) CheckInstalled( @@ -59,11 +60,12 @@ func (m *mockDotNetTools) Restore( func (m *mockDotNetTools) Build( _ context.Context, - project string, _ string, + project string, configuration string, _ string, _ []string, ) error { m.buildCalled = true m.buildProject = project + m.buildConfiguration = configuration return m.buildErr } @@ -596,3 +598,159 @@ func TestDotNetExecutor_TableDriven(t *testing.T) { }) } } + +// --------------------------------------------------------------------------- +// Hook config tests — configuration and framework flags +// --------------------------------------------------------------------------- + +func TestDotNetExecutor_HookConfig(t *testing.T) { + tests := []struct { + name string + config map[string]any + wantBuildCfg string + wantRunHas []string // substrings expected in run args + wantRunMissing []string // substrings that must NOT appear + }{ + { + name: "ConfigurationRelease", + config: map[string]any{"configuration": "Release"}, + wantBuildCfg: "Release", + wantRunHas: []string{"-c", "Release"}, + }, + { + name: "ConfigurationDebug", + config: map[string]any{"configuration": "Debug"}, + wantBuildCfg: "Debug", + wantRunHas: []string{"-c", "Debug"}, + }, + { + name: "FrameworkOnly", + config: map[string]any{"framework": "net10.0"}, + wantBuildCfg: "", + wantRunHas: []string{"--framework", "net10.0"}, + wantRunMissing: []string{"-c"}, + }, + { + name: "BothConfigAndFramework", + config: map[string]any{ + "configuration": "Release", + "framework": "net8.0", + }, + wantBuildCfg: "Release", + wantRunHas: []string{ + "-c", "Release", + "--framework", "net8.0", + }, + }, + { + name: "NeitherSet", + config: nil, + wantBuildCfg: "", + wantRunMissing: []string{"-c", "--framework"}, + }, + { + name: "EmptyConfigMap", + config: map[string]any{}, + wantBuildCfg: "", + wantRunMissing: []string{"-c", "--framework"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + projectDir := filepath.Join(root, "proj") + require.NoError(t, + os.MkdirAll(projectDir, 0o700), + ) + writeFile(t, + filepath.Join(projectDir, "App.csproj"), + "", + ) + + cli := &mockDotNetTools{} + runner := &mockCommandRunner{} + e := newDotNetExecutorInternal(runner, cli) + + execCtx := tools.ExecutionContext{ + BoundaryDir: root, + Cwd: projectDir, + Config: tt.config, + } + scriptPath := filepath.Join( + projectDir, "hook.cs", + ) + + require.NoError(t, + e.Prepare(t.Context(), scriptPath, execCtx), + ) + + // Verify Build received the configuration. + assert.Equal(t, tt.wantBuildCfg, + cli.buildConfiguration, + "Build configuration mismatch", + ) + + _, err := e.Execute( + t.Context(), scriptPath, execCtx, + ) + require.NoError(t, err) + + // Verify run args contain expected flags. + runArgs := runner.lastRunArgs.Args + for _, want := range tt.wantRunHas { + assert.Contains(t, runArgs, want, + "run args should contain %q", want, + ) + } + + // Verify run args do NOT contain excluded flags. + for _, absent := range tt.wantRunMissing { + assert.NotContains(t, runArgs, absent, + "run args should not contain %q", + absent, + ) + } + + // Always verify project-mode invariants. + assert.Contains(t, runArgs, "--project") + assert.Contains(t, runArgs, "--no-build") + }) + } +} + +func TestDotNetExecutor_SingleFileIgnoresConfig(t *testing.T) { + dir := t.TempDir() + runner := &mockCommandRunner{} + cli := &mockDotNetTools{ + sdkVersionResult: semver.Version{ + Major: 10, Minor: 0, Patch: 0, + }, + } + e := newDotNetExecutorInternal(runner, cli) + + scriptPath := filepath.Join(dir, "hook.cs") + execCtx := tools.ExecutionContext{ + BoundaryDir: dir, + Config: map[string]any{ + "configuration": "Release", + "framework": "net10.0", + }, + } + + require.NoError(t, + e.Prepare(t.Context(), scriptPath, execCtx), + ) + + _, err := e.Execute( + t.Context(), scriptPath, execCtx, + ) + require.NoError(t, err) + + // Single-file mode uses "dotnet run