Enable CFSClean network isolation for Python pipelines - #48483
Enable CFSClean network isolation for Python pipelines#48483Daniel Jurek (danieljurek) wants to merge 15 commits into
Conversation
The default PIP_EXTRA_INDEX_URL was https://pypi.python.org/simple, which is applied by set_envvar_defaults() across ~18 azpysdk checks. Every pip invocation in those checks therefore probed public PyPI alongside the CFS feed, producing the bulk of the CFSClean network isolation violations observed in the "python - pullrequest" pipeline. Point both the shared default and the virtualenv seeding script at the azure-sdk-for-python Azure Artifacts feed, which has an upstream to PyPI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f9d7432-df2e-4c67-8838-bde213908a16
|
Azure Pipelines: Successfully started running 1 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Validation-only commit. The PR otherwise touches eng/ only, so the diff-driven package resolution produced TargetingString=null and every package check (apistub, pyright, verifytypes, mypy, whl/sdist) was skipped -- which made the network isolation result a false negative. Revert before merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f9d7432-df2e-4c67-8838-bde213908a16
… targeting" This reverts commit c3a62f7.
This reverts commit 4f72fc1.
|
Azure Pipelines: Successfully started running 1 pipeline(s). 10 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
Routes Python package resolution through the CFS-backed Azure DevOps feed and enables CFSClean pipeline enforcement.
Changes:
- Replaces public PyPI extra-index defaults with the CFS feed.
- Enables
Permissive, CFSCleannetwork isolation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
eng/tools/azure-sdk-tools/ci_tools/variables.py |
Updates the default pip extra index. |
eng/scripts/seed-virtualenv-wheels.ps1 |
Seeds virtualenv wheels through CFS. |
eng/pipelines/templates/stages/1es-redirect.yml |
Enables CFSClean enforcement. |
| "VIRTUALENV_PIP": "24.0", | ||
| "VIRTUALENV_SETUPTOOLS": "75.3.2", | ||
| "PIP_EXTRA_INDEX_URL": "https://pypi.python.org/simple", | ||
| "PIP_EXTRA_INDEX_URL": "https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-python/pypi/simple/", |
Repointing PIP_EXTRA_INDEX_URL at the CFS feed made it redundant rather than correct. azpysdk.main already defaults PIP_INDEX_URL and UV_DEFAULT_INDEX to CFS_INDEX_URL when unset (main.py:198-201), and use-python-version.yml does the same at pipeline level -- both using "set only if unset" so PipAuthenticate@1's authenticated URL wins in CI. Keeping an extra index pointed at that same feed had three downsides: it was unauthenticated, it made pip query the same index twice per resolution, and it leaked the CFS feed into `azpysdk --pypi` runs, which explicitly set PIP_INDEX_URL to pypi.org and are meant to resolve from PyPI only. seed-virtualenv-wheels.ps1 runs virtualenv --download outside azpysdk, so it does not inherit that defaulting. It now prefers an already-set PIP_INDEX_URL and only falls back to the public CFS feed, matching the pattern in use-python-version.yml. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f9d7432-df2e-4c67-8838-bde213908a16
Validation-only. Without a package file in the diff the resolver yields TargetingString=null and every package check is skipped, which makes the network isolation result a false negative. Revert before merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f9d7432-df2e-4c67-8838-bde213908a16
Validation-only. Without a package file in the diff the resolver yields
TargetingString=null and every package check is skipped, making the
network isolation result a false negative.
Targets three deliberately different shapes:
- azure-keyvault-secrets: ordinary data-plane client
- azure-core: root dependency, rebuilt/revalidated widely
- azure-storage-extensions: the repo's only C-extension package, so it
exercises the cibuildwheel path (native build containers, and the
bundled nuget.exe that reaches api.nuget.org) that the other two do not
Revert before merge.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6f9d7432-df2e-4c67-8838-bde213908a16
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
sdk/keyvault/azure-keyvault-secrets/README.md:380
- This package-only whitespace change contradicts the PR description's statement that the temporary targeting touches were reverted, and it leaves an unrelated package in the final change/targeting set. Please remove the added blank line so the PR remains scoped to the three network-isolation files.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
sdk/storage/azure-storage-extensions/README.md:51
- The PR description says these validation-only package touches were reverted, but this marker is still present and explicitly says to revert it before merge. Remove it so the unrelated package README is not changed by this infrastructure PR.
<!-- Trivial change to exercise CI network-isolation validation. Revert before merge. -->
sdk/keyvault/azure-keyvault-secrets/README.md:381
- The PR description says these validation-only package touches were reverted, but this marker is still present and explicitly says to revert it before merge. Remove it so the unrelated package README is not changed by this infrastructure PR.
<!-- Trivial change to exercise CI network-isolation validation. Revert before merge. -->
sdk/core/azure-core/README.md:281
- The PR description says these validation-only package touches were reverted, but this marker is still present and explicitly says to revert it before merge. Remove it so the unrelated package README is not changed by this infrastructure PR.
<!-- Trivial change to exercise CI network-isolation validation. Revert before merge. -->
This comment has been minimized.
This comment has been minimized.
Touching azure-core pulled in all 22 of its dependents, including azure-communication-chat, azure-communication-identity, and azure-eventhub. Those are the packages whose dev_requirements need version resolution, which routes through PyPIClient.project_release() -> the hardcoded https://pypi.org JSON API and fails under CFSClean (issue #48254, fix in progress separately). Keeping azure-keyvault-secrets (ordinary data-plane client) and azure-storage-extensions (the repo's only C-extension package, exercising the cibuildwheel path) so the run still validates real package checks. Revert before merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f9d7432-df2e-4c67-8838-bde213908a16
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
sdk/keyvault/azure-keyvault-secrets/README.md:381
- The PR description says these validation-only package touches were reverted, but this marker is still in the proposed changes. It has no product documentation value and would leave the package README with a temporary CI artifact; remove it before merge.
<!-- Trivial change to exercise CI network-isolation validation. Revert before merge. -->
sdk/storage/azure-storage-extensions/README.md:51
- The PR description says these validation-only package touches were reverted, but this marker is still in the proposed changes. It has no product documentation value and would leave the package README with a temporary CI artifact; remove it before merge.
<!-- Trivial change to exercise CI network-isolation validation. Revert before merge. -->
| settings: | ||
| skipBuildTagsForGitHubPullRequests: true | ||
| networkIsolationPolicy: Permissive | ||
| networkIsolationPolicy: Permissive, CFSClean |
…ouch Comment mindependency out of both PR_BUILD_SET and FULL_BUILD_SET in eng/scripts/set_checks.py. Under CFSClean, mindependency fails for any package whose dev_requirements need version resolution: that path routes through PyPIClient.project_release() -> the hardcoded https://pypi.org JSON API in pypi_tools/pypi.py, which the network isolation policy blocks. Tracked as #48346. With mindependency out of the way, restore the TEMP azure-core README touch so the validation run fans out to all 22 azure-core dependents again -- the widest blast radius available, and the one that surfaced the mindependency problem in build 6673903. TEMP README touches (azure-core, azure-keyvault-secrets, azure-storage-extensions) must be reverted before merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f9d7432-df2e-4c67-8838-bde213908a16
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (5)
eng/scripts/set_checks.py:28
- This also removes minimum-dependency validation from all ordinary PR builds, so the reported compliant run no longer covers a check that previously reached PyPI. The PR description neither discloses this test reduction nor fixes that path, while linked issue #48346 confirms it is expected to be restored. Keep the check in the PR set and address its CFSClean incompatibility instead of suppressing it.
# Testing mindependency is disabled for CFS onboarding.
# https://github.com/Azure/azure-sdk-for-python/issues/48346
PR_BUILD_SET = ["whl", "sdist"] #, "mindependency"]
sdk/storage/azure-storage-extensions/README.md:51
- This validation-only marker explicitly says it must be reverted, and the PR description states that the package-file touches were already reverted. Leaving it in the merge diff creates an unrelated package change solely to affect CI targeting; remove these two added lines before merge.
<!-- Trivial change to exercise CI network-isolation validation. Revert before merge. -->
sdk/keyvault/azure-keyvault-secrets/README.md:381
- This validation-only marker explicitly says it must be reverted, and the PR description states that the package-file touches were already reverted. Leaving it in the merge diff creates an unrelated package change solely to affect CI targeting; remove these two added lines before merge.
<!-- Trivial change to exercise CI network-isolation validation. Revert before merge. -->
eng/scripts/set_checks.py:22
- Removing
mindependencymakes every internal/full build silently stop validating packages against their minimum supported dependencies. This is an omitted third workaround rather than the claimed fix for the network access, and linked issue #48346 explicitly tracks re-enabling the check. Keep it enabled and resolve the package-metadata access under CFSClean before enforcing the policy.
This issue also appears on line 26 of the same file.
# Testing mindependency is disabled for CFS onboarding.
# https://github.com/Azure/azure-sdk-for-python/issues/48346
# "mindependency",
sdk/core/azure-core/README.md:281
- This validation-only marker explicitly says it must be reverted, and the PR description states that the package-file touches were already reverted. Leaving it in the merge diff creates an unrelated package change solely to affect CI targeting; remove these two added lines before merge.
<!-- Trivial change to exercise CI network-isolation validation. Revert before merge. -->
[Pilot] PR Pipeline Failure AnalysisA CI pipeline failed on this pull request. Here is an automated analysis of what went wrong and how to get the build green. What failedFour distinct pipeline failures were detected across this PR:
Recommended next steps
Raw pipeline analysis (azsdk ci analyze)
|
Removes the trivial HTML comments added to azure-core, azure-keyvault-secrets, and azure-storage-extensions READMEs. Those existed only to make resolve-package-info.ps1 target real packages so the network isolation policy could be validated against actual check execution rather than a no-op run. Validation is complete: build 6674359 fanned out to all 22 azure-core dependents and reported CFSClean COMPLIANT on 21/21 jobs, with python - cosmos - ci (6674357) COMPLIANT on 12/12. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f9d7432-df2e-4c67-8838-bde213908a16
The conda jobs were the only remaining CI path that never received the CFS treatment. They used the raw UsePythonVersion@0 task followed by a bare `python -m pip install`, so PIP_INDEX_URL was never set and pip's PEP 517 build-isolation subprocess resolved setuptools>=42 straight from pypi.org. Under `networkIsolationPolicy: Permissive, CFSClean` that connection is severed and the job fails before it can assemble any packages. Swap in the repo's use-python-version.yml wrapper (which seeds a default PIP_INDEX_URL before the task's own pip auto-restore runs) and add auth-dev-feed.yml to upgrade that to an authenticated feed URL, matching what build-package-artifacts.yml and analyze.yml already do. Twine auth is disabled since these jobs publish pipeline artifacts, not packages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe967ba8-45e1-4dfc-9a91-266c553ea44c
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
eng/scripts/set_checks.py:28
- The default PR build no longer runs
mindependency, allowing minimum-supported dependency regressions to pass PR validation. This material reduction in checks is also absent from the PR description, which says package checks genuinely executed. Please make the metadata lookup CFSClean-compatible and retainmindependencyinPR_BUILD_SET.
# Testing mindependency is disabled for CFS onboarding.
# https://github.com/Azure/azure-sdk-for-python/issues/48346
PR_BUILD_SET = ["whl", "sdist"] #, "mindependency"]
eng/scripts/set_checks.py:22
- This removes
mindependencyfrom every internal/full build, so incompatible minimum dependency combinations can now merge without being exercised. The linked issue explicitly tracks re-enabling this check; suppressing it changes validation scope rather than fixing CFSClean compatibility. Please fix thePyPIClient.project_release()path to use an isolation-compliant source and keep this check enabled.
This issue also appears on line 26 of the same file.
# Testing mindependency is disabled for CFS onboarding.
# https://github.com/Azure/azure-sdk-for-python/issues/48346
# "mindependency",
sdk_build_conda resolved every package's source distribution by calling the public PyPI JSON API directly (get_package_sdist_url) and then downloading from files.pythonhosted.org over raw urllib. Neither call goes through pip, so PIP_INDEX_URL did not apply and both hosts are unreachable under CFSClean network isolation. Resolution now happens at download time via `pip download --no-deps --no-binary :all:`, which honors PIP_INDEX_URL and therefore uses the authenticated Azure Artifacts feed. This also stops the resolution from running at config-parse time, where it issued a network call for every configured package even when only a small batch was being built. Explicit download_uri entries keep their existing behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe967ba8-45e1-4dfc-9a91-266c553ea44c
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
eng/scripts/set_checks.py:22
- The PR description says CFSClean is enabled by fixing two network violations and lists only three changed files, but this also removes minimum-dependency validation from both PR and full nightly/release build sets. Issue #48346 confirms that re-enabling it is deferred. Please disclose this temporary loss of validation and its follow-up in the PR description so reviewers can assess the rollout impact.
# Testing mindependency is disabled for CFS onboarding.
# https://github.com/Azure/azure-sdk-for-python/issues/48346
# "mindependency",
| # repackaging it. so we download and move it directly to assembled | ||
| if len(conda_build.checkout) == 1: | ||
| return download_pypi_source(output_folder, checkout_config.download_uri) | ||
| return resolve_package_source(checkout_config, output_folder) |
conda resolves its build environment against conda.anaconda.org and repo.anaconda.com. It has no PIP_INDEX_URL equivalent and Azure Artifacts has no conda feed type, so under CFSClean the connection is severed with EPERM before `conda env create` can build the environment. Make networkIsolationPolicy a parameter on 1es-redirect.yml, keep "Permissive, CFSClean" as the default for every pipeline, and lower only conda-sdk-client.yml to "Permissive". The pip-based portions of the conda pipelines do restore from the CFS feed and are unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe967ba8-45e1-4dfc-9a91-266c553ea44c
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
eng/tools/azure-sdk-tools/ci_tools/conda/conda_functions.py:434
- This fallback also handles configurations that specify
checkout_pathplusversion:get_package_sourcecurrently enters the download branch for any version, so those configurations will now download the published package instead of cloning the requested repository path/tag. Gate that branch onfrom_package_indexso the documented git-checkout form continues to reachget_git_source.
return download_sdist_from_index(target_folder, checkout_config.package, checkout_config.version)
eng/scripts/set_checks.py:22
- The PR description says CFSClean is enabled by fixing two network violations and lists only three changed files, but this removes
mindependencyfrom both the public PR and internal full-build defaults. Issue #48346 confirms this is a temporary repository-wide test reduction. Either make the check CFS-compatible or explicitly disclose this coverage loss and its follow-up in the PR description/validation claims.
# Testing mindependency is disabled for CFS onboarding.
# https://github.com/Azure/azure-sdk-for-python/issues/48346
# "mindependency",
eng/tools/azure-sdk-tools/ci_tools/conda/CondaConfiguration.py:66
- The existing Conda configuration test only verifies construction does not raise, so it does not validate this new routing flag. Add assertions for version-only, explicit-download, and
checkout_pathconfigurations; these cases determine whether source retrieval uses the package index or git and would catch routing regressions.
# A package identified only by name + version is sourced from a package index rather than
# from a git checkout. Resolution is deferred to download time so that it can be performed
# by pip against PIP_INDEX_URL, rather than by a direct call to the public PyPI API here.
# Resolving eagerly would also force a network call for every configured package, including
# ones that are not part of the current batch.
self.from_package_index = bool(self.version and self.checkout_path is None)
Bring
python - pullrequest(and friends) into CFSClean network-isolation compliancePart of the S360/SFI network-isolation effort. This enables the CFSClean policy and fixes the two things that were violating it.
Changes
eng/pipelines/templates/stages/1es-redirect.ymlnetworkIsolationPolicy: Permissive→Permissive, CFSCleaneng/tools/azure-sdk-tools/ci_tools/variables.pyPIP_EXTRA_INDEX_URL→ ADO feed instead ofpypi.python.orgeng/scripts/seed-virtualenv-wheels.ps1Root cause
PipAuthenticate@1sets onlyPIP_INDEX_URL.DEFAULT_ENVIRONMENT_VARIABLESis applied viaos.environ.setdefault, so the hardcodedPIP_EXTRA_INDEX_URL=https://pypi.python.org/simplesurvived authentication and was appended to every dependency resolution in every isolated venv (apistub,pyright,verifytypes,mypy,sphinx, …).Smoking gun from a baseline run:
Two pre-existing local workarounds corroborate this —
ci_tools/scenario/generation.pypassesPIP_EXTRA_INDEX_URL="", andeng/common/pipelines/templates/steps/python-auth-dev-feed.ymlclears it outright. This fixes it at the source instead.Validation
Baseline of 14 recent def-7050 builds: 9/14 NOT COMPLIANT for CFSClean, concentrated in
AnalyzeandBuild Extended.pypi.orgpypi.python.orgfiles.pythonhosted.orgFinal run 6670611 — succeeded, CFSClean COMPLIANT 13/13, with real package targeting (
azure-core, azure-keyvault-secrets, azure-template), so package checks genuinely executed rather than being skipped.pyright / verifytypes under enforcement
pyright-pythonself-checkshttps://pypi.org/pypi/pyright/jsonon startup to warn about new versions. Under CFSClean that request is now blocked — verified harmless:mureq.get(..., timeout=1)caps the wait at 1s; a bareexcept Exceptionswallows it and returnsNone, so no warning is printed and nothing raises.Run Pyrightsucceeded in 8.5s,Run verifytypesin 35.3s.WARNING: there is a new pyright version availableline present in baseline logs is absent here, confirming the call was blocked and handled gracefully.registry.npmjs.orgis never contacted either.Optionally,
PYRIGHT_PYTHON_IGNORE_WARNINGS=1would skip the request entirely (~1s saved per check), but it is not required for correctness.Out of scope (still non-compliant under other policies)
www.powershellgallery.comfrompwsh(eng/common/scripts/Helpers/PSModule-Helpers.ps1). Lives ineng/common, so it must be fixed inAzure/azure-sdk-toolsand synced.intersphinx_mappingreaching readthedocs-family domains (doc/sphinx/conf.py).Neither affects CFSClean.