Skip to content

fix: dev to main - Use package feed proxy - Update Azure configuration and enhance WAF with proxy changes - #1055

Merged
Avijit-Microsoft merged 10 commits into
mainfrom
dev
Aug 27, 2026
Merged

fix: dev to main - Use package feed proxy - Update Azure configuration and enhance WAF with proxy changes#1055
Avijit-Microsoft merged 10 commits into
mainfrom
dev

Conversation

@Pavan-Microsoft

Copy link
Copy Markdown
Contributor

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:

  • All Python requirements.txt files now set the --index-url to https://packagefeedproxy.microsoft.io/pypi/simple/ to ensure packages are installed from the internal proxy. [1] [2] [3]
  • All relevant GitHub Actions workflows (deploy.yml, test.yml, test-automation.yml, job-test-automation.yml, pylint.yml) set the PIP_INDEX_URL environment variable for consistent pip package sourcing during CI/CD. [1] [2] [3] [4] [5]
  • The .devcontainer configuration and src/app/.npmrc now set the NPM_CONFIG_REGISTRY and registry respectively to https://packagefeedproxy.microsoft.io/npm/, ensuring npm packages are sourced from the internal registry both in dev containers and local development. [1] [2] [3]
  • The Node.js Dockerfile for the web app now copies .npmrc into the build context, further enforcing the use of the internal npm registry during Docker builds.

Infrastructure and deployment template updates:

  • The virtualMachineAvailabilityZone variable in Bicep and generated ARM templates is changed from 1 to -1, likely to indicate no specific availability zone. [1] [2] [3]
  • The order of dependencies in the dependsOn arrays of ARM templates is adjusted for private DNS zone deployments, possibly to ensure correct resource provisioning order. [1] [2]
  • Updated templateHash values in generated ARM templates to reflect changes in template content. [1] [2] [3]

Dependency version update:

  • Upgraded PyYAML from version 6.0.2 to 6.0.3 in src/api/requirements.txt.

Does this introduce a breaking change?

  • Yes
  • No

Golden Path Validation

  • I have tested the primary workflows (the "golden path") to ensure they function correctly without errors.

Deployment Validation

  • I have validated the deployment process successfully and all services are running as expected with this change.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.yml and does not include src/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.

Comment thread infra/avm/main.bicep
Comment thread src/app/.npmrc
Copilot AI review requested due to automatic review settings August 25, 2026 04:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:84 and azure.yaml:145) without the proxy. A standard deployment therefore still downloads pip from the default index before this file is processed. Propagate PIP_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/health returns HTTP 503 whenever any check is unhealthy, and Cosmos is optional (deployCosmos defaults to false; an unset Cosmos endpoint is reported as unavailable). Invoke-RestMethod therefore enters catch even when checks.sql is "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

  • $elapsed is incremented only by the poll interval after the request completes, but each failed Invoke-RestMethod can 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) so TimeoutSec is 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 v3 resolved entries point to ms-feed-*.pkgs.visualstudio.com (for example @alloc/quick-lru), and npm install honors those tarball URLs instead of rewriting them from registry. 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/

Comment thread src/api/storage/sql_service.py
Copilot AI review requested due to automatic review settings August 26, 2026 11:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-RestMethod throws for non-2xx responses (including the expected 503 “degraded” response from /api/health while 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 virtualMachineAvailabilityZone to -1 is passed through to the proximityPlacementGroup module (see availabilityZone: virtualMachineAvailabilityZone a few lines below). The wrapper module infra/avm/modules/compute/proximity-placement-group.bicep defines availabilityZone as an int defaulting to 1 and forwards it directly to the AVM module; -1 is therefore likely to produce an invalid ARM value and break deployments when enablePrivateNetworking is 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_failed no longer set on exceptions, _ensure_init() will attempt a full init (including DefaultAzureCredential().get_token(...) and table creation) on every .available check 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}")

@Pavan-Microsoft Pavan-Microsoft changed the title fix - dev to main - Use package feed proxy - Update Azure configuration and enhance WAF with proxy changes fix: dev to main - Use package feed proxy - Update Azure configuration and enhance WAF with proxy changes Aug 27, 2026
@Avijit-Microsoft
Avijit-Microsoft merged commit 3a8a439 into main Aug 27, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants