-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
{AKS} [KMS]: Remove private keyvault kms tests #29965
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
d744728
to
7167515
Compare
--enableVnetIntegration=true
/azp run |
Commenter does not have sufficient privileges for PR 29965 in repo Azure/azure-cli |
7167515
to
4dd25a1
Compare
AKS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Queued live test to validate the change.
- test_aks_create_with_azurekeyvaultkms_private_key_vault
- test_aks_update_with_azurekeyvaultkms_private_key_vault
- test_aks_create_with_azurekeyvaultkms_private_cluster_v1_private_key_vault
I would say this is not a breaking change to user as you've only modified tests. Also, the changes are invisible to users, you could change the beginning of the PR title to {AKS} (won't leave a note in the release history). |
4dd25a1
to
4d8a7c1
Compare
--enableVnetIntegration=true
--enableVnetIntegration=true
--enableVnetIntegration=true
--enableVnetIntegration=true
Thank you! Updated. |
'--assign-identity {identity_id} ' \ | ||
'--enable-azure-keyvault-kms --azure-keyvault-kms-key-id={key_id} ' \ | ||
'--azure-keyvault-kms-key-vault-network-access=Private --azure-keyvault-kms-key-vault-resource-id {kv_resource_id} ' \ | ||
'--enable-apiserver-vnet-integration=true ' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an action option and does not accept a value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
4d8a7c1
to
b9b7443
Compare
private keyvault + konnectivity is blocked now, so vnet integration should be used. Signed-off-by: Zhecheng Li <[email protected]>
b9b7443
to
ea96ee8
Compare
--enableVnetIntegration=true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@zhoxing-ms @yanzhudd Could you please merge this one? Thanks. |
May I ask why do you need to remove this test? |
The test case is no longer a valid scenario. To use private key vault with KMS enabled, user need to enable the preview feature vnet integration for the cluster. Since it is a preview feature, the official azure-cli does not support it yet. Therefore, the relevant test cases are retained in cli-extensions/aks-preview after being appropriately modified, but have been deleted here. |
Related command
az aks create/update --enable-azure-keyvault-kms
Description
[KMS] Update tests. private keyvault + konnectivity is blocked now, so vnet integration should be used
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a
: Make some customer-facing breaking change[Component Name 2]
az command b
: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.