Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/azd/cmd/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ func registerCommonDependencies(container *ioc.NestedContainer) {
container.MustRegisterSingleton(func() workflow.AzdCommandRunner {
return &workflowCmdAdapter{
newCommand: func() *cobra.Command {
return NewRootCmd(false, nil, container)
return newRootCmdWithoutRegistration(container)
},
globalArgs: extractGlobalArgs(),
}
Expand Down
151 changes: 149 additions & 2 deletions cli/azd/cmd/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,12 @@ type testConcreteComponent[T comparable] struct {
concrete T
}

// Test_WorkflowCmdAdapter_ContextPropagation validates that the workflowCmdAdapter
// Test_workflowCmdAdapter_ContextPropagation validates that the workflowCmdAdapter
// properly marks contexts as child actions when executing subcommands.
// The main.go entrypoint wraps the root context with context.WithoutCancel,
// so workflow steps always receive a non-cancellable context.
// See: https://github.com/Azure/azure-dev/issues/6530
func Test_WorkflowCmdAdapter_ContextPropagation(t *testing.T) {
func Test_workflowCmdAdapter_ContextPropagation(t *testing.T) {
t.Run("SubcommandReceivesChildActionContext", func(t *testing.T) {
// Track which contexts were seen by the subcommand
var receivedContexts []context.Context
Expand Down Expand Up @@ -409,4 +409,151 @@ func Test_WorkflowCmdAdapter_ContextPropagation(t *testing.T) {
require.True(t, foundProvision2, "second tree: provision command should be registered")
require.NotSame(t, rootCmd, rootCmd2, "each NewRootCmd call should produce a distinct instance")
})

t.Run("WorkflowAdapterMiddlewareRunsForChildActions", func(t *testing.T) {
// Verify that when the workflowCmdAdapter executes a command, the middleware chain
// (registered on the command tree) is invoked despite the context being a child action.
// This validates that hooks middleware would fire during workflow step execution.
var middlewareRan bool
var receivedIsChild bool

newCommand := func() *cobra.Command {
rootCmd := &cobra.Command{Use: "root"}

// Create a child action descriptor-style setup:
// The "provision" command wraps its RunE to simulate middleware execution
provisionCmd := &cobra.Command{
Use: "provision",
RunE: func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
middlewareRan = true
receivedIsChild = middleware.IsChildAction(ctx)
return nil
},
}
rootCmd.AddCommand(provisionCmd)
return rootCmd
}

adapter := &workflowCmdAdapter{newCommand: newCommand}
ctx := context.WithoutCancel(context.Background())

// Execute "provision" through the adapter (simulates workflow step)
err := adapter.ExecuteContext(ctx, []string{"provision"})
require.NoError(t, err)
require.True(t, middlewareRan, "Provision command should have been executed")
require.True(t, receivedIsChild,
"Context should be marked as child action when executed through workflow adapter")
})

t.Run("WorkflowAdapterMiddlewareChainForAllSteps", func(t *testing.T) {
// Simulate the full workflow execution path: package → provision → deploy
// Verify each step's command runs with the child action context and fresh tree
var executedCommands []string

newCommand := func() *cobra.Command {
rootCmd := &cobra.Command{Use: "root"}

for _, cmdName := range []string{"package", "provision", "deploy"} {
name := cmdName // capture for closure
cmd := &cobra.Command{
Use: name,
RunE: func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
require.True(t, middleware.IsChildAction(ctx),
"Step %q should have child action context", name)
executedCommands = append(executedCommands, name)
return nil
},
}
if name == "package" || name == "deploy" {
cmd.Flags().Bool("all", false, "")
}
rootCmd.AddCommand(cmd)
}
return rootCmd
}

adapter := &workflowCmdAdapter{newCommand: newCommand}
ctx := context.WithoutCancel(context.Background())

// Simulate the default "up" workflow steps
steps := [][]string{
{"package", "--all"},
{"provision"},
{"deploy", "--all"},
}

for _, args := range steps {
err := adapter.ExecuteContext(ctx, args)
require.NoError(t, err, "Step %v should succeed", args)
}

require.Equal(t, []string{"package", "provision", "deploy"}, executedCommands,
"All workflow steps should execute in order")
})
}

func Test_NewRootCmd_ReregistrationReplacesProjectConfig(t *testing.T) {
// This test proves the regression from PR #7171: when workflowCmdAdapter called
// NewRootCmd (with full registration) for each workflow step, registerCommonDependencies
// re-registered singletons. The golobby IoC container replaces cached singleton instances
// on re-registration, so event handlers registered on ProjectConfig/ServiceConfig (by the
// hooks middleware) were silently lost.
//
// Steps:
// 1. Create root command (registers dependencies)
// 2. Resolve ProjectConfig, add an event handler
// 3. Create another root command (re-registers dependencies)
// 4. Resolve ProjectConfig again
// 5. Validate the handler is gone (proving the bug)
// 6. Use newRootCmdWithoutRegistration instead, validate handler is preserved (proving the fix)

container := ioc.NewNestedContainer(nil)
ctx := context.WithoutCancel(context.Background())
ioc.RegisterInstance(container, ctx)
ioc.RegisterInstance(container, &internal.GlobalCommandOptions{})

// Set up a project directory with azure.yaml so ProjectConfig can be resolved
dir := t.TempDir()
t.Chdir(dir)
azdCtx := azdcontext.NewAzdContextWithDirectory(dir)
ioc.RegisterInstance(container, azdCtx)

projectConfig := &project.ProjectConfig{
Name: "test-project",
}
_ = project.Save(ctx, projectConfig, azdCtx.ProjectPath())

// Step 1: Create root command (registers dependencies including ProjectConfig factory)
_ = NewRootCmd(false, nil, container)

// Step 2: Resolve ProjectConfig and add an event handler (simulates hooks middleware)
var pc1 *project.ProjectConfig
require.NoError(t, container.Resolve(&pc1))

// Step 3: Create another root command with full re-registration
_ = NewRootCmd(false, nil, container)

// Step 4: Resolve ProjectConfig again
var pc2 *project.ProjectConfig
require.NoError(t, container.Resolve(&pc2))

// Step 5: The re-registration replaced the singleton — it's a different instance
require.NotSame(t, pc1, pc2,
"BUG PROOF: NewRootCmd re-registration replaces the cached ProjectConfig singleton, "+
"losing any event handlers attached to the original instance")

// Step 6: Now use newRootCmdWithoutRegistration and verify the instance is preserved
var pc3 *project.ProjectConfig
require.NoError(t, container.Resolve(&pc3))

_ = newRootCmdWithoutRegistration(container)

var pc4 *project.ProjectConfig
require.NoError(t, container.Resolve(&pc4))

require.Same(t, pc3, pc4,
"FIX PROOF: newRootCmdWithoutRegistration preserves the cached ProjectConfig singleton, "+
"keeping event handlers intact")
}
181 changes: 180 additions & 1 deletion cli/azd/cmd/middleware/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,17 @@ func runMiddleware(
projectConfig *project.ProjectConfig,
runOptions *Options,
nextFn NextFn,
) (*actions.ActionResult, error) {
return runMiddlewareWithContext(*mockContext.Context, mockContext, envName, projectConfig, runOptions, nextFn)
}

func runMiddlewareWithContext(
ctx context.Context,
mockContext *mocks.MockContext,
envName string,
projectConfig *project.ProjectConfig,
runOptions *Options,
nextFn NextFn,
) (*actions.ActionResult, error) {
env := environment.NewWithValues(envName, nil)

Expand All @@ -354,7 +365,7 @@ func runMiddleware(
mockContext.Container,
)

result, err := middleware.Run(*mockContext.Context, nextFn)
result, err := middleware.Run(ctx, nextFn)

return result, err
}
Expand Down Expand Up @@ -540,6 +551,174 @@ func Test_PowerShellWarning_WithoutPowerShellHooks(t *testing.T) {
require.False(t, foundWarning, "Expected no PowerShell warning for bash hooks")
}

// Test_CommandHooks_ChildAction_HooksStillFire verifies that command hooks fire even when running
// as a child action (e.g., "provision" step inside "azd up" workflow). PR #7171 changed the
// workflowCmdAdapter to rebuild the command tree, and this test ensures hooks still execute
// for workflow step commands.
func Test_CommandHooks_ChildAction_HooksStillFire(t *testing.T) {
tests := []struct {
name string
commandPath string
hookName string
}{
{
name: "ProvisionHooksFireInWorkflow",
commandPath: "azd provision",
hookName: "preprovision",
},
{
name: "DeployHooksFireInWorkflow",
commandPath: "azd deploy",
hookName: "predeploy",
},
{
name: "PackageHooksFireInWorkflow",
commandPath: "azd package",
hookName: "prepackage",
},
{
name: "RestoreHooksFireInWorkflow",
commandPath: "azd restore",
hookName: "prerestore",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
azdContext := createAzdContext(t)

envName := "test"
runOptions := Options{CommandPath: tt.commandPath}

projectConfig := project.ProjectConfig{
Name: envName,
Hooks: map[string][]*ext.HookConfig{
tt.hookName: {
{
Run: "echo 'hook running'",
Shell: ext.ShellTypeBash,
},
},
},
}

err := ensureAzdValid(mockContext, azdContext, envName, &projectConfig)
require.NoError(t, err)

nextFn, actionRan := createNextFn()
hookRan := setupHookMock(mockContext, 0)

// Simulate workflow execution: mark context as child action
childCtx := WithChildAction(*mockContext.Context)
result, err := runMiddlewareWithContext(
childCtx, mockContext, envName, &projectConfig, &runOptions, nextFn,
)

require.NotNil(t, result)
require.NoError(t, err)
require.True(t, *hookRan, "Hook %q should fire even when running as a child action (workflow step)", tt.hookName)
require.True(t, *actionRan, "Action should run for child action")
})
}
}

// Test_CommandHooks_ChildAction_SkipsValidationOnly verifies that when running as a child action,
// hook validation warnings are suppressed but hooks still execute. This ensures the IsChildAction
// guard in HooksMiddleware.Run() only affects validation, not hook execution itself.
func Test_CommandHooks_ChildAction_SkipsValidationOnly(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
azdContext := createAzdContext(t)

envName := "test"
runOptions := Options{CommandPath: "azd provision"}

projectConfig := project.ProjectConfig{
Name: envName,
Hooks: map[string][]*ext.HookConfig{
"preprovision": {
{
Run: "echo 'preprovision hook'",
Shell: ext.ShellTypeBash,
},
},
"postprovision": {
{
Run: "echo 'postprovision hook'",
Shell: ext.ShellTypeBash,
},
},
},
}

err := ensureAzdValid(mockContext, azdContext, envName, &projectConfig)
require.NoError(t, err)

hookCount := 0
mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool {
return true
}).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) {
hookCount++
return exec.NewRunResult(0, "", ""), nil
})

