-
Notifications
You must be signed in to change notification settings - Fork 342
feat: add service-scoped env to Foundry extensions #9079
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
a4eb576
fix: use core env in Foundry service targets
huimiu afc5216
fix: document Foundry service env migration
huimiu cfc56b3
chore: start merge conflict resolution
Copilot b634164
fix: resolve merge conflicts in azure.ai.connections service target
Copilot f6940b1
fix: preserve Foundry service environment migration
huimiu 1371bb1
fix: sync Foundry environment targets with main
huimiu 362c651
fix: preserve Foundry environment templates
huimiu a41b0ab
fix: preserve Foundry env templates when persisting service env
huimiu d8c6f4e
fix: prioritize service env for local agent runs
huimiu 2618e57
fix: forward service env through Foundry deployment paths
huimiu 31278e7
feat: reconcile service-scoped env with unified azure.yaml
huimiu 2b33122
fix: restore Bicep modules before linting
huimiu bfa0dcd
fix: merge main into Foundry service env changes
huimiu d67f4a3
ci: drop unrelated Bicep restore change
huimiu 47b7c9a
fix: merge main before Foundry environment fixes
huimiu fea0f49
fix: preserve Foundry environment migration semantics
huimiu ce4d70a
fix: honor explicit empty service env as an isolated scope
huimiu 3ad246a
fix: use deploy-time service env in Foundry extensions
huimiu 9cb1676
Merge remote-tracking branch 'origin/main' into hui/update-foundry-ex…
huimiu 0f0ae20
fix: share one env var scanner across agents init paths
huimiu de05ddf
test: replace global env seam with injected project client
huimiu 4f00973
test: pin nested env var defaults as unsupported by the scanner
huimiu d190b5a
fix: explain which fields drive each env escaping constant
huimiu 58f7b7c
fix: require a brace after $ when scanning env references
huimiu 511ea28
test: cover bare-dollar shapes in the env scanner parity guard
huimiu a6bf02e
fix: surface failures reading the agent service env block
huimiu 74a2861
test: pin the legacy env var fallback as deliberate
huimiu dadc930
fix: warn when an agent service still sets config.env
huimiu 9b80c90
fix: parse env reference prefixes with regex
huimiu 593b294
fix: persist empty env for generated resources
huimiu c51bcd5
fix: validate and scope Foundry infrastructure config
huimiu d717f70
fix: merge main into Foundry environment changes
huimiu fc54476
fix: persist empty env for generated agent services
huimiu 88a6b76
fix: drop unpersistable empty env for generated services
huimiu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
233 changes: 233 additions & 0 deletions
233
cli/azd/extensions/azure.ai.agents/internal/cmd/env_refs.go
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,233 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "regexp" | ||
| "strings" | ||
|
|
||
| "github.com/azure/azure-dev/cli/azd/pkg/foundry" | ||
| ) | ||
|
|
||
| // Escape handling must match the expander that owns each field. | ||
| // Fields resolved by foundry.ExpandEnv take | ||
| // honorEnvironmentEscaping: it collapses '$' pairs, so $${VAR} | ||
| // stays literal, and it reserves ${{...}} spans for Foundry. | ||
| // The three project network fields (network.agentSubnet.vnet, | ||
| // network.peSubnet.vnet, network.dns.subscription) take | ||
| // ignoreEnvironmentEscaping because resolveVars in the projects | ||
| // synthesizer is a plain regex replace with no '$$' handling, | ||
| // so $${VAR} does expand there. The split mirrors that existing | ||
| // divergence rather than choosing two policies; it collapses | ||
| // once resolveVars moves to foundry.ExpandEnv. | ||
| // | ||
| // resolveVars also diverges on ':-'. Its pattern matches only | ||
| // ${NAME}, so a ${NAME:-default} on one of those three fields is | ||
| // never substituted and no error is raised; the literal then | ||
| // fails the field's own ARM id or subscription validation. No | ||
| // escaping flag can mirror that, so the scanner still reports | ||
| // the name and the gap is tracked upstream. | ||
| // See: https://github.com/Azure/azure-dev/issues/9350 | ||
| const ( | ||
| honorEnvironmentEscaping = true | ||
|
huimiu marked this conversation as resolved.
|
||
| ignoreEnvironmentEscaping = false | ||
| ) | ||
|
|
||
| // environmentReferencePrefix parses only the reference prefix. | ||
| // Balanced defaults remain the scanner's responsibility. | ||
| var environmentReferencePrefix = regexp.MustCompile( | ||
| `^\$\{([A-Za-z_][A-Za-z0-9_]*)(\}|:-)`, | ||
| ) | ||
|
|
||
| // environmentReference is one azd ${VAR} occurrence in a string. | ||
| // Start and End bound the whole reference, including any :- | ||
| // default, so a caller can resume scanning at End. | ||
| type environmentReference struct { | ||
| Name string | ||
| Start int | ||
| End int | ||
| HasDefault bool | ||
| } | ||
|
|
||
| // findEnvironmentReferences returns the azd ${VAR} references in | ||
| // value, in order of appearance. It is the single scanner for the | ||
| // package: callers layer their own policy on the result rather | ||
| // than reimplementing discovery. init prompting skips references | ||
| // with a default because the expander supplies the fallback, | ||
| // while the generated service env block records them so the | ||
| // owning extension can re-apply the default. | ||
| // | ||
| // References the expander would not resolve are dropped: escaped | ||
| // ones and any reserved by a Foundry ${{...}} span. honorEscaping | ||
| // must match the expander that owns the field. | ||
| // | ||
| // A reference inside a :- default is not reported: nested azd | ||
| // references are unsupported by design, so ${OUTER:-${NESTED}} | ||
| // yields OUTER only. foundry.ExpandEnv still resolves NESTED at | ||
| // deploy, but nothing discovers it, so init never prompts for it | ||
| // and it gets no entry in the generated service env block. It | ||
| // then resolves only where the consumer keeps an azd environment | ||
| // fallback, and to empty where a declared env: drops it. Keep | ||
| // defaults literal. | ||
| func findEnvironmentReferences(value string, honorEscaping bool) []environmentReference { | ||
| candidates := environmentReferenceCandidates(value, honorEscaping) | ||
| if !honorEscaping || len(candidates) == 0 { | ||
| return candidates | ||
| } | ||
|
|
||
| protected := protectedEnvironmentReferences(value, candidates) | ||
| references := make([]environmentReference, 0, len(candidates)) | ||
| for i, candidate := range candidates { | ||
| if protected[i] { | ||
| continue | ||
| } | ||
| references = append(references, candidate) | ||
| } | ||
| if len(references) == 0 { | ||
| return nil | ||
| } | ||
| return references | ||
| } | ||
|
|
||
| // environmentReferenceCandidates scans value left to right for | ||
| // ${NAME} and ${NAME:-default} occurrences. drone/envsubst, which | ||
| // backs foundry.ExpandEnv, collapses a '$' pair into a literal | ||
| // '$' and keeps reading, so an escape only neutralizes the '${' | ||
| // it precedes: the text after it, including a default, still | ||
| // holds live references. Membership of a ${{...}} span is left to | ||
| // findEnvironmentReferences. Scanning resumes at the end of a | ||
| // match, so a default span is never scanned again; that is what | ||
| // keeps nested references out. | ||
| func environmentReferenceCandidates(value string, honorEscaping bool) []environmentReference { | ||
| var references []environmentReference | ||
| for index := 0; index < len(value); { | ||
| if value[index] != '$' { | ||
| index++ | ||
| continue | ||
| } | ||
| if honorEscaping && strings.HasPrefix(value[index:], "$$") { | ||
| index += 2 | ||
| continue | ||
| } | ||
|
|
||
| reference, found := environmentReferenceAt(value, index) | ||
| if !found { | ||
| index++ | ||
| continue | ||
| } | ||
|
|
||
| references = append(references, reference) | ||
| index = reference.End | ||
|
huimiu marked this conversation as resolved.
|
||
| } | ||
| return references | ||
| } | ||
|
|
||
| // environmentReferenceAt parses the reference opening at start. | ||
| // The anchored prefix keeps a bare '$' from being read as one. | ||
| // Balanced defaults still need the stateful end scanner below. | ||
| func environmentReferenceAt(value string, start int) (environmentReference, bool) { | ||
| if start < 0 || start >= len(value) { | ||
| return environmentReference{}, false | ||
| } | ||
|
|
||
| match := environmentReferencePrefix.FindStringSubmatch(value[start:]) | ||
| if match == nil { | ||
| return environmentReference{}, false | ||
| } | ||
|
|
||
| name := match[1] | ||
| prefixEnd := start + len(match[0]) | ||
| if match[2] == "}" { | ||
| return environmentReference{ | ||
| Name: name, | ||
| Start: start, | ||
| End: prefixEnd, | ||
| }, true | ||
| } | ||
|
|
||
| end, found := environmentReferenceEnd(value, prefixEnd) | ||
| if !found { | ||
| return environmentReference{}, false | ||
| } | ||
| return environmentReference{ | ||
| Name: name, | ||
| Start: start, | ||
| End: end, | ||
| HasDefault: true, | ||
| }, true | ||
| } | ||
|
|
||
| // environmentReferenceEnd finds the '}' closing a :- default. It | ||
| // counts nested ${...} and steps over Foundry ${{...}} spans, | ||
| // which are legal default values, so the reported span covers the | ||
| // whole reference. | ||
| func environmentReferenceEnd(value string, index int) (int, bool) { | ||
| depth := 1 | ||
| for index < len(value) { | ||
| if strings.HasPrefix(value[index:], "${{") { | ||
| end := strings.Index(value[index+3:], "}}") | ||
| if end < 0 { | ||
| return 0, false | ||
| } | ||
| index += end + 5 | ||
| continue | ||
| } | ||
| if strings.HasPrefix(value[index:], "${") { | ||
| depth++ | ||
| index += 2 | ||
| continue | ||
| } | ||
| if value[index] == '}' { | ||
| depth-- | ||
| index++ | ||
| if depth == 0 { | ||
| return index, true | ||
| } | ||
| continue | ||
| } | ||
| index++ | ||
| } | ||
| return 0, false | ||
| } | ||
|
|
||
| // protectedEnvironmentReferences reports which candidates sit | ||
| // inside a server-side ${{...}} span. Each candidate is replaced | ||
| // with a unique probe before running [foundry.ExpandEnv]; probes | ||
| // left verbatim are reserved by the shared expander. This keeps | ||
| // discovery linked to the owning implementation without ambiguous | ||
| // name-based occurrence counting. | ||
| func protectedEnvironmentReferences(value string, references []environmentReference) []bool { | ||
| protected := make([]bool, len(references)) | ||
| if len(references) == 0 { | ||
| return protected | ||
| } | ||
|
|
||
| probePrefix := "AZD_ENV_REFERENCE_PROBE_" | ||
| for strings.Contains(value, probePrefix) { | ||
| probePrefix += "_" | ||
| } | ||
|
|
||
| probeRefs := make([]string, len(references)) | ||
| var probed strings.Builder | ||
| last := 0 | ||
| for i, reference := range references { | ||
| probed.WriteString(value[last:reference.Start]) | ||
| probeRefs[i] = fmt.Sprintf("${%s%d}", probePrefix, i) | ||
| probed.WriteString(probeRefs[i]) | ||
| last = reference.End | ||
| } | ||
| probed.WriteString(value[last:]) | ||
|
|
||
| expanded, err := foundry.ExpandEnv(probed.String(), func(name string) string { | ||
| return "expanded_" + name | ||
| }) | ||
| if err != nil { | ||
| return protected | ||
| } | ||
| for i, probeRef := range probeRefs { | ||
| protected[i] = strings.Contains(expanded, probeRef) | ||
| } | ||
| return protected | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.