feat(agents): provision terraform project connections#9112
Conversation
The Bicep eject path already creates connections at provision time; the Terraform eject path silently dropped the same `connections` param. Adds `connections.tf` (an `azapi_resource` for_each, mirroring `acr.tf`'s `acr_connection`) and forwards `connections` through to main.tfvars.json.
hund030
requested review from
JeffreyCA,
glharper,
huimiu,
therealjohn,
trangevi and
trrwilson
as code owners
microsoft-github-policy-service
Bot
added
the
customer-reported
|
Thank you for your contribution @hund030! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Pull request overview
Adds Terraform provisioning for azure.ai.connection services in Foundry projects.
Changes:
- Generates connection resources and Terraform variables.
- Exports provisioned connection names.
- Adds eject-path tests and template embedding coverage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
variables.tf |
Defines connection inputs. |
outputs.tf.tmpl |
Exports connection names. |
connections.tf |
Provisions Foundry connections. |
synthesizer_test.go |
Verifies template embedding. |
init_infra.go |
Writes connections to tfvars. |
init_infra_test.go |
Tests Terraform eject output. |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
A couple of notes on top of the existing comments.
No CHANGELOG entry. This extension logs every feature/fix under ## 1.0.0-beta.6 (Unreleased) with the PR number, but this PR doesn't add one. The sibling provision-time PR (#9046) is also absent from the changelog, so if the plan is to track the whole connections feature under a single entry later, ignore this. Otherwise, add a line noting that Terraform eject now synthesizes azure.ai.connection services.
On the already-flagged map(any) credentials comment - that's the one I'd prioritize. connections.bicep types credentials as object?, and the synthesized Go value is map[string]any that can hold nested objects (CustomKeys' keys:). Terraform's map(any) unifies to a single element type, so a project mixing an ApiKey connection ({key: "..."}) with a CustomKeys connection ({keys: {...}}) can fail type conversion or coerce the shape. That undercuts the "supports every auth type" intent. optional(any) keeps parity with the Bicep object? contract.
hund030
requested review from
RickWinter,
hemarina,
tg-msft and
vhvb1989
as code owners
| var value any | ||
| if err := json.Unmarshal(parameters, &value); err != nil { | ||
| return "", fmt.Errorf("parsing JSON parameters: %w", err) | ||
| } |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
My earlier concerns are resolved. Credentials now type as optional(any), so a project mixing an ApiKey connection with a CustomKeys connection round-trips its shape instead of hitting a map(any) type-unification error, and moving them into sensitive_body keeps the values out of terraform plan output. The switch from raw envsubst over the whole file to per-leaf foundry.ExpandEnv is the right call and the new tests cover the quote/newline/${{...}} cases that motivated it.
One thing worth a conscious call before this ships: the credential handling now depends on the reworked core substitution path, but the extension still sets requiredAzdVersion: ">=1.27.0", and those releases predate this change. Bumping it to the azd version that actually carries this substitution rework avoids an install on an older host silently falling back to the raw path for credentials containing quotes or newlines.
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
The revert drops the core Terraform provider substitution rework out of this PR, which clears the version-coupling concern from my last pass: the extension no longer depends on a substitution path that shipped after requiredAzdVersion: ">=1.27.0". Credentials now ride the same ${VAR} envsubst path azd already uses for deployments and the other tfvars values, and the optional(any) typing plus sensitive_body handling from the earlier commits are intact. Tests cover the mixed ApiKey/CustomKeys shapes and the CHANGELOG entry is in. Re-approving to clear the stale state from the new push.
Summary
Fixes #9111.
Terraform infrastructure eject currently omits
azure.ai.connectionservices declared inazure.yaml. This prevents Terraform-based Foundry projects from provisioning project connections alongside the account and project.Scope
azure.ai.connectionservices into Terraform variables.azapi_resourceper connection under the Foundry project.AZURE_AI_PROJECT_CONNECTION_NAMES.Acceptance Criteria
azure.ai.connectionservice is emitted toinfra/connections.tf.azd provisioncreates the Foundry project connection.Validation
go test ./internal/cmd -run TestEjectInfra_Terraform_EjectsConnectionServices -count=1