[App Config] az appconfig kv set/import: Add support for JSON comments (#32130)

Author: albertofori

src/azure-cli/azure/cli/command_modules/appconfig/_constants.py

@@ -63,6 +63,10 @@ class KeyVaultConstants:
     KEYVAULT_CONTENT_TYPE = "application/vnd.microsoft.appconfig.keyvaultref+json;charset=utf-8"
 
 
+class AIConfigConstants:
+    AI_CHAT_COMPLETION_CONTENT_TYPE = "application/vnd.microsoft.appconfig.aichatcompletion+json;charset=utf-8"
+
+
 class AppServiceConstants:
     APPSVC_CONFIG_REFERENCE_PREFIX = "@Microsoft.AppConfiguration"
     APPSVC_KEYVAULT_PREFIX = "@Microsoft.KeyVault"

src/azure-cli/azure/cli/command_modules/appconfig/_json.py

@@ -0,0 +1,96 @@
+# --------------------------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for license information.
+# --------------------------------------------------------------------------------------------
+
+import json
+
+
+DOUBLE_QUOTE = '\"'
+BACKSLASH = '\\'
+DOUBLE_SLASH = '//'
+MULTILINE_COMMENT_START = '/*'
+MULTILINE_COMMENT_END = '*/'
+NEW_LINE = '\n'
+
+
+def parse_json_with_comments(json_string):
+    try:
+        return json.loads(json_string)
+
+    except json.JSONDecodeError:
+        return json.loads(__strip_json_comments(json_string))
+
+
+def __strip_json_comments(json_string):
+    current_index = 0
+    length = len(json_string)
+    result = []
+
+    while current_index < length:
+        # Single line comment
+        if json_string[current_index:current_index + 2] == DOUBLE_SLASH:
+            current_index = __find_next_newline_index(json_string, current_index, length)
+            if current_index < length and json_string[current_index] == NEW_LINE:
+                result.append(NEW_LINE)
+
+        # Multi-line comment
+        elif json_string[current_index:current_index + 2] == MULTILINE_COMMENT_START:
+            current_index = __find_next_multiline_comment_end(json_string, current_index + 2, length)
+
+        # String literal
+        elif json_string[current_index] == DOUBLE_QUOTE:
+            literal_start_index = current_index
+            current_index = __find_next_double_quote_index(json_string, current_index + 1, length)
+
+            result.extend(json_string[literal_start_index:current_index + 1])
+        else:
+            result.append(json_string[current_index])
+
+        current_index += 1
+
+    return "".join(result)
+
+
+def __is_escaped(json_string, char_index):
+    backslash_count = 0
+    index = char_index - 1
+    while index >= 0 and json_string[index] == BACKSLASH:
+        backslash_count += 1
+        index -= 1
+
+    return backslash_count % 2 == 1
+
+
+def __find_next_newline_index(json_string, start_index, end_index):
+    index = start_index
+
+    while index < end_index:
+        if json_string[index] == NEW_LINE:
+            return index
+
+        index += 1
+
+    return index
+
+
+def __find_next_double_quote_index(json_string, start_index, end_index):
+    index = start_index
+    while index < end_index:
+        if json_string[index] == DOUBLE_QUOTE and not __is_escaped(json_string, index):
+            return index
+
+        index += 1
+
+    raise ValueError("Unterminated string literal")
+
+
+def __find_next_multiline_comment_end(json_string, start_index, end_index):
+    index = start_index
+    while index < end_index - 1:
+        if json_string[index:index + 2] == MULTILINE_COMMENT_END:
+            return index + 1
+
+        index += 1
+
+    raise ValueError("Unterminated multi-line comment")

src/azure-cli/azure/cli/command_modules/appconfig/_kv_import_helpers.py

@@ -47,6 +47,7 @@
     ImportMode,
 )
 from ._diff_utils import KVComparer, print_preview
+from ._json import parse_json_with_comments
 from ._utils import (
     is_json_content_type,
     validate_feature_flag_name,
@@ -87,7 +88,7 @@ def __read_with_appropriate_encoding(file_path, format_):
     try:
         with io.open(file_path, "r", encoding=default_encoding) as config_file:
             if format_ == "json":
-                config_data = json.load(config_file)
+                config_data = parse_json_with_comments(config_file.read())
                 # Only accept json objects
                 if not isinstance(config_data, (dict, list)):
                     raise ValueError(
@@ -112,7 +113,7 @@ def __read_with_appropriate_encoding(file_path, format_):
 
         with io.open(file_path, "r", encoding=detected_encoding) as config_file:
             if format_ == "json":
-                config_data = json.load(config_file)
+                config_data = parse_json_with_comments(config_file.read())
 
             elif format_ == "yaml":
                 for yaml_data in list(yaml.safe_load_all(config_file)):

src/azure-cli/azure/cli/command_modules/appconfig/keyvalue.py

@@ -35,7 +35,6 @@
 )
 from knack.log import get_logger
 from knack.util import CLIError
-from ._constants import HttpHeaders
 
 from azure.appconfiguration import (ConfigurationSetting,
                                     ResourceReadOnlyError)
@@ -50,8 +49,10 @@
 from ._constants import (FeatureFlagConstants, KeyVaultConstants,
                          SearchFilterOptions, StatusCodes,
                          ImportExportProfiles, CompareFieldsMap,
-                         JsonDiff, ImportMode)
+                         JsonDiff, ImportMode,
+                         AIConfigConstants, HttpHeaders)
 from ._featuremodels import map_keyvalue_to_featureflag
