[AppConfig] Support AAD auth for data operations #15160
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
avanigupta
force-pushed
the
avanigupta/aadauthdataplane
branch
from
eb01f76 to
badda2f
Compare
Member
|
hi @bim-msft could you pls review the PR? thanks. |
avanigupta
force-pushed
the
avanigupta/aadauthdataplane
branch
from
badda2f to
176810e
Compare
avanigupta
requested review from
fengzhou-msft and
haroldrandom
as code owners
Member
Author
|
Hi @recao, could you please help review this PR? |
avanigupta
force-pushed
the
avanigupta/aadauthdataplane
branch
from
f0f435f to
5968768
Compare
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
avanigupta
force-pushed
the
avanigupta/aadauthdataplane
branch
from
2df627d to
b112dad
Compare
Member
Author
Member
|
hi @fengzhou-msft could you pls help to review since @bim-msft is oof? thanks. |
fengzhou-msft
approved these changes
Sep 30, 2020
Member
Author
|
Hi @yonzhan , since Jiashuo is OOF, can someone else help review this PR? |
Collaborator
|
@avanigupta We are in Chinese National holiday. @jiasli can help review this PR after holiday. Sorry for the inconvenience caused. |
jiasli
approved these changes
Oct 9, 2020
jiasli
reviewed
Oct 9, 2020
Member
|
/azp run |
|
No pipelines are associated with this pull request. |
Member
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
avanigupta
force-pushed
the
avanigupta/aadauthdataplane
branch
from
ba3811f to
f3eac0a
Compare
yungezz
reviewed
Oct 9, 2020
| argcomplete==1.11.1 | ||
| asn1crypto==0.24.0 | ||
| azure-appconfiguration==1.1.0 | ||
| azure-appconfiguration==1.1.1 |
Member
There was a problem hiding this comment.
pls run live test to make sure no breaking change in sdk version bumping
Member
Author
There was a problem hiding this comment.
I've run all live tests and updated test recordings in this PR.
yungezz
approved these changes
Oct 9, 2020
jiasli
reviewed
Oct 27, 2020
Comment on lines
+152
to
+154
| # Due to this bug in get_login_credentials: https://github.com/Azure/azure-cli/issues/15179, | ||
| # we need to manage the AAD scope by passing appconfig endpoint as resource | ||
| cred, _, _ = profile.get_login_credentials(resource=endpoint) |
Member
There was a problem hiding this comment.
#15184 has been merged. You may now safely remove resource=endpoint.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
App Configuration supports AAD authentication and authorization for all data operations. Since we are now using the official Python SDK for AppConfiguration, we can easily support AAD auth in appconfig CLI module by utilizing the
az logincredentials.This PR introduces two new arguments for users to authenticate using AAD credentials:
--auth-mode (login or key; default is key which preserves the current behavior)
--endpoint (AppConfiguration endpoint like
https://storename.azconfig.io)For any existing AppConfig commands, providing the
--nameand--connection-stringparameters will use key (HMAC) auth for data operations (preserving current behavior). Instead of these two arguments, if users provide--auth-mode loginand--endpointparameters, AAD credentials will be used to connect to AzureAppConfigurationClient.Testing Guide
This read request should succeed:
az appconfig kv show --key testKey --auth-mode login --endpoint https://aadtestcli.azconfig.ioThis write request should fail with 'Forbidden' error:
az appconfig kv lock --key testKey --auth-mode login --endpoint https://aadtestcli.azconfig.ioHistory Notes
This 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.