nextFn, actionRan := createNextFn()

// Execute as child action (workflow step)
childCtx := WithChildAction(*mockContext.Context)
result, err := runMiddlewareWithContext(
childCtx, mockContext, envName, &projectConfig, &runOptions, nextFn,
)

require.NotNil(t, result)
require.NoError(t, err)
require.True(t, *actionRan, "Action should run")

// Both pre and post hooks should fire (2 hooks total)
require.Equal(t, 2, hookCount,
"Both preprovision and postprovision hooks should fire for child actions")
}

// Test_CommandHooks_ChildAction_PreHookError_StopsAction verifies that when running as a child
// action, a failing pre-hook still prevents the action from executing (same behavior as direct
// command execution).
func Test_CommandHooks_ChildAction_PreHookError_StopsAction(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
azdContext := createAzdContext(t)

envName := "test"
runOptions := Options{CommandPath: "azd provision"}

projectConfig := project.ProjectConfig{
Name: envName,
Hooks: map[string][]*ext.HookConfig{
"preprovision": {
{
Run: "exit 1",
Shell: ext.ShellTypeBash,
},
},
},
}

err := ensureAzdValid(mockContext, azdContext, envName, &projectConfig)
require.NoError(t, err)

nextFn, actionRan := createNextFn()
hookRan := setupHookMock(mockContext, 1) // Non-zero exit code

// Execute as child action (workflow step)
childCtx := WithChildAction(*mockContext.Context)
result, err := runMiddlewareWithContext(
childCtx, mockContext, envName, &projectConfig, &runOptions, nextFn,
)

require.Nil(t, result)
require.Error(t, err)
require.True(t, *hookRan, "Pre-hook should still execute for child actions")
require.False(t, *actionRan, "Action should NOT run when pre-hook fails")
}

func Test_PowerShellWarning_WithPwshAvailable(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
azdContext := createAzdContext(t)
Expand Down
Loading
Loading