fix: dev to main - Use package feed proxy - Update Azure configuration and enhance WAF with proxy changes - #1055
Conversation
fix: down merge to dev - Update Azure configuration, enhance WAF, and restructure infrastructure
fix: updated the Microsoft allowed package manager proxy
There was a problem hiding this comment.
Pull request overview
This pull request standardizes internal PyPI/npm package sourcing, updates PyYAML, and modifies WAF infrastructure configuration and generated templates.
Changes:
- Adds pip and npm proxy configuration across dependencies, workflows, Docker, and devcontainer files.
- Updates PyYAML to 6.0.3.
- Changes the VM availability-zone setting and refreshes generated ARM templates.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Summary |
|---|---|
src/app/WebApp.Dockerfile |
Copies npm registry configuration into builds. |
src/app/.npmrc |
Configures the internal npm registry. Moderate issue (3 votes): the lockfile still pins tarballs to ms-feed-*.pkgs.visualstudio.com; regenerate or rewrite its resolved hosts. |
src/api/requirements.txt |
Adds the PyPI proxy and updates PyYAML. |
infra/vscode_web/requirements.txt |
Adds the PyPI proxy. |
infra/vscode_web/endpoint-requirements.txt |
Adds the PyPI proxy. |
infra/scripts/post-provision/requirements.txt |
Adds the PyPI proxy. |
infra/main.json |
Updates the generated ARM template. |
infra/avm/main.json |
Updates the generated AVM template. |
infra/avm/main.bicep |
Changes the WAF VM availability-zone setting. Critical issue (2 votes): changing the shared value from 1 to -1 can fail for existing environments because Azure VM availability zones are immutable; preserve the existing zone or provide a replacement/migration path. |
.github/workflows/test.yml |
Configures the pip proxy for tests. |
.github/workflows/test-automation.yml |
Configures the pip proxy for automation tests. |
.github/workflows/pylint.yml |
Configures the pip proxy for linting. |
.github/workflows/job-test-automation.yml |
Configures the pip proxy for job automation. |
.github/workflows/deploy.yml |
Configures the pip proxy for deployment. |
.devcontainer/Dockerfile |
Configures the npm proxy in the container. |
.devcontainer/devcontainer.json |
Configures the npm proxy at runtime. |
Suppressed comments (1)
src/app/.npmrc:1
- The PR deployment workflow uses explicit path filters in
.github/workflows/deploy-v2.ymland does not includesrc/app/.npmrc. A future change to this build-affecting registry configuration alone would therefore skip the PR deployment/e2e validation, even though it changes where the Docker build obtains dependencies. Add this file to that workflow's paths (or broaden the app path filter).
registry=https://packagefeedproxy.microsoft.io/npm/
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
infra/scripts/post-provision/requirements.txt:1
- This option only applies when pip reads this requirements file, but the normal azd post-provision hook first runs
python -m pip install --quiet --upgrade pip(azure.yaml:84andazure.yaml:145) without the proxy. A standard deployment therefore still downloads pip from the default index before this file is processed. PropagatePIP_INDEX_URL/pip configuration or pass the proxy index to that bootstrap install as well.
--index-url https://packagefeedproxy.microsoft.io/pypi/simple/
infra/scripts/post-provision/setup-data.ps1:262
/api/healthreturns HTTP 503 whenever any check is unhealthy, and Cosmos is optional (deployCosmosdefaults tofalse; an unset Cosmos endpoint is reported asunavailable).Invoke-RestMethodtherefore enterscatcheven whenchecks.sqlis"ok", so every deployed setup waits the full 300 seconds before proceeding. Read the response body despite the 503 (or make the health endpoint return 200 when only optional checks are degraded) before evaluating SQL readiness.
$resp = Invoke-RestMethod -Uri $healthUrl -Method GET -TimeoutSec 10
if ($resp.checks.sql -eq "ok") {
infra/scripts/post-provision/setup-data.ps1:278
- The return value from the readiness wait is discarded, so after 300 seconds of non-healthy SQL this hook still performs cleanup, scenario registration, and uploads. Those API paths intentionally swallow SQL persistence failures and can return 200, allowing the post-provision step to appear successful while the scenario is not durable; abort data setup when the wait returns false.
Wait-ForBackendSqlHealthy -BackendUrl $BackendUrl | Out-Null
infra/scripts/post-provision/setup-data.ps1:271
$elapsedis incremented only by the poll interval after the request completes, but each failedInvoke-RestMethodcan already consume the 10-second request timeout. With the defaults, an unreachable backend can therefore make this function wait about 600 seconds while reporting a 300-second timeout. Track a wall-clock deadline (or include request duration) soTimeoutSecis an actual upper bound.
$elapsed += $PollIntervalSec
src/app/.npmrc:1
- This registry setting does not cover the dependency tree already pinned in
src/app/package-lock.json: its v3resolvedentries point toms-feed-*.pkgs.visualstudio.com(for example@alloc/quick-lru), andnpm installhonors those tarball URLs instead of rewriting them fromregistry. Regenerate and commit the lockfile with the proxy (or configure npm's registry-host replacement) if the Docker/local installs must consistently use this proxy.
registry=https://packagefeedproxy.microsoft.io/npm/
fix: fixed the devcontainer Issue for upgrading the pip using the feedproxy
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
infra/scripts/post-provision/setup-data.ps1:269
Invoke-RestMethodthrows for non-2xx responses (including the expected 503 “degraded” response from/api/healthwhile SQL is still coming up). The current catch logs “Backend not reachable yet”, which is misleading when the backend is reachable but unhealthy; it also hides the HTTP status code that would help diagnose readiness issues.
} catch {
Write-Host " Backend not reachable yet — retrying in ${PollIntervalSec}s..." -ForegroundColor DarkGray
}
infra/avm/main.bicep:408
- Setting
virtualMachineAvailabilityZoneto-1is passed through to theproximityPlacementGroupmodule (seeavailabilityZone: virtualMachineAvailabilityZonea few lines below). The wrapper moduleinfra/avm/modules/compute/proximity-placement-group.bicepdefinesavailabilityZoneas anintdefaulting to1and forwards it directly to the AVM module;-1is therefore likely to produce an invalid ARM value and break deployments whenenablePrivateNetworkingis true.
Consider either (a) keeping a valid zone value (1–3) here, or (b) updating the proximity placement group wrapper (and regenerating the compiled templates) to treat -1 as “no zone” by omitting/NULLing the zonal property (and/or conditionally skipping the PPG deployment when the VM is not zonal).
var virtualMachineAvailabilityZone = -1
module proximityPlacementGroup './modules/compute/proximity-placement-group.bicep' = if (enablePrivateNetworking) {
name: take('module.proximity-placement-group.${solutionName}', 64)
params: {
solutionName: solutionSuffix
src/api/storage/sql_service.py:60
- With
_init_failedno longer set on exceptions,_ensure_init()will attempt a full init (includingDefaultAzureCredential().get_token(...)and table creation) on every.availablecheck until SQL succeeds. Under load, that can generate a large number of token requests and repeated init work while SQL permissions are still propagating (or if SQL is misconfigured), which can amplify startup incidents.
Consider adding a simple retry throttle/backoff (e.g., store self._next_init_retry_at and return early until that time) so the service retries periodically without doing expensive init on every request.
except Exception as e:
logger.warning(f"Azure SQL init failed (will retry on next use): {e}")
Purpose
This pull request introduces several improvements to package management and environment configuration across the development, CI/CD, and infrastructure code. The main focus is to standardize the use of internal package registries for both Python (
pip) and Node.js (npm) dependencies, ensuring consistent and secure package sourcing. Additionally, there are small updates to infrastructure deployment templates and dependency versions.Package registry and dependency management improvements:
requirements.txtfiles now set the--index-urltohttps://packagefeedproxy.microsoft.io/pypi/simple/to ensure packages are installed from the internal proxy. [1] [2] [3]deploy.yml,test.yml,test-automation.yml,job-test-automation.yml,pylint.yml) set thePIP_INDEX_URLenvironment variable for consistent pip package sourcing during CI/CD. [1] [2] [3] [4] [5].devcontainerconfiguration andsrc/app/.npmrcnow set theNPM_CONFIG_REGISTRYandregistryrespectively tohttps://packagefeedproxy.microsoft.io/npm/, ensuring npm packages are sourced from the internal registry both in dev containers and local development. [1] [2] [3].npmrcinto the build context, further enforcing the use of the internal npm registry during Docker builds.Infrastructure and deployment template updates:
virtualMachineAvailabilityZonevariable in Bicep and generated ARM templates is changed from1to-1, likely to indicate no specific availability zone. [1] [2] [3]dependsOnarrays of ARM templates is adjusted for private DNS zone deployments, possibly to ensure correct resource provisioning order. [1] [2]templateHashvalues in generated ARM templates to reflect changes in template content. [1] [2] [3]Dependency version update:
PyYAMLfrom version6.0.2to6.0.3insrc/api/requirements.txt.Does this introduce a breaking change?
Golden Path Validation
Deployment Validation