{AKS} az aks bastion: Fix --subscription not being passed to internal az network bastion tunnel and bastion discovery commands#9671
Conversation
️✔️Azure CLI Extensions Breaking Change Test
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
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>
|
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 |
|
Queued live test to validate the change.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes az aks bastion so the active --subscription is forwarded to the internal bastion discovery (az network bastion show/list) and tunnel (az network bastion tunnel) commands, and bumps the aks-preview extension version accordingly.
Changes:
- Pass subscription ID from
az aks bastioninto bastion discovery helper logic. - Forward
--subscriptioninto theaz network bastion tunnelinvocation. - Bump extension version to
19.0.0b25and add a correspondingHISTORY.rstentry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/aks-preview/setup.py |
Bumps extension version to 19.0.0b25. |
src/aks-preview/azext_aks_preview/custom.py |
Retrieves subscription ID from CLI context and passes it into bastion helpers/runner. |
src/aks-preview/azext_aks_preview/bastion/bastion.py |
Adds optional subscription_id plumbing to bastion discovery and tunnel launch. |
src/aks-preview/HISTORY.rst |
Adds release notes entry for the subscription forwarding fix. |
Comments suppressed due to low confidence (1)
src/aks-preview/azext_aks_preview/bastion/bastion.py:496
- The tunnel command is built as a single string and then tokenized via
cmd.split()before callingcreate_subprocess_exec. With the newsubscription_idincluded, this becomes vulnerable to argument-splitting issues (and potential argument injection if the subscription string contains spaces). Build the subprocess command as an argument list and append--subscriptionand its value as separate elements instead of concatenating/splitting a string.
cmd = (
f"{az_cmd_name} network bastion tunnel --resource-group {bastion_resource.resource_group} "
f"--name {bastion_resource.name} --port {port} --target-resource-id {mc_id} --resource-port 443"
)
if subscription_id:
cmd += f" --subscription {subscription_id}"
logger.warning("Creating bastion tunnel with command: '%s'", cmd)
# Use start_new_session on Unix to create a new process group
# This allows us to kill the entire process tree when cleaning up
start_new_session = not sys.platform.startswith("win")
tunnel_proces = await asyncio.create_subprocess_exec(
*(cmd.split()),
stdin=asyncio.subprocess.DEVNULL,
| result = run_az_cmd( | ||
| [ | ||
| "network", | ||
| "bastion", | ||
| "show", | ||
| "--resource-group", | ||
| resource_group, | ||
| "--name", | ||
| bastion, | ||
| "--output", | ||
| "json", | ||
| ], | ||
| show_cmd, | ||
| out_file=TextIO(), | ||
| ) |
There was a problem hiding this comment.
run_az_cmd is being called with out_file=TextIO() but TextIO here comes from typing (a type annotation), not a writable file-like object. This will fail at runtime (or at least won’t behave as intended) when suppressing output. Use a real in-memory buffer (e.g., io.StringIO) or omit out_file and rely on run_az_cmd’s returned result instead.
| subscription_id = get_subscription_id(cmd.cli_ctx) | ||
| mc = client.get(resource_group_name, name) | ||
| mc_id = mc.id | ||
| nrg = mc.node_resource_group | ||
| bastion_resource = aks_bastion_parse_bastion_resource(bastion, [nrg]) | ||
| bastion_resource = aks_bastion_parse_bastion_resource(bastion, [nrg], subscription_id) | ||
| port = aks_bastion_get_local_port(port) | ||
|
|
There was a problem hiding this comment.
This change introduces explicit --subscription propagation into bastion discovery and the tunnel launch, but there’s no automated test asserting that the subscription flag is forwarded correctly. Consider adding a unit test that mocks run_az_cmd and the tunnel subprocess creation to verify --subscription is included for both bastion discovery and az network bastion tunnel when the user sets --subscription.
|
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az aks bastionGeneral 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.