-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[App Config] az appconfig kv import: Support importing key-values from AKS ConfigMap
#31861
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
Changes from all commits
7a5af88
853300f
6a346cc
a88288e
7ce1d50
692c843
46a44a0
c99dad3
25ea74b
7e9c87d
99ec0b6
1c01a66
44c6471
fb6a562
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |
| from azure.cli.core.util import user_confirmation | ||
| from azure.cli.core.azclierror import ( | ||
| AzureInternalError, | ||
| AzureResponseError, | ||
| FileOperationError, | ||
| InvalidArgumentValueError, | ||
| RequiredArgumentMissingError, | ||
|
|
@@ -550,7 +551,40 @@ def __read_kv_from_file( | |
| except OSError: | ||
| raise FileOperationError("File is not available.") | ||
|
|
||
| flattened_data = __flatten_config_data( | ||
| config_data=config_data, | ||
| format_=format_, | ||
| content_type=content_type, | ||
| prefix_to_add=prefix_to_add, | ||
| depth=depth, | ||
| separator=separator | ||
| ) | ||
|
|
||
| # convert to KeyValue list | ||
| key_values = [] | ||
| for k, v in flattened_data.items(): | ||
| if validate_import_key(key=k): | ||
| key_values.append(KeyValue(key=k, value=v)) | ||
| return key_values | ||
|
|
||
|
|
||
| def __flatten_config_data(config_data, format_, content_type, prefix_to_add="", depth=None, separator=None): | ||
| """ | ||
| Flatten configuration data into a dictionary of key-value pairs. | ||
|
|
||
| Args: | ||
| config_data: The configuration data to flatten (dict or list) | ||
| format_ (str): The format of the configuration data ('json', 'yaml', 'properties') | ||
| content_type (str): Content type for JSON validation | ||
| prefix_to_add (str): Prefix to add to each key | ||
| depth (int): Maximum depth for flattening hierarchical data | ||
| separator (str): Separator for hierarchical keys | ||
|
|
||
| Returns: | ||
| dict: Flattened key-value pairs | ||
| """ | ||
| flattened_data = {} | ||
|
|
||
| if format_ == "json" and content_type and is_json_content_type(content_type): | ||
| for key in config_data: | ||
| __flatten_json_key_value( | ||
|
|
@@ -582,13 +616,7 @@ def __read_kv_from_file( | |
| separator=separator, | ||
| ) | ||
|
|
||
| # convert to KeyValue list | ||
| key_values = [] | ||
| for k, v in flattened_data.items(): | ||
| if validate_import_key(key=k): | ||
| key_values.append(KeyValue(key=k, value=v)) | ||
| return key_values | ||
|
|
||
| return flattened_data | ||
|
|
||
| # App Service <-> List of KeyValue object | ||
|
|
||
|
|
@@ -715,6 +743,170 @@ def __read_kv_from_app_service( | |
| raise CLIError("Failed to read key-values from appservice.\n" + str(exception)) | ||
|
|
||
|
|
||
| def __read_kv_from_kubernetes_configmap( | ||
| cmd, | ||
| aks_cluster, | ||
| configmap_name, | ||
| format_, | ||
| namespace="default", | ||
| prefix_to_add="", | ||
| content_type=None, | ||
| depth=None, | ||
| separator=None | ||
| ): | ||
| """ | ||
| Read key-value pairs from a Kubernetes ConfigMap using aks_runcommand. | ||
|
|
||
| Args: | ||
| cmd: The command context object | ||
| aks_cluster (str): Name of the AKS cluster | ||
| configmap_name (str): Name of the ConfigMap to read from | ||
| format_ (str): Format of the data in the ConfigMap (e.g., "json", "yaml") | ||
| namespace (str): Kubernetes namespace where the ConfigMap resides (default: "default") | ||
| prefix_to_add (str): Prefix to add to each key in the ConfigMap | ||
| content_type (str): Content type to apply to the key-values | ||
| depth (int): Maximum depth for flattening hierarchical data | ||
| separator (str): Separator for hierarchical keys | ||
|
|
||
| Returns: | ||
| list: List of KeyValue objects | ||
| """ | ||
| key_values = [] | ||
| from azure.cli.command_modules.acs.custom import aks_runcommand | ||
| from azure.cli.command_modules.acs._client_factory import cf_managed_clusters | ||
|
|
||
| # Preserve only the necessary CLI context data | ||
| original_subscription = cmd.cli_ctx.data.get('subscription_id') | ||
| original_safe_params = cmd.cli_ctx.data.get('safe_params', []) | ||
|
|
||
| try: | ||
| # Temporarily modify the CLI context | ||
| cmd.cli_ctx.data['subscription_id'] = aks_cluster["subscription"] | ||
|
Contributor
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. In the event where the store and cluster are in different subscriptions, and we don't restore the original subscription value, wouldn't subsequent calls within the command that depend on
Member
Author
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. The cli_ctx will not be used afterward since the appconfig client is already created before we modified the subscription. But from the function design perspective, this function should not have a side effect on the original data. I agree with your viewpoint, let's do the right thing. |
||
| params_to_keep = ["--debug", "--verbose"] | ||
| cmd.cli_ctx.data['safe_params'] = [p for p in original_safe_params if p in params_to_keep] | ||
| # It must be set to return the result. | ||
| cmd.cli_ctx.data['safe_params'].append("--output") | ||
| # Get the AKS client from the factory | ||
| aks_client = cf_managed_clusters(cmd.cli_ctx) | ||
|
|
||
| # Command to get the ConfigMap and output it as JSON | ||
| command = f"kubectl get configmap {configmap_name} -n {namespace} -o json" | ||
|
|
||
| # Execute the command on the cluster | ||
| result = aks_runcommand(cmd, aks_client, aks_cluster["resource_group"], aks_cluster["name"], command_string=command) | ||
|
|
||
| if hasattr(result, 'logs') and result.logs: | ||
| if not hasattr(result, 'exit_code') or result.exit_code == 0: | ||
| try: | ||
| configmap_data = json.loads(result.logs) | ||
|
|
||
| # Extract the data section which contains the key-value pairs | ||
| kvs = __extract_kv_from_configmap_data( | ||
| configmap_data, content_type, prefix_to_add, format_, depth, separator) | ||
|
|
||
| key_values.extend(kvs) | ||
| except json.JSONDecodeError: | ||
| raise ValueError( | ||
| f"The result from ConfigMap {configmap_name} could not be parsed. {result.logs.strip()}" | ||
| ) | ||
| else: | ||
| raise AzureResponseError(f"{result.logs.strip()}") | ||
| else: | ||
| raise AzureResponseError("Unable to get the ConfigMap.") | ||
|
Contributor
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. Just curious, what is returned when there is no data in the ConfigMap? Would it be
Member
Author
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. aks_runcommand result for configMap does not exist
Member
Author
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. aks_runcommand result for configMap with no data
Contributor
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. Thanks! |
||
|
|
||
| return key_values | ||
| except Exception as exception: | ||
| raise AzureInternalError( | ||
| f"Failed to read key-values from ConfigMap '{configmap_name}' in namespace '{namespace}'.\n{str(exception)}" | ||
| ) | ||
| finally: | ||
| # Restore original CLI context data | ||
| cmd.cli_ctx.data['subscription_id'] = original_subscription | ||
| cmd.cli_ctx.data['safe_params'] = original_safe_params | ||
|
|
||
|
|
||
| def __extract_kv_from_configmap_data(configmap, content_type, prefix_to_add="", format_=None, depth=None, separator=None): | ||
| """ | ||
| Helper function to extract key-value pairs from ConfigMap data. | ||
|
|
||
| Args: | ||
| configmap (dict): The ConfigMap data as a dictionary | ||
| prefix_to_add (str): Prefix to add to each key | ||
| content_type (str): Content type to apply to the key-values | ||
| format_ (str): Format of the data in the ConfigMap (e.g., "json", "yaml") | ||
| depth (int): Maximum depth for flattening hierarchical data | ||
| separator (str): Separator for hierarchical keys | ||
|
|
||
| Returns: | ||
| list: List of KeyValue objects | ||
| """ | ||
| key_values = [] | ||
|
|
||
| if not configmap.get('data', None): | ||
| logger.warning("ConfigMap exists but has no data") | ||
| return key_values | ||
|
|
||
| for key, value in configmap['data'].items(): | ||
| if format_ in ("json", "yaml", "properties"): | ||
| if format_ == "json": | ||
| try: | ||
| value = json.loads(value) | ||
| except json.JSONDecodeError: | ||
| logger.warning( | ||
| 'Value "%s" for key "%s" is not a well formatted JSON data.', | ||
| value, key | ||
| ) | ||
| continue | ||
| elif format_ == "yaml": | ||
| try: | ||
| value = yaml.safe_load(value) | ||
| except yaml.YAMLError: | ||
| logger.warning( | ||
| 'Value "%s" for key "%s" is not a well formatted YAML data.', | ||
| value, key | ||
| ) | ||
| continue | ||
| else: | ||
| try: | ||
| value = javaproperties.load(io.StringIO(value)) | ||
| except javaproperties.InvalidUEscapeError: | ||
| logger.warning( | ||
| 'Value "%s" for key "%s" is not a well formatted properties data.', | ||
| value, key | ||
| ) | ||
| continue | ||
|
|
||
| flattened_data = __flatten_config_data( | ||
| config_data=value, | ||
| format_=format_, | ||
| content_type=content_type, | ||
| prefix_to_add=prefix_to_add, | ||
| depth=depth, | ||
| separator=separator | ||
| ) | ||
|
|
||
| for k, v in flattened_data.items(): | ||
| if validate_import_key(key=k): | ||
| key_values.append(KeyValue(key=k, value=v)) | ||
|
|
||
| elif validate_import_key(key): | ||
| # If content_type is JSON, validate the value | ||
| if content_type and is_json_content_type(content_type): | ||
| try: | ||
| json.loads(value) | ||
| except json.JSONDecodeError: | ||
| logger.warning( | ||
| 'Value "%s" for key "%s" is not a valid JSON object, which conflicts with the provided content type "%s".', | ||
| value, key, content_type | ||
| ) | ||
| continue | ||
|
|
||
| kv = KeyValue(key=prefix_to_add + key, value=value) | ||
| key_values.append(kv) | ||
|
|
||
| return key_values | ||
|
|
||
|
|
||
| def __validate_import_keyvault_ref(kv): | ||
| if kv and validate_import_key(kv.key): | ||
| try: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,8 @@ | |
| get_default_location_from_resource_group | ||
| from ._constants import ImportExportProfiles, ImportMode, FeatureFlagConstants, ARMAuthenticationMode | ||
|
|
||
| from ._validators import (validate_appservice_name_or_id, validate_sku, validate_snapshot_query_fields, | ||
| from ._validators import (validate_appservice_name_or_id, validate_aks_cluster_name_or_id, | ||
| validate_sku, validate_snapshot_query_fields, | ||
| validate_connection_string, validate_datetime, | ||
| validate_export, validate_import, | ||
| validate_import_depth, validate_query_fields, | ||
|
|
@@ -232,7 +233,7 @@ def load_arguments(self, _): | |
| c.argument('label', help="Imported KVs and feature flags will be assigned with this label. If no label specified, will assign null label.") | ||
| c.argument('tags', nargs="*", help="Imported KVs and feature flags will be assigned with these tags. If no tags are specified, imported KVs and feature flags will retain existing tags. Support space-separated tags: key[=value] [key[=value] ...]. Use "" to clear existing tags.") | ||
| c.argument('prefix', help="This prefix will be appended to the front of imported keys. Prefix will be ignored for feature flags.") | ||
| c.argument('source', options_list=['--source', '-s'], arg_type=get_enum_type(['file', 'appconfig', 'appservice']), validator=validate_import, help="The source of importing. Note that importing feature flags from appservice is not supported.") | ||
| c.argument('source', options_list=['--source', '-s'], arg_type=get_enum_type(['file', 'appconfig', 'appservice', 'aks']), validator=validate_import, help="The source of importing. Note that importing feature flags from appservice is not supported.") | ||
| c.argument('yes', help="Do not prompt for preview.") | ||
| c.argument('skip_features', help="Import only key values and exclude all feature flags. By default, all feature flags will be imported from file or appconfig. Not applicable for appservice.", arg_type=get_three_state_flag()) | ||
| c.argument('content_type', help='Content type of all imported items.') | ||
|
|
@@ -264,6 +265,11 @@ def load_arguments(self, _): | |
| with self.argument_context('appconfig kv import', arg_group='AppService') as c: | ||
| c.argument('appservice_account', validator=validate_appservice_name_or_id, help='ARM ID for AppService OR the name of the AppService, assuming it is in the same subscription and resource group as the App Configuration store. Required for AppService arguments') | ||
|
|
||
| with self.argument_context('appconfig kv import', arg_group='AKS') as c: | ||
| c.argument('aks_cluster', validator=validate_aks_cluster_name_or_id, help='ARM ID for AKS OR the name of the AKS, assuming it is in the same subscription and resource group as the App Configuration store. Required for AKS arguments') | ||
| c.argument('configmap_name', help='Name of the ConfigMap. Required for AKS arguments.') | ||
|
||
| c.argument('configmap_namespace', help='Namespace of the ConfigMap. default to "default" namespace if not specified.') | ||
|
|
||
| with self.argument_context('appconfig kv export') as c: | ||
| c.argument('label', help="Only keys and feature flags with this label will be exported. If no label specified, export keys and feature flags with null label by default. When export destination is appconfig, or when export destination is file with `appconfig/kvset` profile, this argument supports asterisk and comma signs for label filtering, for instance, * means all labels, abc* means labels with abc as prefix, and abc,xyz means labels with abc or xyz.") | ||
| c.argument('prefix', help="Prefix to be trimmed from keys. Prefix will be ignored for feature flags.") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ | |
| __delete_configuration_setting_from_config_store, | ||
| __read_features_from_file, | ||
| __read_kv_from_app_service, | ||
| __read_kv_from_kubernetes_configmap, | ||
| __read_kv_from_file, | ||
| ) | ||
| from knack.log import get_logger | ||
|
|
@@ -92,7 +93,11 @@ def import_config(cmd, | |
| src_endpoint=None, | ||
| src_tags=None, # tags to filter | ||
| # from-appservice parameters | ||
| appservice_account=None): | ||
| appservice_account=None, | ||
| # from-aks parameters | ||
| aks_cluster=None, | ||
| configmap_namespace="default", | ||
| configmap_name=None): | ||
|
|
||
| src_features = [] | ||
| dest_features = [] | ||
|
|
@@ -176,6 +181,25 @@ def import_config(cmd, | |
| src_kvs = __read_kv_from_app_service( | ||
| cmd, appservice_account=appservice_account, prefix_to_add=prefix, content_type=content_type) | ||
|
|
||
| elif source == 'aks': | ||
| if separator: | ||
| # If separator is provided, use max depth by default unless depth is specified. | ||
|
Contributor
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. Should we let a user know this that we use max depth by default if depth is not provided maybe somewhere in the params.py?
Member
Author
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. Do you mean this?
Contributor
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. Yes this works! Hadn't seen it. |
||
| depth = sys.maxsize if depth is None else int(depth) | ||
| else: | ||
| if depth and int(depth) != 1: | ||
| logger.warning("Cannot flatten hierarchical data without a separator. --depth argument will be ignored.") | ||
| depth = 1 | ||
|
|
||
| src_kvs = __read_kv_from_kubernetes_configmap(cmd, | ||
| aks_cluster=aks_cluster, | ||
| configmap_name=configmap_name, | ||
| namespace=configmap_namespace, | ||
| prefix_to_add=prefix, | ||
| content_type=content_type, | ||
| format_=format_, | ||
| separator=separator, | ||
| depth=depth) | ||
|
|
||
| if strict or not yes or import_mode == ImportMode.IGNORE_MATCH: | ||
| # fetch key values from user's configstore | ||
| dest_kvs = __read_kv_from_config_store(azconfig_client, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Does this mean we are allowing the imports from a cluster in a different subscription? I would expect this to be scoped to the user's subscription.
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.
Why should we limit it to the user's subscription?
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.
Actually, I think my comment can be ignored.
I was initially under the impression that we only take cluster name as input, but user can also pass the ARM ID , similar to app service. So I guess there need for the restriction.
Thanks for calling this out @ChristineWanjau !