Default values in OpenAPI, patternProperties are not properly handled

[piotrminkina]

Expected behavior (what you expected to happen):
The configuration is validated correctly. The default values for the fields in the object defined as patternProperties should be in the file pointed to by the VALUES_PATH variable.

Actual behavior (what actually happened):
The configuration is validated correctly. The default values for the fields in the object defined as patternProperties are not available in the file pointed to by the VALUES_PATH variable.

Steps to reproduce:

1. Create ConfigMap with the following content:

   apiVersion: v1
   kind: ConfigMap
   metadata:
     # ...
   data:
     global: |
       parent: {}

2. Create a file /global-hooks/openapi/config-values.yaml with the following content:

   type: object
   additionalProperties: false
   patternProperties:
     ^parent$:
       type: object
       required:
         - child
       properties:
         child:
           type: object
           default: {} # Deleting this field causes the global configuration to be invalid, as expected,

3. Create a executable hook file /global-hooks/check-if-child-object-exists.bash with the following content:

   #!/usr/bin/env bash
   
   declare VALUES_PATH
   declare BINDING_CONTEXT_PATH
   
   if [[ "${1:-}" == "--config" ]]; then
       cat <<EOF
       {
           "configVersion": "v1",
           "beforeAll": 1
       }
   EOF
       exit 0
   fi
   
   declare binding="$(jq -r '.[0].binding' "${BINDING_CONTEXT_PATH}")"
   
   if [[ "${binding}" == 'beforeAll' ]]; then
       jq '.global.parent | if .child != null then "Child Object exists! " + (.child | tostring) else "Child Object does NOT exist!" end' < "${VALUES_PATH}"
   fi

4. You should see the msg="\"Child Object exists! {}\"" message in the console log, but you will see msg="\"Child Object does NOT exist!\"".

Environment:

• Addon-operator version: v1.1.2
• Helm version: v3.10.3+g835b733
• Kubernetes version: v1.23.6+k3s1
• Installation type (kubectl apply, helm chart, etc.): Own Helm Chart

[diafour]

First of all, I suggest not to use default for the required property. It makes no sense for the user: if property is marked as required, it has to be set by the user in the ConfigMap. There is an issue deckhouse/deckhouse#2987 that inspired a long discussion about default/required contract. We came to a conclusion that required property should not have a default value.

Second, it seems you think of defaults in JSON schema as a fillers for missing values in the ConfigMap. In short, these defaults are default values that user may override in the ConfigMap. Addon-operator instantiates defaults starting from the empty map before merging ConfigMap, so it can't support patternProperties by design, see ApplyDefault and moduleManager.GlobalValues.

Overall you've faced a triple combo:

• addon-operator can't instantiate defaults for patternProperties on an empty map
• validation process instantiates defaults: {} for the child and treats parent: {} as valid
• merged values in VALUES_PATH have no child field.

The workaround is to define a default for the root object if child is optional or remove a default if child is required:

type: object
additionalProperties: false
default:
  parent:
    child: {}
patternProperties:
  ^parent$:
    type: object
    properties:
      child:
        type: object

type: object
additionalProperties: false
patternProperties:
  ^parent$:
    type: object
    required:
      - child
    properties:
      child:
        type: object