Pack config: Process defaults and datastore values in patternProperties and additionalItems

CHANGELOG.rst

@@ -185,6 +185,12 @@ Added
 
   Contributed by @nzlosh
 
+* Fix a bug in the pack config loader so that objects covered by an ``patternProperties`` schema
+  or arrays using ``additionalItems`` schema(s) can use encrypted datastore keys and have their
+  default values applied correctly. #5321
+
+  Contributed by @cognifloyd.
+
 Changed
 ~~~~~~~
 

st2common/st2common/util/config_loader.py

@@ -15,6 +15,7 @@
 
 from __future__ import absolute_import
 import copy
+import re
 
 import six
 
@@ -101,7 +102,8 @@ def _get_values_for_config(self, config_schema_db, config_db):
     @staticmethod
     def _get_object_property_schema(object_schema, additional_properties_keys=None):
         """
-        Create a schema for an object property using both additionalProperties and properties.
+        Create a schema for an object property using all of: properties,
+        patternProperties, and additionalProperties.
 
         :rtype: ``dict``
         """
@@ -112,9 +114,56 @@ def _get_object_property_schema(object_schema, additional_properties_keys=None):
             # ensure that these keys are present in the object
             for key in additional_properties_keys:
                 property_schema[key] = additional_properties
-        property_schema.update(object_schema.get("properties", {}))
+
+        properties_schema = object_schema.get("properties", {})
+        property_schema.update(properties_schema)
+
+        potential_patterned_keys = set(additional_properties_keys) - set(
+            properties_schema.keys()
+        )
+
+        pattern_properties = object_schema.get("patternProperties", {})
+        # patternProperties can be a boolean or a dict
+        if pattern_properties and isinstance(pattern_properties, dict):
+            # update any matching key
+            for raw_pattern, pattern_schema in pattern_properties.items():
+                if not potential_patterned_keys:
+                    # nothing to check. Don't compile any more patterns
+                    break
+                pattern = re.compile(raw_pattern)
+                for key in list(potential_patterned_keys):
+                    if pattern.search(key):
+                        property_schema[key] = pattern_schema
+                        potential_patterned_keys.remove(key)
         return property_schema
 
+    @staticmethod
+    def _get_array_items_schema(object_schema, items_count=0):
+        """
+        Create a schema for array items using both additionalItems and items.
+
+        :rtype: ``list``
+        """
+        items_schema = []
+        object_items_schema = object_schema.get("items", [])
+        if isinstance(object_items_schema, dict):
+            items_schema.extend([object_items_schema] * items_count)
+        else:
+            items_schema.extend(object_items_schema)
+
+        items_schema_count = len(items_schema)
+        if items_schema_count >= items_count:
+            # no additional items to account for.
+            return items_schema
+
+        additional_items = object_schema.get("additionalItems", {})
+        # additionalItems can be a boolean or a dict
+        if additional_items and isinstance(additional_items, dict):
+            # ensure that these keys are present in the object
+            items_schema.extend([additional_items] * (items_count - items_schema_count))
+
+        return items_schema
+
     def _assign_dynamic_config_values(self, schema, config, parent_keys=None):
         """
         Assign dynamic config value for a particular config item if the ite utilizes a Jinja
@@ -137,7 +186,13 @@ def _assign_dynamic_config_values(self, schema, config, parent_keys=None):
             if config_is_dict:
                 # different schema for each key/value pair
                 schema_item = schema.get(config_item_key, {})
-            if config_is_list:
+            if config_is_list and isinstance(schema, list):
+                # positional schema for list items
+                try:
+                    schema_item = schema[config_item_key]
+                except IndexError:
+                    schema_item = {}
+            elif config_is_list:
                 # same schema is shared between every item in the list
                 schema_item = schema
 
@@ -160,8 +215,12 @@ def _assign_dynamic_config_values(self, schema, config, parent_keys=None):
                 )
             # Inspect nested list items
             elif is_list:
+                items_schema = self._get_array_items_schema(
+                    schema_item,
+                    items_count=len(config[config_item_key]),
+                )
                 self._assign_dynamic_config_values(
-                    schema=schema_item.get("items", {}),
+                    schema=items_schema,
                     config=config[config_item_key],
                     parent_keys=current_keys,
                 )
@@ -193,35 +252,72 @@ def _assign_default_values(self, schema, config):
 
         Note: This method mutates config argument in place.
 
-        :rtype: ``dict``
+        :rtype: ``dict|list``
         """
