-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[AppConfig] Support AAD auth for data operations #15160
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
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ef39950
Support AAD auth for data operations
avanigupta cf93bc5
Fix appservice validation if auth mode is AAD
avanigupta 972e686
Add scenario tests
avanigupta 3642839
Make AAD test LiveScenarioTest
avanigupta 7785696
Change --auth-mode options to 'login' and 'key'
avanigupta 8e37052
Allow users to provide store name in AAD auth mode
avanigupta 4a9634a
Resolve PR feedback
avanigupta e495761
Change AAD example commands to use endpoint
avanigupta 8a88e59
Change exception handling during client creation
avanigupta 3c28376
Remove extra whitespace
avanigupta b74b425
Update error message and appconfig sdk version
avanigupta f3eac0a
Update tests
avanigupta File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,41 +6,48 @@ | |
| # -------------------------------------------------------------------------------------------- | ||
|
|
||
| # pylint: disable=line-too-long | ||
| from knack.log import get_logger | ||
| from knack.prompting import NoTTYException, prompt_y_n | ||
| from knack.util import CLIError | ||
| from azure.appconfiguration import AzureAppConfigurationClient | ||
| from azure.mgmt.appconfiguration.models import ErrorException | ||
|
|
||
| from ._client_factory import cf_configstore | ||
| from ._constants import HttpHeaders | ||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
|
|
||
| def construct_connection_string(cmd, config_store_name): | ||
| connection_string_template = 'Endpoint={};Id={};Secret={}' | ||
| # If the logged in user/Service Principal does not have 'Reader' or 'Contributor' role | ||
| # assigned for the requested AppConfig, resolve_store_metadata will raise CLI error | ||
| resource_group_name, endpoint = resolve_store_metadata(cmd, config_store_name) | ||
|
|
||
| try: | ||
| resource_group_name, endpoint = resolve_resource_group( | ||
| cmd, config_store_name) | ||
| config_store_client = cf_configstore(cmd.cli_ctx) | ||
| access_keys = config_store_client.list_keys( | ||
| resource_group_name, config_store_name) | ||
| access_keys = config_store_client.list_keys(resource_group_name, config_store_name) | ||
| for entry in access_keys: | ||
| if not entry.read_only: | ||
| return connection_string_template.format(endpoint, entry.id, entry.value) | ||
| except Exception: | ||
| raise CLIError( | ||
| 'Cannot find the App Configuration {}. Check if it exists in the subscription that logged in. '.format(config_store_name)) | ||
| except ErrorException as ex: | ||
| raise CLIError('Failed to get access keys for the App Configuration "{}". Make sure that the account that logged in has sufficient permissions to access the App Configuration store.\n{}'.format(config_store_name, str(ex))) | ||
|
|
||
| raise CLIError('Cannot find a read write access key for the App Configuration {}'.format( | ||
| config_store_name)) | ||
| raise CLIError('Cannot find a read write access key for the App Configuration {}'.format(config_store_name)) | ||
|
|
||
|
|
||
| def resolve_resource_group(cmd, config_store_name): | ||
| config_store_client = cf_configstore(cmd.cli_ctx) | ||
| all_stores = config_store_client.list() | ||
| for store in all_stores: | ||
| if store.name.lower() == config_store_name.lower(): | ||
| # Id has a fixed structure /subscriptions/subscriptionName/resourceGroups/groupName/providers/providerName/configurationStores/storeName" | ||
| return store.id.split('/')[4], store.endpoint | ||
| raise CLIError( | ||
| "App Configuration store: {} does not exist".format(config_store_name)) | ||
| def resolve_store_metadata(cmd, config_store_name): | ||
| try: | ||
| config_store_client = cf_configstore(cmd.cli_ctx) | ||
| all_stores = config_store_client.list() | ||
| for store in all_stores: | ||
| if store.name.lower() == config_store_name.lower(): | ||
| # Id has a fixed structure /subscriptions/subscriptionName/resourceGroups/groupName/providers/providerName/configurationStores/storeName" | ||
| return store.id.split('/')[4], store.endpoint | ||
| except ErrorException as ex: | ||
| raise CLIError("Failed to get the list of App Configuration stores for the current user. Make sure that the account that logged in has sufficient permissions to access the App Configuration store.\n{}".format(str(ex))) | ||
|
|
||
| raise CLIError("Failed to find the App Configuration store '{}'.".format(config_store_name)) | ||
|
|
||
avanigupta marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| def user_confirmation(message, yes=False): | ||
|
|
@@ -118,3 +125,38 @@ def prep_null_label_for_url_encoding(label=None): | |
| label = '"{0}"'.format(label) | ||
| label = ast.literal_eval(label) | ||
| return label | ||
|
|
||
|
|
||
| def get_appconfig_data_client(cmd, name, connection_string, auth_mode, endpoint): | ||
| azconfig_client = None | ||
| if auth_mode == "key": | ||
| connection_string = resolve_connection_string(cmd, name, connection_string) | ||
| try: | ||
| azconfig_client = AzureAppConfigurationClient.from_connection_string(connection_string=connection_string, | ||
| user_agent=HttpHeaders.USER_AGENT) | ||
| except ValueError as ex: | ||
| raise CLIError("Failed to initialize AzureAppConfigurationClient due to an exception: {}".format(str(ex))) | ||
|
|
||
| if auth_mode == "login": | ||
| if not endpoint: | ||
| try: | ||
| if name: | ||
| _, endpoint = resolve_store_metadata(cmd, name) | ||
| else: | ||
| raise CLIError("App Configuration endpoint or name should be provided if auth mode is 'login'.") | ||
| except Exception as ex: | ||
| raise CLIError(str(ex) + "\nYou may be able to resolve this issue by providing App Configuration endpoint instead of name.") | ||
|
|
||
| from azure.cli.core._profile import Profile | ||
| profile = Profile(cli_ctx=cmd.cli_ctx) | ||
| # 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) | ||
|
Comment on lines
+152
to
+154
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #15184 has been merged. You may now safely remove |
||
| try: | ||
| azconfig_client = AzureAppConfigurationClient(credential=cred, | ||
| base_url=endpoint, | ||
| user_agent=HttpHeaders.USER_AGENT) | ||
| except (ValueError, TypeError) as ex: | ||
| raise CLIError("Failed to initialize AzureAppConfigurationClient due to an exception: {}".format(str(ex))) | ||
|
|
||
| return azconfig_client | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.