fix: replace spaces with underscores in service name env var keys#7723
Open
fix: replace spaces with underscores in service name env var keys#7723
Conversation
) Service names with spaces in azure.yaml (e.g., "api and frontend") produced invalid environment variable names like SERVICE_API AND FRONTEND_IMAGE_NAME. The Key() function only replaced hyphens with underscores but not spaces. - Update environment.Key() to replace spaces with underscores - Centralize slotEnvVarNameForService to use environment.Key() instead of duplicating normalization logic - Add tests for Key(), ServiceProperty round-trip with spaces, and slot env var names with spaces Fixes #4996 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
spboyer
requested review from
JeffreyCA,
hemarina,
rajeshkamal5050,
tg-msft,
vhvb1989,
wbreza and
weikanglim
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes invalid environment variable names generated from service names containing spaces by normalizing keys consistently across the environment and App Service slot env var naming.
Changes:
- Update
environment.Key()to replace spaces and hyphens with underscores. - Centralize App Service slot env var naming to use
environment.Key()(removes duplicated normalization logic). - Add/extend unit tests to cover space-containing service names and round-trip behavior.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/environment/environment.go | Normalizes keys by replacing spaces and hyphens with underscores before use in env var names. |
| cli/azd/pkg/project/service_target_appservice.go | Uses shared environment.Key() for slot env var key generation to ensure consistent normalization. |
| cli/azd/pkg/environment/environment_test.go | Adds tests for Key() behavior and verifies service property env var round-trip when service names contain spaces. |
| cli/azd/pkg/project/service_targets_coverage3_test.go | Adds slot env var name test cases for service names containing spaces (and spaces + hyphens). |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
jongio
approved these changes
Apr 14, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Clean fix for the reported issue. Key() now normalizes spaces alongside hyphens, and slotEnvVarNameForService delegates to Key() instead of duplicating the logic. Tests cover single spaces, mixed spaces+hyphens, and the round-trip via SetServiceProperty/GetServiceProperty.
FYI: there are several other spots that duplicate the same strings.ReplaceAll(x, "-", "_") normalization without going through Key() - scaffold/spec.go:232, scaffold/funcs.go:218, apphost/generate.go (multiple), dotnet_importer.go:567, and the AI agents extension. They'd have the same gap if space-containing names reach them. Not blocking here, but worth a follow-up issue to centralize.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
weikanglim
requested changes
Apr 14, 2026
Contributor
weikanglim
left a comment
weikanglim
left a comment
There was a problem hiding this comment.
Hi spboyer — on the original issue #4996: my understanding was that we preferred rejecting invalid service/project names upfront, since fixing env var names alone doesn’t prevent downstream issues (e.g., invalid image tags in Docker).
Given users will likely hit those errors anyway, shouldn’t we validate early here? If we’re not taking that approach, I’d like to better understand the rationale behind that decision.
wbreza
approved these changes
Apr 14, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — replace spaces with underscores in service name env var keys
Clean, well-scoped fix for #4996. The Key() change is correct, the slotEnvVarNameForService refactoring is a good DRY improvement, and test coverage is thorough (7 unit cases + round-trip + coverage3).
Findings
No new significant issues beyond what jongio, weikanglim, and Copilot already flagged.
On weikanglim's concern (input validation): Valid long-term concern — spaces in service names will cause issues beyond env vars (Docker tags, resource names, etc.). But this PR is correctly scoped to fix #4996 (the env var symptom). A follow-up issue for upfront service name validation would be the right way to address the broader problem without blocking this fix.
On jongio's centralization note: Agreed — scaffold/spec.go, scaffold/funcs.go, �pphost/generate.go, and dotnet_importer.go all duplicate normalization logic. A follow-up to centralize through Key() would be valuable.
✅ What looks good
- Correct fix — spaces now normalized alongside hyphens
- DRY refactoring of slotEnvVarNameForService to delegate to Key()
- Solid test coverage: edge cases (multiple spaces, mixed spaces+hyphens, empty string)
- Round-trip test verifying no space-containing keys leak into the environment
LGTM — approve contingent on author confirming a follow-up issue for broader service name validation (weikanglim's concern) and centralization (jongio's note). 👍
- Use strings.Map + unicode.IsSpace to normalize all whitespace variants (tabs, newlines, non-breaking spaces) not just ASCII spaces - Update doc comment to accurately describe behavior: whitespace and hyphens are replaced, other characters pass through unchanged - Add test cases for tab and newline whitespace Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Review feedback addressed in bea9c96:
All threads resolved. @jongio — ready for another look! |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
|
jongio
approved these changes
Apr 15, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Latest commit addresses Copilot's unicode whitespace feedback - strings.Map + unicode.IsSpace is the right approach. Re-approving; my earlier notes on centralizing the duplicate normalization sites still apply as a follow-up.
jongio
approved these changes
Apr 15, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Latest commit addresses Copilot's unicode whitespace feedback - strings.Map + unicode.IsSpace is the right approach. Re-approving; my earlier notes on centralizing the duplicate normalization sites still apply as a follow-up.
Member
Author
|
@weikanglim — Could you take a look at the comments from @jongio and @wbreza above and kindly sign off - |
rajeshkamal5050
approved these changes
Apr 16, 2026
Member
|
@weikanglim - the code changes here look solid. The Your point about validating service names with spaces upfront is a good one - but I think it's orthogonal to this fix. Even with upstream validation, Can you take another look and either approve or let us know if there's something specific you'd want changed in this PR? |
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 #4996 — Service names with spaces in
azure.yaml(e.g.,"api and frontend") produced invalid environment variable names likeSERVICE_API AND FRONTEND_IMAGE_NAME.Root Cause
environment.Key()only replaced hyphens (-) with underscores but did not handle spaces. TheslotEnvVarNameForService()function in appservice.go duplicated this same logic with the same gap.Changes
pkg/environment/environment.goKey()now replaces both spaces and hyphens with underscorespkg/project/service_target_appservice.goslotEnvVarNameForService()centralized to useenvironment.Key()pkg/environment/environment_test.goTestKey(7 cases) +TestServicePropertyRoundTripWithSpacespkg/project/service_targets_coverage3_test.goTelemetry
Queried Kusto (
ddazureclients/DevCLI) — found 2"Resource has invalid name"errors in the last 180 days related to this pattern. Low frequency since most templates avoid spaces, but when it hits the error is confusing.Before/After