-        for schema_item_key, schema_item in six.iteritems(schema):
+        schema_is_dict = isinstance(schema, dict)
+        iterator = schema.items() if schema_is_dict else enumerate(schema)
+
+        # _get_*_schema ensures that schema_item is always a dict
+        for schema_item_key, schema_item in iterator:
             has_default_value = "default" in schema_item
-            has_config_value = schema_item_key in config
+            if isinstance(config, dict):
+                has_config_value = schema_item_key in config
+            else:
+                has_config_value = schema_item_key < len(config)
 
             default_value = schema_item.get("default", None)
-            is_object = schema_item.get("type", None) == "object"
-            has_properties = schema_item.get("properties", None)
-            has_additional_properties = schema_item.get("additionalProperties", None)
-
             if has_default_value and not has_config_value:
                 # Config value is not provided, but default value is, use a default value
                 config[schema_item_key] = default_value
 
-            # Inspect nested object properties
-            if is_object and (has_properties or has_additional_properties):
-                if not config.get(schema_item_key, None):
-                    config[schema_item_key] = {}
+            try:
+                config_value = config[schema_item_key]
+            except (KeyError, IndexError):
+                config_value = None
 
-                property_schema = self._get_object_property_schema(
-                    schema_item,
-                    additional_properties_keys=config[schema_item_key].keys(),
-                )
+            schema_item_type = schema_item.get("type", None)
 
