fix: return error instead of panic for empty service in azure.yaml#7340
Closed
spboyer wants to merge 2 commits into
Closed
fix: return error instead of panic for empty service in azure.yaml#7340spboyer wants to merge 2 commits into
spboyer wants to merge 2 commits into
Conversation
When a service is defined in azure.yaml without any properties (e.g., `web:` with nothing below it), the YAML unmarshaller creates a nil *ServiceConfig entry. Previously this caused a nil pointer panic in Parse(). Now returns a clear error message. Fixes #7254 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
spboyer
requested review from
JeffreyCA,
hemarina,
vhvb1989,
wbreza and
weikanglim
as code owners
March 26, 2026 14:42
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a nil-pointer panic when parsing azure.yaml where a service key is present but has no nested properties (e.g., web:), by validating nil service configs and returning a user-facing error instead.
Changes:
- Add a nil guard in
project.Parse()to return an actionable error when a service config is empty (nil) after YAML unmarshal. - Add nil guards in
ImportManagerservice-iteration code paths to avoid dereferencing nil service configs. - Add unit test coverage to ensure empty service definitions return an error (and include the service name/message text).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
cli/azd/pkg/project/project.go |
Adds nil validation for Services entries during Parse() to prevent panic and return a clear error. |
cli/azd/pkg/project/importer.go |
Adds defensive nil checks while iterating projectConfig.Services across importer flows. |
cli/azd/pkg/project/project_config_test.go |
Adds tests asserting empty service definitions produce an error (not a panic) and include helpful message content. |
Comments suppressed due to low confidence (3)
cli/azd/pkg/project/importer.go:65
ServiceStablenow skips nil service entries but still useslen(projectConfig.Services)to enforce the “single service with Aspire AppHost” constraint. If the map contains nil values,len(projectConfig.Services)will count them and can incorrectly returnerrNoMultipleServicesWithAppHosteven when there is only one non-nil service. Consider computing the count of non-nil services (or validating and returning an explicit error on nil entries) before this check.
This issue also appears in the following locations of the same file:
- line 336
- line 435
for name, svcConfig := range projectConfig.Services {
if svcConfig == nil {
continue
}
if svcConfig.Language == ServiceLanguageDotNet {
if canImport, err := im.dotNetImporter.CanImport(ctx, svcConfig.Path()); canImport {
if len(projectConfig.Services) != 1 {
return nil, errNoMultipleServicesWithAppHost
}
cli/azd/pkg/project/importer.go:345
ProjectInfrastructureskips nil service entries, but the laterlen(projectConfig.Services) != 1check will still count nil entries and may reject otherwise-valid inputs. Consider iteratingfor name, svcConfig := range projectConfig.Servicesand either (a) returning a clear error for nil service configs or (b) basing the “single service” constraint on a non-nil service count.
// Temp infra from AppHost
for _, svcConfig := range projectConfig.Services {
if svcConfig == nil {
continue
}
if svcConfig.Language == ServiceLanguageDotNet {
if canImport, err := im.dotNetImporter.CanImport(ctx, svcConfig.Path()); canImport {
if len(projectConfig.Services) != 1 {
return nil, errNoMultipleServicesWithAppHost
}
cli/azd/pkg/project/importer.go:444
GenerateAllInfrastructureskips nil service entries, but still checkslen(projectConfig.Services) != 1, which counts nil values and can cause false positives forerrNoMultipleServicesWithAppHost. Consider validating nil entries up front (returning an error) or computing a non-nil service count for this condition.
func (im *ImportManager) GenerateAllInfrastructure(ctx context.Context, projectConfig *ProjectConfig) (fs.FS, error) {
for _, svcConfig := range projectConfig.Services {
if svcConfig == nil {
continue
}
if svcConfig.Language == ServiceLanguageDotNet {
if canImport, err := im.dotNetImporter.CanImport(ctx, svcConfig.Path()); canImport {
if len(projectConfig.Services) != 1 {
return nil, errNoMultipleServicesWithAppHost
}
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Member
Author
|
PR #7343 is the stronger fix. It's more comprehensive (covers Resources + Hooks), architecturally sounder (single validation gate vs. scattered guards), has better UX (multi-error, sorted, actionable), and avoids the semantic len() bug. #7340 is a reasonable first response but leaves gaps that Copilot itself flagged. |
spboyer
commented
Mar 27, 2026
spboyer
left a comment
spboyer
left a comment
Member
Author
There was a problem hiding this comment.
Addressed both review comments:
- Variable rename: Renamed
yamltoprojectYamlin the test to avoid shadowing the imported package. - Resources nil check: Added the same nil validation for
Resourcesmap entries inParse()— emptyresources.<name>:YAML nodes now return a clear error instead of panicking.
Pushed as squashed single commit.
spboyer
commented
Mar 27, 2026
spboyer
left a comment
spboyer
left a comment
Member
Author
There was a problem hiding this comment.
Both review comments were addressed in the latest push:
- ✅ Variable rename:
yaml→projectYamlin test to avoid shadowing the imported package. - ✅ Resources nil check: Added the same nil validation for
Resourcesmap entries — emptyresources.<name>:YAML nodes now return a clear error.
Please resolve the threads.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #7254
When a service is defined in
azure.yamlwithout any properties (e.g.,web:with nothing below it), the YAML unmarshaller creates a nil*ServiceConfigentry in theServicesmap. Previously this caused a nil pointer panic inParse()atproject.go:95. Now returns a clear, actionable error message.Changes
project.go— Added nil validation inParse()before the service processing loop. Returns error:"service 'web' has empty configuration, must specify at least 'host' and either 'language' or 'image'"importer.go— Added defensive nil guards in all 4 service iteration sites (ServiceStable,HasAppHost,GetInfrastructure,GenerateAllInfrastructure)project_config_test.go— Added test case verifying empty service returns error (not panic) with service name in messageTesting
go test ./pkg/project/... -run TestProjectConfig -count=1All existing tests pass. New test validates the fix.