cocreators-ee / project-template

Project template for kick-starting your work the right way
Other
5 stars 7 forks source link

Incorrect merge for arrays #15

Closed fbjorn closed 4 years ago

fbjorn commented 4 years ago

It's better to describe by a simple example:


CONTAINER_ENVS = """
containers:
- env:
    - name: MONGODB_OPTIONS
      valueFrom:
        configMapKeyRef:
          name: app-config
          key: mongodb_options
"""

CONTAINER_ENVS_OVERRIDES = """
containers:
- env:
    - name: PRODUCT_KEY
      valueFrom:
        secretKeyRef:
          name: app-secrets
          key: product_key
"""

CONTAINER_ENVS_EXPECTED = """
containers:
- env:
    - name: MONGODB_OPTIONS
      valueFrom:
        configMapKeyRef:
          name: app-config
          key: mongodb_options
    - name: PRODUCT_KEY
      valueFrom:
        secretKeyRef:
          name: app-secrets
          key: product_key
"""

def test_merge_container_env_list():
    docs = list(yaml.load_all(StringIO(CONTAINER_ENVS), yaml.Loader))
    overrides = list(yaml.load_all(StringIO(CONTAINER_ENVS_OVERRIDES), yaml.BaseLoader))
    expected = list(yaml.load_all(StringIO(CONTAINER_ENVS_EXPECTED), yaml.Loader))

    merged = merge_docs(docs, overrides)
    for i, merged_doc in enumerate(merged):
        expected_doc = expected[i]

        merged_json = json.dumps(merged_doc, **JSON_FORMAT)
        expected_json = json.dumps(expected_doc, **JSON_FORMAT)

        print(f"Doc {i + 1} expected: {expected_json}")
        print(f"Doc {i + 1} actual  : {merged_json}")

        assert merged_json == expected_json
test_utils.py::test_merge_container_env_list FAILED                      [100%]Doc 1 expected: {
  "containers": [
    {
      "env": [
        {
          "name": "MONGODB_OPTIONS", 
          "valueFrom": {
            "configMapKeyRef": {
              "key": "mongodb_options", 
              "name": "app-config"
            }
          }
        }, 
        {
          "name": "PRODUCT_KEY", 
          "valueFrom": {
            "secretKeyRef": {
              "key": "product_key", 
              "name": "app-secrets"
            }
          }
        }
      ]
    }
  ]
}
Doc 1 actual  : {
  "containers": [
    {
      "env": [
        {
          "name": "PRODUCT_KEY", 
          "valueFrom": {
            "configMapKeyRef": {
              "key": "mongodb_options", 
              "name": "app-config"
            }, 
            "secretKeyRef": {
              "key": "product_key", 
              "name": "app-secrets"
            }
          }
        }
      ]
    }
  ]
}
lietu commented 4 years ago

Your assumption is just incorrect, this is behavior described in https://github.com/Lieturd/project-template#yaml-merges

The merge system would have to understand the intricate details of the identifiers in the arrays for it to know which one to choose, so it doesn't assume, instead it expects YOU to tell it which entry in the array it will merge to.

This overrides the first item

array:
  - item

This adds a second item

array:
  - 
  - item
lietu commented 4 years ago

Actually the issue was with dicts, deleting previous entries in a dict is plain YAML & assumed by Kubernetes to work in the standard (yet a bit odd) way:

key: ~