Skip to content

Commit e8a1d72

Browse files
weikanglimCopilot
andcommitted
Fix hook validation cwd handling and provider validation errors
Validate hooks using scope-specific working directories so relative file hooks are resolved correctly for services and layers, remove the redundant layer hook conversion during provision, and clean up provisioning validation error wrapping so root and layer errors are wrapped consistently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fdd9404 commit e8a1d72

7 files changed

Lines changed: 208 additions & 83 deletions

File tree

cli/azd/cmd/hooks.go

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -351,44 +351,43 @@ func (hra *hooksRunAction) execHook(
351351

352352
// Validates hooks and displays warnings for default shell usage and other issues
353353
func (hra *hooksRunAction) validateAndWarnHooks(ctx context.Context) error {
354-
// Collect all hooks from project and services
355-
allHooks := make(map[string][]*ext.HookConfig)
354+
warningKeys := map[string]struct{}{}
355+
validateAndWarn := func(cwd string, hooks map[string][]*ext.HookConfig) {
356+
if len(hooks) == 0 {
357+
return
358+
}
359+
360+
hooksManager := ext.NewHooksManager(cwd, hra.commandRunner)
361+
validationResult := hooksManager.ValidateHooks(ctx, hooks)
362+
363+
for _, warning := range validationResult.Warnings {
364+
key := warning.Message + "\x00" + warning.Suggestion
365+
if _, has := warningKeys[key]; has {
366+
continue
367+
}
356368

357-
// Add project hooks
358-
for hookName, hookConfigs := range hra.projectConfig.Hooks {
359-
allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
369+
warningKeys[key] = struct{}{}
370+
hra.console.MessageUxItem(ctx, &ux.WarningMessage{
371+
Description: warning.Message,
372+
})
373+
if warning.Suggestion != "" {
374+
hra.console.Message(ctx, warning.Suggestion)
375+
}
376+
hra.console.Message(ctx, "")
377+
}
360378
}
361379

362-
// Add service hooks
380+
validateAndWarn(hra.projectConfig.Path, hra.projectConfig.Hooks)
381+
363382
stableServices, err := hra.importManager.ServiceStable(ctx, hra.projectConfig)
364383
if err == nil {
365384
for _, service := range stableServices {
366-
for hookName, hookConfigs := range service.Hooks {
367-
allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
368-
}
385+
validateAndWarn(service.Path(), service.Hooks)
369386
}
370387
}
371388

372-
// Add layer hooks
373389
for _, layer := range hra.projectConfig.Infra.Layers {
374-
for hookName, hookConfigs := range layer.Hooks {
375-
allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
376-
}
377-
}
378-
379-
// Create hooks manager and validate
380-
hooksManager := ext.NewHooksManager(hra.projectConfig.Path, hra.commandRunner)
381-
validationResult := hooksManager.ValidateHooks(ctx, allHooks)
382-
383-
// Display any warnings
384-
for _, warning := range validationResult.Warnings {
385-
hra.console.MessageUxItem(ctx, &ux.WarningMessage{
386-
Description: warning.Message,
387-
})
388-
if warning.Suggestion != "" {
389-
hra.console.Message(ctx, warning.Suggestion)
390-
}
391-
hra.console.Message(ctx, "")
390+
validateAndWarn(layer.AbsolutePath(hra.projectConfig.Path), layer.Hooks)
392391
}
393392

394393
return nil

cli/azd/cmd/hooks_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package cmd
55

66
import (
77
"context"
8+
"os"
89
"path/filepath"
910
"testing"
1011

@@ -227,3 +228,49 @@ func Test_HooksRunAction_RejectsServiceAndLayerTogether(t *testing.T) {
227228
require.Error(t, err)
228229
require.ErrorContains(t, err, "--service and --layer cannot be used together")
229230
}
231+
232+
func Test_HooksRunAction_ValidatesLayerHooksRelativeToLayerPath(t *testing.T) {
233+
mockContext := mocks.NewMockContext(context.Background())
234+
env := environment.NewWithValues("test", nil)
235+
236+
projectPath := t.TempDir()
237+
layerScriptPath := filepath.Join(projectPath, "infra", "core", "scripts", "preprovision.sh")
238+
require.NoError(t, os.MkdirAll(filepath.Dir(layerScriptPath), 0o755))
239+
require.NoError(t, os.WriteFile(layerScriptPath, []byte("echo pre"), 0o600))
240+
241+
layerHook := &ext.HookConfig{
242+
Run: filepath.Join("scripts", "preprovision.sh"),
243+
}
244+
245+
projectConfig := &project.ProjectConfig{
246+
Name: "test",
247+
Path: projectPath,
248+
Services: map[string]*project.ServiceConfig{},
249+
Infra: provisioning.Options{
250+
Layers: []provisioning.Options{
251+
{
252+
Name: "core",
253+
Path: filepath.Join("infra", "core"),
254+
Hooks: provisioning.HooksConfig{
255+
"preprovision": {layerHook},
256+
},
257+
},
258+
},
259+
},
260+
}
261+
262+
action := &hooksRunAction{
263+
projectConfig: projectConfig,
264+
env: env,
265+
importManager: project.NewImportManager(nil),
266+
commandRunner: mockContext.CommandRunner,
267+
console: mockContext.Console,
268+
flags: &hooksRunFlags{},
269+
serviceLocator: mockContext.Container,
270+
}
271+
272+
err := action.validateAndWarnHooks(*mockContext.Context)
273+
require.NoError(t, err)
274+
require.False(t, layerHook.IsUsingDefaultShell())
275+
require.Equal(t, ext.ScriptTypeUnknown, layerHook.Shell)
276+
}

cli/azd/cmd/middleware/hooks.go

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -181,45 +181,41 @@ func (m *HooksMiddleware) createServiceEventHandler(
181181

182182
// validateHooks validates hook configurations and displays any warnings
183183
func (m *HooksMiddleware) validateHooks(ctx context.Context, projectConfig *project.ProjectConfig) error {
184-
// Get service hooks for validation
185-
var serviceHooks []map[string][]*ext.HookConfig
186-
stableServices, err := m.importManager.ServiceStable(ctx, projectConfig)
187-
if err != nil {
188-
return fmt.Errorf("failed getting services for hook validation: %w", err)
189-
}
184+
warningKeys := map[string]struct{}{}
185+
validateAndWarn := func(cwd string, hooks map[string][]*ext.HookConfig) {
186+
if len(hooks) == 0 {
187+
return
188+
}
190189

191-
for _, service := range stableServices {
192-
serviceHooks = append(serviceHooks, service.Hooks)
193-
}
190+
hooksManager := ext.NewHooksManager(cwd, m.commandRunner)
191+
validationResult := hooksManager.ValidateHooks(ctx, hooks)
194192

195-
// Combine project and service hooks into a single map
196-
allHooks := make(map[string][]*ext.HookConfig)
193+
for _, warning := range validationResult.Warnings {
194+
key := warning.Message + "\x00" + warning.Suggestion
195+
if _, has := warningKeys[key]; has {
196+
continue
197+
}
197198

198-
// Add project hooks
199-
for hookName, hookConfigs := range projectConfig.Hooks {
200-
allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
199+
warningKeys[key] = struct{}{}
200+
m.console.MessageUxItem(ctx, &ux.WarningMessage{
201+
Description: warning.Message,
202+
})
203+
if warning.Suggestion != "" {
204+
m.console.Message(ctx, warning.Suggestion)
205+
}
206+
m.console.Message(ctx, "")
207+
}
201208
}
202209

203-
// Add service hooks
204-
for _, serviceHookMap := range serviceHooks {
205-
for hookName, hookConfigs := range serviceHookMap {
206-
allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
207-
}
210+
validateAndWarn(projectConfig.Path, projectConfig.Hooks)
211+
212+
stableServices, err := m.importManager.ServiceStable(ctx, projectConfig)
213+
if err != nil {
214+
return fmt.Errorf("failed getting services for hook validation: %w", err)
208215
}
209216

210-
// Create hooks manager and validate
211-
hooksManager := ext.NewHooksManager(projectConfig.Path, m.commandRunner)
212-
validationResult := hooksManager.ValidateHooks(ctx, allHooks)
213-
214-
// Display any warnings
215-
for _, warning := range validationResult.Warnings {
216-
m.console.MessageUxItem(ctx, &ux.WarningMessage{
217-
Description: warning.Message,
218-
})
219-
if warning.Suggestion != "" {
220-
m.console.Message(ctx, warning.Suggestion)
221-
}
222-
m.console.Message(ctx, "")
217+
for _, service := range stableServices {
218+
validateAndWarn(service.Path(), service.Hooks)
223219
}
224220

225221
return nil

cli/azd/cmd/middleware/hooks_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ package middleware
66
import (
77
"context"
88
"errors"
9+
"os"
910
osexec "os/exec"
11+
"path/filepath"
12+
"runtime"
1013
"strings"
1114
"testing"
1215

@@ -288,6 +291,81 @@ func Test_ServiceHooks_Registered(t *testing.T) {
288291
require.Equal(t, 1, preDeployCount)
289292
}
290293

294+
func Test_ServiceHooks_ValidationUsesServicePath(t *testing.T) {
295+
mockContext := mocks.NewMockContext(context.Background())
296+
azdContext := createAzdContext(t)
297+
298+
envName := "test"
299+
runOptions := Options{CommandPath: "deploy"}
300+
301+
projectConfig := project.ProjectConfig{
302+
Name: envName,
303+
Services: map[string]*project.ServiceConfig{},
304+
}
305+
306+
hookPath := filepath.Join("scripts", "predeploy.ps1")
307+
expectedShell := "pwsh"
308+
scriptContents := "Write-Host 'Hello'\n"
309+
if runtime.GOOS == "windows" {
310+
hookPath = filepath.Join("scripts", "predeploy.sh")
311+
expectedShell = "bash"
312+
scriptContents = "echo hello\n"
313+
}
314+
315+
serviceConfig := &project.ServiceConfig{
316+
EventDispatcher: ext.NewEventDispatcher[project.ServiceLifecycleEventArgs](project.ServiceEvents...),
317+
Language: "ts",
318+
RelativePath: "./src/api",
319+
Host: "appservice",
320+
Hooks: map[string][]*ext.HookConfig{
321+
"predeploy": {
322+
{
323+
Run: hookPath,
324+
},
325+
},
326+
},
327+
}
328+
329+
projectConfig.Services["api"] = serviceConfig
330+
331+
err := ensureAzdValid(mockContext, azdContext, envName, &projectConfig)
332+
require.NoError(t, err)
333+
334+
projectConfig.Services["api"].Project = &projectConfig
335+
336+
serviceHookPath := filepath.Join(serviceConfig.Path(), hookPath)
337+
require.NoError(t, os.MkdirAll(filepath.Dir(serviceHookPath), 0o755))
338+
require.NoError(t, os.WriteFile(serviceHookPath, []byte(scriptContents), 0o600))
339+
340+
mockContext.CommandRunner.MockToolInPath("pwsh", nil)
341+
342+
var executedShell string
343+
mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool {
344+
return true
345+
}).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) {
346+
executedShell = args.Cmd
347+
return exec.NewRunResult(0, "", ""), nil
348+
})
349+
350+
nextFn := func(ctx context.Context) (*actions.ActionResult, error) {
351+
err := serviceConfig.Invoke(ctx, project.ServiceEventDeploy, project.ServiceLifecycleEventArgs{
352+
Project: &projectConfig,
353+
Service: serviceConfig,
354+
ServiceContext: project.NewServiceContext(),
355+
}, func() error {
356+
return nil
357+
})
358+
359+
return &actions.ActionResult{}, err
360+
}
361+
362+
result, err := runMiddleware(mockContext, envName, &projectConfig, &runOptions, nextFn)
363+
364+
require.NotNil(t, result)
365+
require.NoError(t, err)
366+
require.Equal(t, expectedShell, executedShell)
367+
}
368+
291369
func createAzdContext(t *testing.T) *azdcontext.AzdContext {
292370
tempDir := t.TempDir()
293371
ostest.Chdir(t, tempDir)

cli/azd/internal/cmd/provision.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,7 @@ func (p *ProvisionAction) runLayerProvisionWithHooks(
531531
layerPath string,
532532
actionFn ext.InvokeFn,
533533
) error {
534-
hooks := map[string][]*ext.HookConfig(layer.Hooks)
535-
if len(hooks) == 0 {
534+
if len(layer.Hooks) == 0 {
536535
return actionFn()
537536
}
538537

@@ -543,12 +542,12 @@ func (p *ProvisionAction) runLayerProvisionWithHooks(
543542
p.envManager,
544543
p.console,
545544
layerPath,
546-
hooks,
545+
layer.Hooks,
547546
p.env,
548547
p.serviceLocator,
549548
)
550549

551-
p.validateAndWarnLayerHooks(ctx, hooksManager, hooks)
550+
p.validateAndWarnLayerHooks(ctx, hooksManager, layer.Hooks)
552551

553552
if err := hooksRunner.Invoke(ctx, []string{string(project.ProjectEventProvision)}, actionFn); err != nil {
554553
if layer.Name == "" {

0 commit comments

Comments
 (0)