Conversation
* forcedelete * format * add code owner * mypy
… '2025-08-01-preview' (#17)
…add E2E coverage and improve logging (#20) * add pester tests for connectedk8s cli extension * Pass the force delete param to the API call (#4) * forcedelete * format * add code owner * mypy * Parameterize for airgapped clouds (#5) * Add parameterization for the airgapped clouds * Fix azdev style * MCR path function * azdev, ruff, and mypy --------- Co-authored-by: Matthew McNeal (from Dev Box) <mmcneal@microsoft.com> * Oras client fix to work with different MCRs (#6) Co-authored-by: mmcneal <mmcneal@microsoft.com> * fix CI testcases for nodepool image issues (#8) * update errors for the config and connectivity issues (#11) * update errors * format * style * update python version to 3.13 (#12) * Update cluster diagnostics image to 1.29.3 (#7) * Update cluster diagnostics helm chart to 1.29.3 * Fix lint issues --------- Co-authored-by: bgriddaluru <bharath.griddaluru@microsoft.com> * RBAC deprecation & fix the issue * typo * fix comments * update tests * add pester tests for connectedk8s cli extension * Pass the force delete param to the API call (#4) * forcedelete * format * add code owner * mypy * fix CI testcases for nodepool image issues (#8) * update errors for the config and connectivity issues (#11) * update errors * format * style * update python version to 3.13 (#12) * rebase * fix tests * fix version * fix mypy, lint * fix test * fix test * fix test * fix test * fix test * rename test * deprecate flags * rebase * rebase * bump version for release --------- Co-authored-by: Bavneet Singh <bavneetsingh@microsoft.com> Co-authored-by: Atchut Kumar Barli <atchut@gmail.com> Co-authored-by: mcnealm13 <57726243+mcnealm13@users.noreply.github.com> Co-authored-by: Matthew McNeal (from Dev Box) <mmcneal@microsoft.com> Co-authored-by: Bavneet Singh <33008256+bavneetsingh16@users.noreply.github.com> Co-authored-by: bgriddaluru <117554445+bgriddaluru@users.noreply.github.com> Co-authored-by: bgriddaluru <bharath.griddaluru@microsoft.com> Co-authored-by: vithumma <vithumma@microsoft.com>
* add agc overrides * update gns endpoint * add indentation * fix linter error * fix ruff formatting * move overrides to it's own method * update method * update ruff formatting
* add pester tests for connectedk8s cli extension * Pass the force delete param to the API call (#4) * forcedelete * format * add code owner * mypy * Parameterize for airgapped clouds (#5) * Add parameterization for the airgapped clouds * Fix azdev style * MCR path function * azdev, ruff, and mypy --------- Co-authored-by: Matthew McNeal (from Dev Box) <mmcneal@microsoft.com> * Oras client fix to work with different MCRs (#6) Co-authored-by: mmcneal <mmcneal@microsoft.com> * fix CI testcases for nodepool image issues (#8) * update errors for the config and connectivity issues (#11) * update errors * format * style * update python version to 3.13 (#12) * Update cluster diagnostics image to 1.29.3 (#7) * Update cluster diagnostics helm chart to 1.29.3 * Fix lint issues --------- Co-authored-by: bgriddaluru <bharath.griddaluru@microsoft.com> * RBAC deprecation & fix the issue * typo * fix comments * update tests * add pester tests for connectedk8s cli extension * Pass the force delete param to the API call (#4) * forcedelete * format * add code owner * mypy * fix CI testcases for nodepool image issues (#8) * update errors for the config and connectivity issues (#11) * update errors * format * style * update python version to 3.13 (#12) * rebase * fix tests * fix version * fix mypy, lint * fix test * fix test * fix test * fix test * fix test * rename test * add pester tests for connectedk8s cli extension * Pass the force delete param to the API call (#4) * forcedelete * format * add code owner * mypy * fix CI testcases for nodepool image issues (#8) * update python version to 3.13 (#12) * changes to support gateway association/disassociation for api version '2025-08-01-preview' (#17) * [Azure RBAC] Deprecate 3P mode flags, fix Azure RBAC enablement bug, add E2E coverage and improve logging (#20) * add pester tests for connectedk8s cli extension * Pass the force delete param to the API call (#4) * forcedelete * format * add code owner * mypy * Parameterize for airgapped clouds (#5) * Add parameterization for the airgapped clouds * Fix azdev style * MCR path function * azdev, ruff, and mypy --------- Co-authored-by: Matthew McNeal (from Dev Box) <mmcneal@microsoft.com> * Oras client fix to work with different MCRs (#6) Co-authored-by: mmcneal <mmcneal@microsoft.com> * fix CI testcases for nodepool image issues (#8) * update errors for the config and connectivity issues (#11) * update errors * format * style * update python version to 3.13 (#12) * Update cluster diagnostics image to 1.29.3 (#7) * Update cluster diagnostics helm chart to 1.29.3 * Fix lint issues --------- Co-authored-by: bgriddaluru <bharath.griddaluru@microsoft.com> * RBAC deprecation & fix the issue * typo * fix comments * update tests * add pester tests for connectedk8s cli extension * Pass the force delete param to the API call (#4) * forcedelete * format * add code owner * mypy * fix CI testcases for nodepool image issues (#8) * update errors for the config and connectivity issues (#11) * update errors * format * style * update python version to 3.13 (#12) * rebase * fix tests * fix version * fix mypy, lint * fix test * fix test * fix test * fix test * fix test * rename test * deprecate flags * rebase * rebase * bump version for release --------- Co-authored-by: Bavneet Singh <bavneetsingh@microsoft.com> Co-authored-by: Atchut Kumar Barli <atchut@gmail.com> Co-authored-by: mcnealm13 <57726243+mcnealm13@users.noreply.github.com> Co-authored-by: Matthew McNeal (from Dev Box) <mmcneal@microsoft.com> Co-authored-by: Bavneet Singh <33008256+bavneetsingh16@users.noreply.github.com> Co-authored-by: bgriddaluru <117554445+bgriddaluru@users.noreply.github.com> Co-authored-by: bgriddaluru <bharath.griddaluru@microsoft.com> Co-authored-by: vithumma <vithumma@microsoft.com> * remove breaking change announcement for removed flags --------- Co-authored-by: Bavneet Singh <bavneetsingh@microsoft.com> Co-authored-by: Atchut Kumar Barli <atchut@gmail.com> Co-authored-by: mcnealm13 <57726243+mcnealm13@users.noreply.github.com> Co-authored-by: Matthew McNeal (from Dev Box) <mmcneal@microsoft.com> Co-authored-by: Bavneet Singh <33008256+bavneetsingh16@users.noreply.github.com> Co-authored-by: bgriddaluru <117554445+bgriddaluru@users.noreply.github.com> Co-authored-by: bgriddaluru <bharath.griddaluru@microsoft.com> Co-authored-by: vithumma <vithumma@microsoft.com>
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
CodeGen Tools Feedback CollectionThank you for using our CodeGen tool. We value your feedback, and we would like to know how we can improve our product. Please take a few minutes to fill our codegen survey |
|
There was a problem hiding this comment.
Pull request overview
This PR updates the connectedk8s extension version and adjusts cloud-specific MCR endpoint resolution, while also introducing an end-to-end integration test suite and Azure DevOps pipeline to run it.
Changes:
- Bumped connectedk8s extension version and updated history; removed deprecated
--app-id/--app-secretparameters. - Updated
get_mcr_pathusage/signature and added AGC (USSec/USNat) endpoint override handling. - Added a new PowerShell-based integration test suite plus Azure DevOps pipeline templates to run the scenarios.
Reviewed changes
Copilot reviewed 31 out of 35 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/test/helper/Constants.ps1 | Adds shared constants/config loading for new Pester tests |
| testing/test/configurations/WorkloadIdentity.Tests.ps1 | Adds workload identity scenario integration tests |
| testing/test/configurations/Troubleshoot.Tests.ps1 | Adds troubleshoot command scenario test |
| testing/test/configurations/Proxy.Tests.ps1 | Adds proxy enable/disable scenario test |
| testing/test/configurations/Gateway.Tests.ps1 | Adds gateway scenario test |
| testing/test/configurations/ForcedDelete.Tests.ps1 | Adds forced delete scenario test |
| testing/test/configurations/EnableDisableFeatures.Tests.ps1 | Adds enable/disable features scenario test with helpers |
| testing/test/configurations/ConnectProxy.Tests.ps1 | Adds az connectedk8s proxy behavior validation scenario |
| testing/test/configurations/BasicOnboarding.Tests.ps1 | Adds basic onboarding + auto-upgrade toggle scenario |
| testing/test/configurations/AutoUpdate.Tests.ps1 | Adds auto-upgrade enable/disable scenario test |
| testing/settings.template.json | Adds template for local test settings |
| testing/pipeline/templates/run-test.yml | Adds reusable pipeline job template to build and run tests |
| testing/pipeline/k8s-custom-pipelines.yml | Adds pipeline wiring to run each scenario and official checks |
| testing/owners.txt | Adds owners list for the testing assets |
| testing/bin/connectedk8s-values.yaml | Adds helm values override used by tests/bootstrap |
| testing/Test.ps1 | Adds test runner script for CI/local usage |
| testing/README.md | Adds documentation for running the integration test suite |
| testing/Bootstrap.ps1 | Adds bootstrap script to prep local/CI environment |
| testing/.gitignore | Adds ignores for generated settings/tmp/results and allows specific tracked binaries |
| src/k8s-extension/azext_k8s_extension/utils.py | Updates get_mcr_path signature/logic for cloud handling |
| src/k8s-extension/azext_k8s_extension/custom.py | Updates call site for new get_mcr_path signature |
| src/connectedk8s/setup.py | Bumps extension version to 1.10.12 |
| src/connectedk8s/azext_connectedk8s/tests/unittests/test_utils_.py | Adds unit tests for get_mcr_path |
| src/connectedk8s/azext_connectedk8s/custom.py | Updates call site for new get_mcr_path; removes deprecated params |
| src/connectedk8s/azext_connectedk8s/clientproxyhelper/_binaryutils.py | Updates call site for new get_mcr_path signature |
| src/connectedk8s/azext_connectedk8s/_utils.py | Updates get_mcr_path; adds AGC endpoint override helper |
| src/connectedk8s/azext_connectedk8s/_precheckutils.py | Updates call site for new get_mcr_path signature |
| src/connectedk8s/azext_connectedk8s/_params.py | Removes deprecated CLI args --app-id/--app-secret |
| src/connectedk8s/azext_connectedk8s/_constants.py | Updates diagnoser helm chart tag and client proxy version |
| src/connectedk8s/azext_connectedk8s/_client_factory.py | Removes hardcoded ARM base_url override |
| src/connectedk8s/azext_connectedk8s/_breaking_change.py | Removes breaking-change registration for removed args |
| src/connectedk8s/HISTORY.rst | Adds 1.10.11/1.10.12 release notes |
Comments suppressed due to low confidence (2)
testing/test/configurations/Proxy.Tests.ps1:1
- The unconditional
breakat line 49 causes the retry loop to exit on the firstSucceededprovisioningState even whenisProxyEnabledis notfalse, which can make the test pass/fail incorrectly. Remove the extrabreak(or convert it tocontinue) so the loop only breaks when the proxy state matches the expected value.
testing/test/configurations/Gateway.Tests.ps1:1 - The gateway resource ID is hardcoded to a specific subscription/resource group, which makes the test suite environment-specific and difficult to reuse in other subscriptions/tenants. Move this value into
settings.json(similar to other environment config fields) and read it from$ENVCONFIG, or pass it via pipeline variables so the same tests can run in different environments without code changes.
| if ($ParallelCI) { | ||
| # This runs the tests in parallel during the CI pipline to speed up testing | ||
|
|
||
| Write-Host "Invoking Pester to run tests from '$testFilePath'..." | ||
| $testFiles = @() | ||
| foreach ($paths in $testFilePaths) | ||
| { | ||
| $temp = Get-ChildItem $paths | ||
| $testFiles += $temp | ||
| } | ||
| $resultFileNumber = 0 | ||
| foreach ($testFile in $testFiles) | ||
| { | ||
| $resultFileNumber++ | ||
| $testName = Split-Path $testFile –leaf | ||
| Start-Job -ArgumentList $testName, $testFile, $resultFileNumber, $TestFileDirectory -Name $testName -ScriptBlock { | ||
| param($name, $testFile, $resultFileNumber, $testFileDirectory) | ||
|
|
||
| Write-Host "$testFile to result file #$resultFileNumber" | ||
| $testResult = Invoke-Pester $testFile -Passthru -Output Detailed | ||
| $testResult | Export-JUnitReport -Path "$testFileDirectory/$name.xml" | ||
| } | ||
| } | ||
|
|
||
| do { | ||
| Write-Host ">> Still running tests @ $(Get-Date –Format "HH:mm:ss")" –ForegroundColor Blue | ||
| Get-Job | Where-Object { $_.State -eq "Running" } | Format-Table –AutoSize | ||
| Start-Sleep –Seconds 30 | ||
| } while((Get-Job | Where-Object { $_.State -eq "Running" } | Measure-Object).Count -ge 1) |
There was a problem hiding this comment.
The script uses $testFilePath (singular) in the ParallelCI branch even though it builds $testFilePaths earlier, and several PowerShell parameters use a Unicode en-dash (–) instead of a hyphen-minus (-) (e.g., Split-Path ... –leaf, Get-Date –Format, Start-Sleep –Seconds). The en-dash will break parameter parsing, and the $testFilePath reference is undefined here. Replace en-dashes with - and use the correct variable ($testFilePaths or a derived path string) for the logging line.
| arm_metadata_endpoint_array = ( | ||
| arm_metadata["authentication"]["loginEndpoint"].strip("/").split(".") | ||
| ) | ||
| if len(arm_metadata_endpoint_array) < 4: | ||
| raise CLIInternalError("Unexpected loginEndpoint format for AGC") | ||
|
|
||
| cloud_suffix = arm_metadata_endpoint_array[3] | ||
| endpoint_suffix = ( | ||
| arm_metadata_endpoint_array[2] + "." + arm_metadata_endpoint_array[3] | ||
| ) | ||
| if cloud_name.lower() == "usnat": | ||
| cloud_suffix = ( | ||
| arm_metadata_endpoint_array[2] | ||
| + "." | ||
| + arm_metadata_endpoint_array[3] | ||
| + "." | ||
| + arm_metadata_endpoint_array[4] | ||
| ) |
There was a problem hiding this comment.
usnat access at arm_metadata_endpoint_array[4] can raise IndexError because the length check only enforces >= 4. If loginEndpoint splits into exactly 4 segments, this will crash. Update validation to require >= 5 for the usnat case (and keep >= 4 for ussec), or restructure parsing so both cases validate the required indexes before indexing.
| def get_mcr_path(active_directory_endpoint: str) -> str: | ||
| active_directory_array = active_directory_endpoint.split(".") | ||
|
|
||
| # default for public, mc, ff clouds | ||
| mcr_postfix = active_directory_array[2] | ||
| # For US Government and China clouds, use public mcr | ||
| if active_directory_endpoint.endswith((".us", ".cn")): | ||
| return "mcr.microsoft.com" | ||
|
|
||
| # Default MCR postfix | ||
| mcr_postfix = "com" |
There was a problem hiding this comment.
This changes behavior for active directory hosts that end with a non-.com public suffix (e.g., login.microsoftonline.de), since the default postfix is now forced to "com" instead of using the endpoint’s suffix segment. Also, .endswith((".us", ".cn")) will not match common forms with a trailing slash (e.g., https://login.microsoftonline.us/). Consider normalizing the endpoint (strip trailing /, and ideally extract the hostname) and defaulting mcr_postfix to the endpoint’s suffix segment (while still overriding .us/.cn to mcr.microsoft.com if that’s the desired behavior).
| curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/master/scripts/get-helm-3 | ||
| chmod 700 get_helm.sh | ||
| ./get_helm.sh --version v3.6.3 | ||
| echo "Installing kubectl" | ||
| curl -LO "https://storage.googleapis.com/kubernetes-release/release/$(curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)/bin/linux/amd64/kubectl" |
There was a problem hiding this comment.
The pipeline downloads and executes scripts/binaries from moving targets (helm install script from the master branch and kind from the GitHub “latest” release) without pinning to a commit and without integrity verification. This is a supply-chain risk and reduces build reproducibility. Pin these downloads to immutable versions (tag/commit) and add checksum/signature verification (or use trusted package sources/tasks).
| curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/master/scripts/get-helm-3 | |
| chmod 700 get_helm.sh | |
| ./get_helm.sh --version v3.6.3 | |
| echo "Installing kubectl" | |
| curl -LO "https://storage.googleapis.com/kubernetes-release/release/$(curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)/bin/linux/amd64/kubectl" | |
| # Pin Helm install script to a specific tagged version and verify checksum | |
| HELM_INSTALL_URL="https://raw.githubusercontent.com/helm/helm/v3.6.3/scripts/get-helm-3" | |
| HELM_INSTALL_SHA256="<INSERT_KNOWN_GOOD_SHA256_FOR_GET_HELM_SH_V3_6_3>" | |
| curl -fsSL -o get_helm.sh "${HELM_INSTALL_URL}" | |
| echo "${HELM_INSTALL_SHA256} get_helm.sh" | sha256sum -c - | |
| chmod 700 get_helm.sh | |
| ./get_helm.sh --version v3.6.3 | |
| echo "Installing kubectl" | |
| # Pin kubectl to a specific version and verify checksum | |
| KUBECTL_VERSION="v1.27.3" | |
| curl -fsSLO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl" | |
| curl -fsSLO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl.sha256" | |
| echo "$(cat kubectl.sha256) kubectl" | sha256sum -c - |
| - bash : | | ||
| echo "Downloading the kind script" | ||
| # Get the latest version tag and download | ||
| LATEST_KIND_VERSION=$(curl -s https://api.github.com/repos/kubernetes-sigs/kind/releases/latest | grep '"tag_name":' | sed -E 's/.*"([^"]+)".*/\1/') | ||
| curl -Lo ./kind "https://kind.sigs.k8s.io/dl/${LATEST_KIND_VERSION}/kind-linux-amd64" | ||
| chmod +x ./kind | ||
| ./kind create cluster |
There was a problem hiding this comment.
The pipeline downloads and executes scripts/binaries from moving targets (helm install script from the master branch and kind from the GitHub “latest” release) without pinning to a commit and without integrity verification. This is a supply-chain risk and reduces build reproducibility. Pin these downloads to immutable versions (tag/commit) and add checksum/signature verification (or use trusted package sources/tasks).
| # Add overrides for AGC Scenario | ||
| if cloud_name.lower() == "ussec" or cloud_name.lower() == "usnat": | ||
| add_agc_endpoint_overrides(location, cloud_name, arm_metadata, cmd_helm_install) |
There was a problem hiding this comment.
add_agc_endpoint_overrides is newly introduced behavior that mutates cmd_helm_install with several cloud-specific overrides, but there’s no accompanying unit test coverage for ussec/usnat inputs and expected --set arguments. Add focused unit tests that pass representative arm_metadata["authentication"]["loginEndpoint"] values and assert the resulting helm args contain the expected overrides.
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.