+from ._json import parse_json_with_comments
 from ._models import (convert_configurationsetting_to_keyvalue, convert_keyvalue_to_configurationsetting)
 from ._utils import get_appconfig_data_client, prep_filter_for_url_encoding, resolve_store_metadata, get_store_endpoint_from_connection_string, is_json_content_type
 
@@ -507,9 +508,10 @@ def set_key(cmd,
         if retrieved_kv is None:
             if is_json_content_type(content_type):
                 try:
-                    # Ensure that provided value is valid JSON. Error out if value is invalid JSON.
+                    # Ensure that provided value is valid JSON and strip comments if needed.
                     value = 'null' if value is None else value
-                    json.loads(value)
+
+                    __validate_json_value(value, content_type)
                 except ValueError:
                     raise CLIErrors.ValidationError('Value "{}" is not a valid JSON object, which conflicts with the content type "{}".'.format(value, content_type))
 
@@ -523,8 +525,8 @@ def set_key(cmd,
             content_type = retrieved_kv.content_type if content_type is None else content_type
             if is_json_content_type(content_type):
                 try:
-                    # Ensure that provided/existing value is valid JSON. Error out if value is invalid JSON.
-                    json.loads(value)
+                    # Ensure that provided value is valid JSON and strip comments if needed.
+                    __validate_json_value(value, content_type)
                 except (TypeError, ValueError):
                     raise CLIErrors.ValidationError('Value "{}" is not a valid JSON object, which conflicts with the content type "{}". Set the value again in valid JSON format.'.format(value, content_type))
             set_kv = ConfigurationSetting(key=key,
@@ -984,3 +986,12 @@ def list_revision(cmd,
         return retrieved_revisions
     except HttpResponseError as ex:
         raise CLIErrors.AzureResponseError('List revision operation failed.\n' + str(ex))
+
+
+def __validate_json_value(json_string, content_type):
+    # We do not allow comments in keyvault references, feature flags, and AI chat completion configs
+    if content_type in (FeatureFlagConstants.FEATURE_FLAG_CONTENT_TYPE, KeyVaultConstants.KEYVAULT_CONTENT_TYPE, AIConfigConstants.AI_CHAT_COMPLETION_CONTENT_TYPE):
+        json.loads(json_string)
+
+    else:
+        parse_json_with_comments(json_string)

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/json_with_comments.json

@@ -0,0 +1,76 @@
+{ // Test single-line backslash comment
+  "Array": [
+    "StringBoolValue",
+    "true",
+    "StringNullValue",
+    "null",
+    "StringNumberValue",
+    "20.2"
+  ],
+  /* Test multi-line comment
+     spanning multiple lines
+  */
+  "ComplexSettings": {
+    "MyHybridList": [
+      1000,
+      true,
+      "\"EscapedString\"",
+      "AppConfig string value",
+      [
+        "NestedArray",
+        12,
+        false,
+        null
+      ],
+      {
+        "ObjectSetting": {
+          "Logging": {
+            "LogLevel": "Information",
+            "Default": "Debug"
+          }
+        }
+      }
+    ],
+// Feature management settings
+    "FeatureManagement": {
+      "Beta": {
+        "EnabledFor": [
+          {
+            "Name": "1",
+            "Parameters": {
+              "q": true,
+              "w": 243,
+              "e": null,
+              "r": [
+                1,
+                2,
+                3
+              ],
+              "t": [
+                "a",
+                "b",
+                "c"
+              ],
+              "y": {
+                "Name": "Value"
+              },
+              "u": "string"
+            }
+          },
+          {
+            "Name": "1"
+          },
+          {
+            "Name": "2",
+            "Parameters": {
+              "a": "ss",
+              "s": 21
+            }
+          }
+        ]
+      },
+      "DisabledFeature": false,
+      "EnabledFeature": true
+    }
+  }
+}

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/json_with_comments_stripped.json

@@ -0,0 +1,72 @@
+{
+  "Array": [
+    "StringBoolValue",
+    "true",
+    "StringNullValue",
+    "null",
+    "StringNumberValue",
+    "20.2"
+  ],
+  "ComplexSettings": {
+    "MyHybridList": [
+      1000,
+      true,
+      "\"EscapedString\"",
+      "AppConfig string value",
+      [
+        "NestedArray",
+        12,
+        false,
+        null
+      ],
+      {
+        "ObjectSetting": {
+          "Logging": {
+            "LogLevel": "Information",
+            "Default": "Debug"
+          }
+        }
+      }
+    ],
+    "FeatureManagement": {
+      "Beta": {
+        "EnabledFor": [
+          {
+            "Name": "1",
+            "Parameters": {
+              "q": true,
+              "w": 243,
+              "e": null,
+              "r": [
+                1,
+                2,
+                3
+              ],
+              "t": [
+                "a",
+                "b",
+                "c"
+              ],
+              "y": {
+                "Name": "Value"
+              },
+              "u": "string"
+            }
+          },
+          {
+            "Name": "1"
+          },
+          {
+            "Name": "2",
+            "Parameters": {
+              "a": "ss",
+              "s": 21
+            }
+          }
+        ]
+      },
+      "DisabledFeature": false,
+      "EnabledFeature": true
+    }
+  }
+}