feat: forward service env to extensions#8936
Merged
Merged
Conversation
huimiu
marked this pull request as ready for review
July 7, 2026 06:35
huimiu
requested review from
JeffreyCA,
RickWinter,
hemarina,
tg-msft and
vhvb1989
as code owners
July 7, 2026 06:35
Contributor
There was a problem hiding this comment.
Pull request overview
This PR forwards service-level env values declared in azure.yaml to extension-provided framework and service-target providers through the gRPC extension contract. It adds an Environment field to the azdext.ServiceConfig proto message, expands the service env map with azd's environment resolver during the forward mapping, and preserves the field on the reverse mapping back into core ServiceConfig. To resolve those values, ExternalFrameworkService and its gRPC registration factory are wired with a *lazy.Lazy[*environment.Environment], replacing the previous empty resolver.
Changes:
- Add
map<string, string> environment = 14toServiceConfig(proto + regeneratedmodels.pb.go) and map it in both directions. - Thread an
*environment.EnvironmentintoExternalFrameworkService(via a lazy env in the gRPCFrameworkService) so serviceenvis expanded before forwarding. - Add tests for mapping round-trips, env expansion, and the new registration wiring, plus a docs note.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
cli/azd/grpc/proto/models.proto |
Adds the environment map field to ServiceConfig. |
cli/azd/pkg/azdext/models.pb.go |
Regenerated proto code for the new field and EnvironmentEntry map type. |
cli/azd/pkg/project/mapper_registry.go |
Expands and maps service env in the forward and reverse ServiceConfig conversions. |
cli/azd/pkg/project/framework_service_external.go |
Adds env field; routes Build/Package through toProtoServiceConfig using the real env resolver. |
cli/azd/internal/grpcserver/framework_service.go |
Injects lazyEnv and resolves the environment in the provider factory. |
cli/azd/pkg/project/*_test.go, cli/azd/internal/grpcserver/framework_service_test.go |
Tests for env mapping, expansion, and registration behavior. |
cli/azd/docs/extensions/extension-framework.md |
Documents that service env is forwarded as ServiceConfig.environment. |
One thing worth resolving before merge: the gRPC registration factory now propagates the environment-load error, which can fail restore/build/package for extension-provided framework services when no environment is selected — inconsistent with the sibling ServiceTargetService registration and the prior empty-resolver behavior.
Files not reviewed (1)
- cli/azd/pkg/azdext/models.pb.go: Generated file
jongio
reviewed
Jul 7, 2026
Member
There was a problem hiding this comment.
The proto extension, mapper logic, and test coverage here are solid. One issue to address before merge:
The factory registration in onRegisterRequest propagates lazyEnv.GetValue() errors, which will fail restore/build/package for extension framework services when no environment is selected. The sibling ServiceTargetService (service_target_service.go:125) ignores this error with env, _ := s.lazyEnv.GetValue(), and envResolver(nil) already degrades gracefully (returns "" for every key). This should match that pattern to avoid a behavioral regression.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
jongio
approved these changes
Jul 7, 2026
jongio
left a comment
jongio
left a comment
Member
There was a problem hiding this comment.
Prior concern addressed. The lazyEnv error is now correctly ignored (matching ServiceTargetService pattern), with nil guard and test coverage for both the error and happy paths.
…9018) Correctness: - Propagate the caller's resolver into nested ServiceConfig conversions in the ProjectConfig -> proto mapping so project-level payloads (ProjectService.Get, event Project payloads) expand service env values instead of blanking them; wrap nested conversion errors with the service name so one malformed service is diagnosable. - Escape '$' and '\' when reverse-mapping extension-supplied env values (new osutil.NewLiteralExpandableString) so literal values survive later envsubst expansion instead of being reinterpreted as templates. - Preserve unchanged ${VAR} env templates in azure.yaml when a service config round-trips through ProjectService.AddService, so read-modify-write extension flows no longer bake expanded values into the persisted file. - Resolve the environment lazily per conversion in external framework and service target providers (shared serviceConfigToProto helper) instead of freezing a possibly-nil env at first registration; log env-load failures instead of silently expanding everything to empty strings. - Use the session environment (honoring -e/--environment) in ProjectService.Get and GetResolvedServices instead of the persisted default environment, matching the framework/service-target/event paths. - Log the previously swallowed conversion error in RequiredExternalTools. - Add the environment field to the extension scaffolding models.proto so scaffolded extensions can consume it. Cleanup: - Remove the now-unused lazyEnvManager dependency from projectService. - Consolidate the duplicated framework service registration tests into one table-driven test; env expansion behavior is asserted in pkg/project. - Use Expand with error assertions and $-containing fixtures in mapper tests so escaping regressions fail visibly. - Document expansion and write-back semantics in the extension framework docs.
github-actions
Bot
added
the
ext-x
jongio
approved these changes
Jul 8, 2026
jongio
left a comment
jongio
left a comment
Member
There was a problem hiding this comment.
Incremental review of 5f0132b (post-approval commit). The lazy env refactoring is clean: removes the unnecessary envManager indirection from projectService, adds round-trip safety for env templates in AddService, and properly escapes literal values in the reverse proto mapping. Test coverage is thorough. No issues.
JeffreyCA
approved these changes
Jul 8, 2026
jongio
approved these changes
Jul 8, 2026
jongio
left a comment
jongio
left a comment
Member
There was a problem hiding this comment.
Reviewed the proto extension, mapper logic (forward + reverse), round-trip template preservation, literal escaping, and shared serviceConfigToProto helper. The graceful env-load-error handling matches the sibling ServiceTargetService pattern correctly. Test coverage is thorough (expansion, error cases, per-call lazy resolution, dollar/backslash escaping). No issues found.
Contributor
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
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
ServiceConfig.environmentto the extension gRPC contract.azure.yamlenvvalues with the existing azd environment resolver before forwarding them to extension providers.Why
External framework and service target extensions need access to service-specific environment configuration declared in
azure.yaml. Forwarding this data through the extension contract lets extension authors consume the same service-levelenvvalues that core azd can already resolve for built-in service flows.Impact
azdext.ServiceConfig.Environment.envis not configured.Tests
go test ./pkg/project -run "TestServiceConfigMapping|TestServiceConfigReverseMapping|TestServiceConfigRoundTripMapping|TestFromProtoServiceConfigMapping"