fix: switch to foundry backend urls - #9334
Conversation
🔗 Linked Issue RequiredThanks for the contribution! Please link a GitHub issue to this PR by adding |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Switches the RLE extension to authenticated, project-relative Foundry APIs.
Changes:
- Adds Foundry authentication, API versioning, and new environment/sandbox routes.
- Adds disk-image conversion polling and updates invocation behavior.
- Bumps the preview version and refreshes documentation.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
version.txt |
Bumps version to 0.2.0-preview. |
README.md |
Documents Foundry setup and workflows. |
internal/project/docker.go |
Clarifies ACR login guidance. |
internal/cmd/invoke.go |
Updates sandbox lifecycle and polling. |
internal/cmd/invoke_test.go |
Tests new invocation routes. |
internal/cmd/deploy.go |
Deploys through the Foundry endpoint. |
internal/cmd/client.go |
Adds authenticated Foundry API client. |
internal/cmd/client_test.go |
Tests authentication and endpoints. |
go.mod |
Promotes Azure identity dependencies. |
extension.yaml |
Updates extension version. |
CHANGELOG.md |
Adds 0.2.0-preview notes. |
Comments suppressed due to low confidence (1)
cli/azd/extensions/azure.ai.rle/internal/cmd/client_test.go:80
- azd-code-reviewer: Use
t.Context()for test I/O so the request is canceled if the test ends, as required by the repository's Go 1.26 test pattern (cli/azd/AGENTS.md:356).
err := client.do(context.Background(), http.MethodGet, environmentCollectionPath, nil, nil)
Validate persisted Foundry endpoints before creating credentials and cover the full project-relative route shape in invoke tests. Correct the release heading, prerequisites, and test contexts. Authored-by: GitHub Copilot CLI v1.0.68 Model: GPT-5.4 (gpt-5.4) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
farhann1
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
cli/azd/extensions/azure.ai.rle/internal/cmd/client.go:228
- azd-code-reviewer:
GetTokenfailures are local authentication failures, but both callers wrap this error withserviceError, labeling itrle-control-planeand suggesting that the endpoint is unreachable. A signed-out user therefore gets the wrong remediation. Return a distinct local authentication error withaz login/azd auth loginguidance before the generic service-error path;azdext.ServiceErroris reserved for HTTP/gRPC service failures (cli/azd/pkg/azdext/extension_error.go:15-16).
token, err := c.credential.GetToken(ctx, policy.TokenRequestOptions{
Scopes: []string{foundryTokenScope},
})
if err != nil {
return fmt.Errorf("authenticate to Foundry: %w", err)
cli/azd/extensions/azure.ai.rle/internal/cmd/invoke.go:98
- azd-code-reviewer: Removing the playground URL makes the browser-launch warning below non-actionable: on headless or SSH systems, users can no longer open the UI manually. Keep the raw sandbox URL hidden, but provide a safe fallback such as a localhost proxy URL or an actionable command when
OpenBrowserfails.
"Sandbox %s ready\n",
sandbox.Id,
cli/azd/extensions/azure.ai.rle/internal/cmd/deploy.go:116
- azd-code-reviewer: Both branches call the same API, so this condition implies create/update behavior that does not exist and can drift later. Collapse it to one call.
if state.EnvironmentId == "" {
environment, err = client.createV1Environment(a.cmd.Context(), request)
} else {
environment, err = client.createV1Environment(a.cmd.Context(), request)
}
cli/azd/extensions/azure.ai.rle/README.md:159
- The code now intentionally falls back to the project endpoint saved in
.azd-rle.jsonwhenFOUNDRY_PROJECT_ENDPOINTis unset, but this says deploy always reads the environment variable. Document the fallback and precedence so users know which Foundry project will receive the deployment.
Deploy reads the Foundry project endpoint from `FOUNDRY_PROJECT_ENDPOINT` and the ACR registry from `AZURE_CONTAINER_REGISTRY_ENDPOINT`. It derives the project route segment from `/api/projects/<project>`, builds the Docker image as `<registry>.azurecr.io/<project>-<environment>:latest`, pushes it to ACR, registers that image by calling `<FOUNDRY_PROJECT_ENDPOINT>/fine_tuning/environments`, and saves the project/environment details in `.azd-rle.json`.
cli/azd/extensions/azure.ai.rle/internal/cmd/client.go:88
- This remediation assumes the endpoint came from
FOUNDRY_PROJECT_ENDPOINT, but deploy can now use the value saved in.azd-rle.json. Refer to the configured endpoint generically and explain that the environment variable overrides it; otherwise the message points users at an unset variable.
This issue also appears on line 224 of the same file.
Suggestion: fmt.Sprintf(
"Ensure the Foundry project endpoint in %s is reachable and enabled for RLE.",
foundryProjectEndpointEnvVar,
),
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
cli/azd/extensions/azure.ai.rle/internal/cmd/client.go:95
normalizeFoundryProjectEndpointcurrently accepts paths with extra segments after/api/projects/<project>. That was mostly harmless when only the project segment was extracted, but this client now uses the whole normalized value as its base URL, so an endpoint such as.../api/projects/p/extrasends every request under/extra/fine_tuning/...and fails with a service error instead of being rejected. Require the project endpoint path to end after the project segment before using it as the base URL.
func newRleClient(endpoint string) (*rleClient, error) {
normalizedEndpoint, err := normalizeFoundryProjectEndpoint(endpoint)
if err != nil {
return nil, err
cli/azd/extensions/azure.ai.rle/internal/cmd/run.go:194
- Removing the runtime override disables
/webfor valid OpenEnv images whose Dockerfile defaultsENABLE_WEB_INTERFACE=false(for example, upstreamgrid_world_envanddipg_safety_env).runstill always opens and documents/web, so those environments now launch a missing playground. Keep passingENABLE_WEB_INTERFACE=trueto preserve the command's behavior.
"-p", portMapping,
image,
cli/azd/extensions/azure.ai.rle/internal/cmd/client.go:228
- A token acquisition failure is returned as an ordinary client error, so both
deployandinvokewrap it withserviceError. This reportsrle-control-planeas the failing service and suggests checking the endpoint even though no request was sent; users who need to sign in or select a credential get misleading recovery guidance. Preserve a distinct auth/local error here and avoid wrapping it as a service response.
token, err := c.credential.GetToken(ctx, policy.TokenRequestOptions{
Scopes: []string{foundryTokenScope},
})
if err != nil {
return fmt.Errorf("authenticate to Foundry: %w", err)
cli/azd/extensions/azure.ai.rle/internal/cmd/invoke.go:98
- The sandbox URL is hidden on the success path, but it is still exposed on failures.
project.WaitForHealthincludesbaseUrlin its timeout error, andRunShellWithContextincludes the full request URL on transport errors, soinvokecan print the data-plane URL whenever health or a shell request fails. Redact or replace those errors on the remote path to satisfy the stated URL-hiding behavior.
sandboxUrl := strings.TrimRight(sandbox.BaseUrl, "/")
if _, err := fmt.Fprintf(
a.cmd.OutOrStdout(),
"Sandbox %s ready\n",
sandbox.Id,
cli/azd/extensions/azure.ai.rle/internal/cmd/invoke.go:178
- The new 15-minute polling loop aborts on the first transient GET failure. Because this client has no Azure SDK retry policy, a temporary network error, 429, or 5xx during image conversion ends
invokeeven though conversion may still be progressing. Retry transient failures within the existing deadline (while still returning permanent 4xx responses) so the asynchronous wait is resilient.
deadline := time.Now().Add(remoteImageConversionTimeout)
for {
environment, err := client.getEnvironmentVersion(ctx, state.Name, state.EnvironmentVersion)
if err != nil {
return err
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
cli/azd/extensions/azure.ai.rle/internal/cmd/client.go:228
- Authentication failures here are later passed through
serviceErrorby both deploy and invoke, so they become anazdext.ServiceErrorattributed torle-control-planewith advice to check the endpoint.GetTokenfails locally before any service call, making the telemetry attribution and remediation incorrect. Preserve credential failures as a local authentication error (withaz login/azd auth loginguidance), and reserveServiceErrorfor an actual HTTP response. (azd-code-reviewer)
token, err := c.credential.GetToken(ctx, policy.TokenRequestOptions{
Scopes: []string{foundryTokenScope},
})
if err != nil {
return fmt.Errorf("authenticate to Foundry: %w", err)
cli/azd/extensions/azure.ai.rle/internal/cmd/invoke.go:94
- The sandbox URL is still exposed on failure paths despite the PR's promise to keep it out of terminal output.
project.WaitForHealthincludesbaseUrlin its returned error (runtime.go:145), andRunShellWithContextprints request errors containing the full URL (runtime.go:175-177). A health timeout or transient shell request failure will therefore reveal the data-plane proxy URL. Add a remote-sandbox redaction path so these helpers never render this URL. (azd-code-reviewer)
sandboxUrl := strings.TrimRight(sandbox.BaseUrl, "/")
cli/azd/extensions/azure.ai.rle/internal/cmd/run.go:194
- Removing this override breaks
run's documented promise to open/webfor custom OpenEnv images. OpenEnv's web UI is off by default unlessENABLE_WEB_INTERFACE=true, and current upstream images such asgrid_world_envanddipg_safety_envexplicitly bake infalse; those containers will remain healthy, but the browser opens a missing UI. Keep the runtime override soazd ai rle runconsistently enables the playground. (azd-code-reviewer)
"run", "-d",
"--name", container,
"--label", localContainerImageLabel + "=" + image,
"-p", portMapping,
image,
…ure/azure-dev into farhannawaz/rle-control-plane
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
cli/azd/extensions/azure.ai.rle/internal/cmd/publish.go:126
- [azd-code-reviewer] Both branches perform the same POST, so the local
EnvironmentIdcheck no longer affects the request. Collapse this to one call; keeping a duplicated create/update branch suggests distinct behavior that does not exist and makes future changes easy to apply to only one copy.
cli/azd/extensions/azure.ai.rle/internal/cmd/environment.go:132 - [azd-code-reviewer]
go fix ./...will rewrite thiserrors.Asusage toerrors.AsType, and the repository preflight enforces that modernization (cli/azd/AGENTS.md:332-358). Use the typed helper here so this new file passes the enforced check.
var localErr *azdext.LocalError
if !errors.As(stateErr, &localErr) || localErr.Code != "rle_project_not_initialized" {
return "", stateErr
cli/azd/extensions/azure.ai.rle/internal/cmd/invoke.go:94
- [azd-code-reviewer] The proxy URL is still exposed on error paths.
project.WaitForHealthincludesbaseUrlin its returned message (internal/project/runtime.go:145), and shell request failures print the full operation URL (runtime.go:177via lines 106-108). A failed health check or sandbox request therefore writessandbox.BaseUrlto the terminal despite the new redaction requirement; redact it in those errors or translate remote errors before displaying them.
sandboxUrl := strings.TrimRight(sandbox.BaseUrl, "/")
cli/azd/extensions/azure.ai.rle/internal/cmd/invoke.go:178
- [azd-code-reviewer] The new 15-minute polling loop aborts on the first transient GET failure. Because
rleClientuses a plainhttp.Clientwith no retry policy, a single 429 or 5xx while conversion is still progressing makesinvokefail immediately instead of continuing until the conversion deadline. Treat retryable HTTP/transport errors as another pending poll (while still honoringctxand the deadline).
environment, err := client.getEnvironmentVersion(ctx, state.Name, state.EnvironmentVersion)
if err != nil {
return err
cli/azd/extensions/azure.ai.rle/internal/cmd/client.go:100
- [azd-code-reviewer] This normalization does not guarantee a project-relative base URL.
projectNameFromFoundryEndpointaccepts any path with at least three segments, so an endpoint such as/api/projects/p/extrapasses validation; the new client then appends/fine_tuning/environmentsafter/extra, guaranteeing requests go to the wrong route. Require exactly/api/projects/<project>(or canonicalize to those three segments) before storingbaseUrl.
func newRleClient(endpoint string) (*rleClient, error) {
normalizedEndpoint, err := normalizeFoundryProjectEndpoint(endpoint)
if err != nil {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
cli/azd/extensions/azure.ai.rle/internal/cmd/publish.go:125
- Both branches issue the same POST, so this conditional no longer selects create versus update behavior and can drift as the client evolves. Call
createV1Environmentonce and keepcreatedonly for the user-facing label. (azd-code-reviewer)
cli/azd/extensions/azure.ai.rle/internal/cmd/invoke.go:178 - A single transient polling failure (for example HTTP 429/503 or a brief network error) aborts the entire 15-minute conversion wait immediately. Since this loop is specifically waiting for an asynchronous Azure operation, retry transient failures until the existing deadline and honor
Retry-After; only permanent responses should return here. (azd-code-reviewer)
environment, err := client.getEnvironmentVersion(ctx, state.Name, state.EnvironmentVersion)
if err != nil {
return err
cli/azd/extensions/azure.ai.rle/internal/cmd/environment.go:131
- Use
errors.AsType[*azdext.LocalError](stateErr)here. This repository requires the Go 1.26errors.AsTypepattern instead oferrors.As, and leaving this form causes the new code to fail the enforced modernization convention. (azd-code-reviewer)
var localErr *azdext.LocalError
if !errors.As(stateErr, &localErr) || localErr.Code != "rle_project_not_initialized" {
return "", stateErr
cli/azd/extensions/azure.ai.rle/internal/cmd/invoke.go:94
- The data-plane URL is still exposed on failure.
project.WaitForHealthformatsbaseUrlinto its returnedLocalError(runtime.go:145), andRunShellWithContextlater prints transport errors containing the full request URL (runtime.go:177). Thus an unhealthy or disconnected sandbox puts this URL back in terminal output despite this PR's redaction requirement. Please add a remote/redacted error path for both helpers. (azd-code-reviewer)
This issue also appears on line 176 of the same file.
sandboxUrl := strings.TrimRight(sandbox.BaseUrl, "/")
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Two things worth a second look here, plus a few nits.
publish.go:122-126both branches of the create/update conditional call the same thing, so either the update path is missing or the branch should go.publishnever writes the server's environment name back into state, andinvokekeys its disk-image poll off that name.
Nits: the 0.2.0-preview changelog heading is invisible to the changelog tooling, resolvePublishState and resolvePublishImage take a flags argument neither reads, and the paging loop in environment list has no upper bound.
Separately, PR Governance is red because there's no linked issue on this PR.
| if state.EnvironmentId == "" { | ||
| environment, err = client.createV1Environment(a.cmd.Context(), projectName, request) | ||
| environment, err = client.createV1Environment(a.cmd.Context(), request) | ||
| } else { | ||
| environment, err = client.updateV1Environment(a.cmd.Context(), projectName, environmentId, request) | ||
| if isNotFoundError(err) { | ||
| // The recorded environment no longer exists in the target project | ||
| // (e.g. the project changed or the control plane was reset). Recreate it. | ||
| if _, msgErr := fmt.Fprintf( | ||
| a.cmd.OutOrStdout(), | ||
| "Environment '%s' not found in project '%s'; creating a new one.\n", | ||
| environmentId, | ||
| projectName, | ||
| ); msgErr != nil { | ||
| return msgErr | ||
| } | ||
| created = true | ||
| environment, err = client.createV1Environment(a.cmd.Context(), projectName, request) | ||
| } | ||
| environment, err = client.createV1Environment(a.cmd.Context(), request) | ||
| } |
There was a problem hiding this comment.
Both branches call createV1Environment with the same request, so this conditional doesn't do anything. The action and label variables right above switch between "Creating" and "Updating", which reads like an update call was meant to land in the else branch.
If the control plane really does handle create-or-update through the same POST, the branch can just go:
| if state.EnvironmentId == "" { | |
| environment, err = client.createV1Environment(a.cmd.Context(), projectName, request) | |
| environment, err = client.createV1Environment(a.cmd.Context(), request) | |
| } else { | |
| environment, err = client.updateV1Environment(a.cmd.Context(), projectName, environmentId, request) | |
| if isNotFoundError(err) { | |
| // The recorded environment no longer exists in the target project | |
| // (e.g. the project changed or the control plane was reset). Recreate it. | |
| if _, msgErr := fmt.Fprintf( | |
| a.cmd.OutOrStdout(), | |
| "Environment '%s' not found in project '%s'; creating a new one.\n", | |
| environmentId, | |
| projectName, | |
| ); msgErr != nil { | |
| return msgErr | |
| } | |
| created = true | |
| environment, err = client.createV1Environment(a.cmd.Context(), projectName, request) | |
| } | |
| environment, err = client.createV1Environment(a.cmd.Context(), request) | |
| } | |
| environment, err = client.createV1Environment(a.cmd.Context(), request) |
If not, the else branch needs the real update call.
| deadline := time.Now().Add(remoteImageConversionTimeout) | ||
| for { | ||
| project, err := projectRouteSegment(state) | ||
| environment, err := client.getEnvironmentVersion(ctx, state.Name, state.EnvironmentVersion) |
There was a problem hiding this comment.
This polls /fine_tuning/environments/{state.Name}/versions/{version}, but publish only writes environment.Id and environment.Version back to state. It leaves state.Name at whatever the local project derived. The publish JSON output prints environment.Name from the response while the console line prints state.Name, so the service clearly hands back a name of its own.
If the service ever normalizes or slugs that name on create, this GET 404s and invoke dies after the 15 minute conversion deadline with a confusing error. Setting state.Name from the publish response alongside the id and version would close that gap. Is the returned name guaranteed to match what was sent?
| return nil, err | ||
| } | ||
| environments = append(environments, page.Value...) | ||
| if len(page.Value) < environmentListPageSize { |
There was a problem hiding this comment.
Nothing caps this loop. The only exit is a short page, so if the service ignores skip or top and keeps returning a full page, environment list spins until the user hits Ctrl+C while all grows the whole time. A max-page or max-item guard is a couple of lines and turns a hang into an error.
| } | ||
| } | ||
|
|
||
| func resolvePublishState(flags *rlePublishFlags) (rleState, bool, error) { |
There was a problem hiding this comment.
flags isn't read anywhere in this function, and resolvePublishImage takes it without reading it either. Both can drop the parameter.
| - Rename `azd ai rle deploy` to `azd ai rle publish` to avoid confusion with the core `azd deploy` command. | ||
| - Add `--version-bump` to `azd ai rle publish` so users can choose major, minor, or patch environment versioning. | ||
|
|
||
| ## 0.2.0-preview |
There was a problem hiding this comment.
eng/common/scripts/ChangeLog-Operations.ps1 builds its release-title regex with a required parenthesized status group, so a heading without one doesn't match and this section is invisible to the changelog tooling. 0.2.1-preview (Unreleased) above is fine. This one needs either a release date or (Unreleased) depending on whether you consider 0.2.0-preview shipped.
Changes: