[keycloak] Fix destructive shallow merge of JSON data

[chrisberkhout]

Proposed commit message

[keycloak] Fix destructive shallow merge of JSON data

This stops some top-level metadata being overwritten by partial data
from the JSON object.

Below is a shell script to run tests on the new Painless function
mergeMaps.

---

: "${ES_URL:=https://localhost:9200}"
: "${ES_USER:=elastic}"
: "${ES_PASSWORD:=changeme}"

painless=$(cat <<'PAINLESS'
def mergeMaps(Map map1, Map map2) {
  for (def key : map2.keySet()) {
    if (!map1.containsKey(key)
        || map1[key] == null
        || map1[key] == ""
        || (map1[key] instanceof Map && map1[key].isEmpty())) {
      map1[key] = map2[key];
    } else if (map1[key] != map2[key]) {
      if (map1[key] instanceof Map && map2[key] instanceof Map) {
        map1[key] = mergeMaps(map1[key], map2[key]);
      } else if (map1[key] instanceof List) {
        def combined = new LinkedHashSet(map1[key]);
        if (map2[key] instanceof List) {
          combined.addAll(map2[key]);
        } else if (map2[key] != null) {
          combined.add(map2[key]);
        }
        map1[key] = new ArrayList(combined);
      }
    }
  }
  return map1;
}

def runTests(List tests) {
  def results = new ArrayList();
  for (def t : tests) {
    def got = mergeMaps(t["map1"], t["map2"]);
    results.add([
      "test": t["name"],
      "passed": got.equals(t["expect"]),
      "got": got,
      "expected": t["expect"]
    ]);
  }
  def passed = new ArrayList();
  def failed = new ArrayList();
  for (def r : results) {
    if (r["passed"]) passed.add(r);
    else failed.add(r);
  }
  def result = new LinkedHashMap();
  result.put("passed_all", passed.size() == tests.size());
  result.put("passed_count", passed.size());
  result.put("failed_count", failed.size());
  result.put("failed", failed);
  return result;
}
def tests = [
  [
    "name": "fill_missing_key",
    "map1": ["a": 1],
    "map2": ["b": 2],
    "expect": ["a": 1, "b": 2]
  ],
  [
    "name": "no_overwrite_non_empty_scalar",
    "map1": ["a": 1],
    "map2": ["a": 2],
    "expect": ["a": 1]
  ],
  [
    "name": "overwrite_null_empty_string_empty_map",
    "map1": ["n": null, "s": "", "m": [:], "keep": "x"],
    "map2": ["n": 5, "s": "hi", "m": ["k": 1], "keep": "y"],
    "expect": ["n": 5, "s": "hi", "m": ["k": 1], "keep": "x"]
  ],
  [
    "name": "deep_merge_nested_maps",
    "map1": ["o": ["x": 1, "y": ""]],
    "map2": ["o": ["x": 2, "y": "filled", "z": 3]],
    "expect": ["o": ["x": 1, "y": "filled", "z": 3]]
  ],
  [
    "name": "list_merge_empty_list_and_dedupe",
    "map1": ["l": [], "l2": [1, 2]],
    "map2": ["l": 7, "l2": [2, 3, 1]],
    "expect": ["l": [7], "l2": [1, 2, 3]]
  ],
  [
    "name": "list_merge_does_not_add_null_scalar",
    "map1": ["l": [1]],
    "map2": ["l": null],
    "expect": ["l": [1]]
  ]
];
runTests(tests);
PAINLESS
)

jq -n --arg src "$painless" '{"script":{"source":$src}}' |
  curl -sSk \
    -u "${ES_USER}:${ES_PASSWORD}" \
    -X POST "${ES_URL}/_scripts/painless/_execute" \
    -H 'Content-Type: application/json' \
    -d @- |
  jq .

Checklist

• I have reviewed tips for building integrations and this pull request is aligned with them.
• I have verified that all data streams collect metrics or logs.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.
• I have verified that any added dashboard complies with Kibana's Dashboard good practices

How to test this PR locally

There are Painless tests that can be manually copied out of the YAML into Kibana's Painless Lab to verify the merge logic.

Related issues

• Supersedes [Keycloak] Prevent data_stream fields from being overwritten by the log #17229
• Relates keycloak: add support for ECS-formated data #14388

[elasticmachine]

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

[elastic-vault-github-plugin-prod]

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

[efd6]

Changelog and manifest entries?

[efd6]

Is there a way to engineer these tests into pipeline tests in a reasonably concise way? If not, I'd suggest putting these either in the PR description or commit message (the latter being my preference, though entirely up to you given the length), and make it runnable from a terminal, e.g.

: "${KIBANA_URL:=https://localhost:5601}"
: "${KIBANA_USER:=elastic}"
: "${KIBANA_PASSWORD:=changeme}"

painless=$(cat <<'PAINLESS'
def mergeMaps(Map map1, Map map2) {
  for (def key : map2.keySet()) {
    if (!map1.containsKey(key)
        || map1[key] == null
        || map1[key] == ""
        || (map1[key] instanceof Map && map1[key].isEmpty())) {
      map1[key] = map2[key];
    } else if (map1[key] != map2[key]) {
      if (map1[key] instanceof Map && map2[key] instanceof Map) {
        map1[key] = mergeMaps(map1[key], map2[key]);
      } else if (map1[key] instanceof List) {
        def combined = new LinkedHashSet(map1[key]);
        if (map2[key] instanceof List) {
          combined.addAll(map2[key]);
        } else if (map2[key] != null) {
          combined.add(map2[key]);
        }
        map1[key] = new ArrayList(combined);
      }
    }
  }
  return map1;
}

def runTests(List tests) {
  def results = new ArrayList();
  for (def t : tests) {
    def got = mergeMaps(t["map1"], t["map2"]);
    results.add([
      "test": t["name"],
      "passed": got.equals(t["expect"]),
      "got": got,
      "expected": t["expect"]
    ]);
  }
  def passed = new ArrayList();
  def failed = new ArrayList();
  for (def r : results) {
    if (r["passed"]) passed.add(r);
    else failed.add(r);
  }
  def result = new LinkedHashMap();
  result.put("passed_all", passed.size() == tests.size());
  result.put("passed_count", passed.size());
  result.put("failed_count", failed.size());
  result.put("failed", failed);
  return result;
}
def tests = [
  [
    "name": "fill_missing_key",
    "map1": ["a": 1],
    "map2": ["b": 2],
    "expect": ["a": 1, "b": 2]
  ],
  [
    "name": "no_overwrite_non_empty_scalar",
    "map1": ["a": 1],
    "map2": ["a": 2],
    "expect": ["a": 1]
  ],
  [
    "name": "overwrite_null_empty_string_empty_map",
    "map1": ["n": null, "s": "", "m": [:], "keep": "x"],
    "map2": ["n": 5, "s": "hi", "m": ["k": 1], "keep": "y"],
    "expect": ["n": 5, "s": "hi", "m": ["k": 1], "keep": "x"]
  ],
  [
    "name": "deep_merge_nested_maps",
    "map1": ["o": ["x": 1, "y": ""]],
    "map2": ["o": ["x": 2, "y": "filled", "z": 3]],
    "expect": ["o": ["x": 1, "y": "filled", "z": 3]]
  ],
  [
    "name": "list_merge_empty_list_and_dedupe",
    "map1": ["l": [], "l2": [1, 2]],
    "map2": ["l": 7, "l2": [2, 3, 1]],
    "expect": ["l": [7], "l2": [1, 2, 3]]
  ],
  [
    "name": "list_merge_does_not_add_null_scalar",
    "map1": ["l": [1]],
    "map2": ["l": null],
    "expect": ["l": [1]]
  ]
];
runTests(tests);
PAINLESS
)

jq -n --arg src "$painless" '{"script":{"source":$src}}' |
  curl -sSk \
    -u "${KIBANA_USER}:${KIBANA_PASSWORD}" \
    -XPOST "${KIBANA_URL}/api/console/proxy?path=_scripts%2Fpainless%2F_execute&method=POST" \
    -H 'kbn-xsrf: true' \
    -H 'Content-Type: application/json' \
    -d @- |
  jq .

[chrisberkhout]

Okay. A pipeline test like below covers all those cases and is processed okay, but it fails because the fields aren't defined. I think trying to jam those cases into existing field names would get too confusing, so I'll just put a script in the commit description.

{
  "events": [
    {
      "event": {
        "original": "{\"newkey\":2,\"scalar\":\"conflicting\",\"null\":\"non-null\",\"empty_string\":\"non-null\",\"empty_map\":\"non-null\",\"map\":{\"additional\":\"val\",\"nested\":{\"additional_nested\":\"val\"}},\"array\":[2,null,3],\"empty_array\":1}"
      },
      "scalar": 1,
      "null": null,
      "empty_string": "",
      "empty_map": {},
      "empty_array": [],
      "array": [
        1,
        2
      ],
      "map": {
        "existing": "value",
        "nested": {
          "existing_nested": "value"
        }
      }
    }
  ]
}

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: c236fdf

History

• 💚 Build #38331 succeeded 8b4c714
• 💚 Build #38296 succeeded 248d157

cc @chrisberkhout