-                self._assign_default_values(
-                    schema=property_schema, config=config[schema_item_key]
+            if schema_item_type == "object":
+                has_properties = schema_item.get("properties", None)
+                has_pattern_properties = schema_item.get("patternProperties", None)
+                has_additional_properties = schema_item.get(
+                    "additionalProperties", None
                 )
 
+                # Inspect nested object properties
+                if (
+                    has_properties
+                    or has_pattern_properties
+                    or has_additional_properties
+                ):
+                    if not config_value:
+                        config_value = config[schema_item_key] = {}
+
+                    property_schema = self._get_object_property_schema(
+                        schema_item,
+                        additional_properties_keys=config_value.keys(),
+                    )
+
+                    self._assign_default_values(
+                        schema=property_schema, config=config_value
+                    )
+            elif schema_item_type == "array":
+                has_items = schema_item.get("items", None)
+                has_additional_items = schema_item.get("additionalItems", None)
+
+                # Inspect nested array items
+                if has_items or has_additional_items:
+                    if not config_value:
+                        config_value = config[schema_item_key] = []
+
+                    items_schema = self._get_array_items_schema(
+                        schema_item,
+                        items_count=len(config_value),
+                    )
+                    self._assign_default_values(
+                        schema=items_schema, config=config_value
+                    )
+
         return config
 
     def _get_datastore_value_for_expression(self, key, value, config_schema_item=None):

st2common/tests/unit/test_config_loader.py

@@ -575,6 +575,116 @@ def test_get_config_dynamic_config_item_under_additional_properties(self):
 
         config_db.delete()
 
+    def test_get_config_dynamic_config_item_under_pattern_properties(self):
+        pack_name = "dummy_pack_schema_with_pattern_properties_1"
+        loader = ContentPackConfigLoader(pack_name=pack_name)
+
+        encrypted_value = crypto.symmetric_encrypt(
+            KeyValuePairAPI.crypto_key, "v1_encrypted"
+        )
+        KeyValuePair.add_or_update(
+            KeyValuePairDB(name="k1_encrypted", value=encrypted_value, secret=True)
+        )
+
+        ####################
+        # values in objects under an object with additionalProperties
+        values = {
+            "profiles": {
+                "dev": {
+                    # no host or port to test default value
+                    "token": "hard-coded-secret",
+                },
+                "prod": {
+                    "host": "127.1.2.7",
+                    "port": 8282,
+                    # encrypted in datastore
+                    "token": "{{st2kv.system.k1_encrypted}}",
+                    # schema declares `secret: true` which triggers auto-decryption.
+                    # If this were not encrypted, it would try to decrypt it and fail.
+                },
+            }
+        }
+        config_db = ConfigDB(pack=pack_name, values=values)
+        config_db = Config.add_or_update(config_db)
+
+        config_rendered = loader.get_config()
+
+        self.assertEqual(
+            config_rendered,
+            {
+                "region": "us-east-1",
+                "profiles": {
+                    "dev": {
+                        "host": "127.0.0.3",
+                        "port": 8080,
+                        "token": "hard-coded-secret",
+                    },
+                    "prod": {
+                        "host": "127.1.2.7",
+                        "port": 8282,
+                        "token": "v1_encrypted",
+                    },
+                },
+            },
+        )
+
+        config_db.delete()
+
+    def test_get_config_dynamic_config_item_under_additional_items(self):
+        pack_name = "dummy_pack_schema_with_additional_items_1"
+        loader = ContentPackConfigLoader(pack_name=pack_name)
+
+        encrypted_value = crypto.symmetric_encrypt(
+            KeyValuePairAPI.crypto_key, "v1_encrypted"
+        )
+        KeyValuePair.add_or_update(
+            KeyValuePairDB(name="k1_encrypted", value=encrypted_value, secret=True)
+        )
+
+        ####################
+        # values in objects under an object with additionalProperties
+        values = {
+            "profiles": [
+                {
+                    # no host or port to test default value
+                    "token": "hard-coded-secret",
+                },
+                {
+                    "host": "127.1.2.7",
+                    "port": 8282,
+                    # encrypted in datastore
+                    "token": "{{st2kv.system.k1_encrypted}}",
+                    # schema declares `secret: true` which triggers auto-decryption.
+                    # If this were not encrypted, it would try to decrypt it and fail.
+                },
+            ]
+        }
+        config_db = ConfigDB(pack=pack_name, values=values)
+        config_db = Config.add_or_update(config_db)
+
+        config_rendered = loader.get_config()
+
+        self.assertEqual(
+            config_rendered,
+            {
+                "region": "us-east-1",
+                "profiles": [
+                    {
+                        "host": "127.0.0.3",
+                        "port": 8080,
+                        "token": "hard-coded-secret",
+                    },
+                    {
+                        "host": "127.1.2.7",
+                        "port": 8282,
+                        "token": "v1_encrypted",
+                    },
+                ],
+            },
+        )
+
+        config_db.delete()
+
     def test_empty_config_object_in_the_database(self):
         pack_name = "dummy_pack_empty_config"
 

st2tests/st2tests/fixtures/packs/dummy_pack_schema_with_additional_items_1/config.schema.yaml

@@ -0,0 +1,24 @@
+---
+  region:
+    type: "string"
+    required: false
+    default: "us-east-1"
+  profiles:
+    type: "array"
+    required: false
+    additionalItems:
+      type: object
+      additionalProperties: false
+      properties:
+        host:
+          type: "string"
+          required: false
+          default: "127.0.0.3"
+        port:
+          type: "integer"
+          required: false
+          default: 8080
+        token:
+          type: "string"
+          required: true
+          secret: true

st2tests/st2tests/fixtures/packs/dummy_pack_schema_with_additional_items_1/pack.yaml

@@ -0,0 +1,6 @@
+---
+name : dummy_pack_schema_with_additional_items_1
+description : dummy pack with nested objects under additionalItems
+version : 0.1.0
+author : st2-dev
+email : [email protected]

st2tests/st2tests/fixtures/packs/dummy_pack_schema_with_pattern_properties_1/config.schema.yaml

@@ -0,0 +1,25 @@
+---
+  region:
+    type: "string"
+    required: false
+    default: "us-east-1"
+  profiles:
+    type: "object"
+    required: false
+    patternProperties:
+      "^\\w+$":
+        type: object
+        additionalProperties: false
+        properties:
+          host:
+            type: "string"
+            required: false
+            default: "127.0.0.3"
+          port:
+            type: "integer"
+            required: false
+            default: 8080
+          token:
+            type: "string"
+            required: true
+            secret: true

st2tests/st2tests/fixtures/packs/dummy_pack_schema_with_pattern_properties_1/pack.yaml

@@ -0,0 +1,6 @@
+---
+name : dummy_pack_schema_with_pattern_properties_1
+description : dummy pack with nested objects under patternProperties
+version : 0.1.0
+author : st2-dev
+email : [email protected]