Conversation
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
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>
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the initial scaffolding for the aks-safeguards Azure CLI extension, including setup configuration, command implementations via AAZ, test stubs, and documentation placeholders.
- Added extension metadata and packaging setup
- Generated AAZ-based command modules for
create,update,show,list,delete, andwait - Included test scaffold and README with usage placeholder
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/aks-safeguards/setup.py | Defines packaging metadata and entry points for the extension |
| src/aks-safeguards/setup.cfg | Placeholder config file (currently empty) |
| src/aks-safeguards/azext_aks_safeguards/tests/latest/test_aks_safeguards.py | Adds test class scaffold without any test cases |
| src/aks-safeguards/README.md | Initial README with placeholder for command usage |
Comments suppressed due to low confidence (3)
src/aks-safeguards/azext_aks_safeguards/tests/latest/test_aks_safeguards.py:13
- The test class is empty and no scenarios are covered yet. Add at least one basic scenario test to validate core command behavior and improve coverage.
pass
src/aks-safeguards/README.md:5
- The README contains a placeholder for usage examples. Populate this section with actual command examples and descriptions to guide users.
Please add commands usage here.
src/aks-safeguards/setup.cfg:1
- [nitpick] The setup.cfg file is empty aside from a comment. Either remove it if not needed or populate it with relevant configuration (e.g., metadata, tool settings).
#setup.cfg
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 |
|
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 8919 in repo Azure/azure-cli-extensions |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Co-authored-by: Yan Zhu <105691024+yanzhudd@users.noreply.github.com>
…rds/_create.py Co-authored-by: Yan Zhu <105691024+yanzhudd@users.noreply.github.com>
Co-authored-by: Yan Zhu <105691024+yanzhudd@users.noreply.github.com>
|
@yanzhudd I've addressed your feedback, and I would appreciate another review, thanks! |
|
Please hold off on merging for now |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| | Example | Description | | ||
| |---------|-------------| | ||
| | `az aks safeguards update --resource-group MyResourceGroup --name MyAKSCluster --level Enforce` | Update Deployment Safeguards to Enforce level for an AKS cluster with a specific name and resource group | | ||
| | `az aks safeguards update --resource-group MyResourceGroup --name MyAKSCluster --excluded-namespaces [ns1,ns2] --level Warn` | Update Deployment Safeguards to Warn level for an AKS cluster with excluded namespaces | |
There was a problem hiding this comment.
| | `az aks safeguards update --resource-group MyResourceGroup --name MyAKSCluster --excluded-namespaces [ns1,ns2] --level Warn` | Update Deployment Safeguards to Warn level for an AKS cluster with excluded namespaces | | |
| | `az aks safeguards update --resource-group MyResourceGroup --name MyAKSCluster --excluded-namespaces ns1 ns2 --level Warn` | Update Deployment Safeguards to Warn level for an AKS cluster with excluded namespaces | |
| | Example | Description | | ||
| |---------|-------------| | ||
| | `az aks safeguards create --resource-group MyResourceGroup --name MyAKSCluster --level Warn` | Enable Deployment Safeguards for an AKS cluster at Warn level | | ||
| | `az aks safeguards create --resource-group MyResourceGroup --name MyAKSCluster --level Warn --excluded-namespaces [ns1,ns2]` | Enable Deployment Safeguards at Warn level for an AKS cluster with excluded namespaces | |
There was a problem hiding this comment.
| | `az aks safeguards create --resource-group MyResourceGroup --name MyAKSCluster --level Warn --excluded-namespaces [ns1,ns2]` | Enable Deployment Safeguards at Warn level for an AKS cluster with excluded namespaces | | |
| | `az aks safeguards create --resource-group MyResourceGroup --name MyAKSCluster --level Warn --excluded-namespaces ns1 ns2` | Enable Deployment Safeguards at Warn level for an AKS cluster with excluded namespaces | |
|
It looks good to me. Please address above comments and I'll merge it. |
|
@yanzhudd @FumingZhang after talking to AKS PMs, the decision is to move this to the AKS module in the core cli: Azure/azure-cli#31793 |
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.