Fix SO query for searching across spaces

[legrego]

Summary

This is a quick followup to #82863.

The original implementation did not construct the query properly when a multi-namespace saved object was being searched via the namespaces=* option.

This also updates the test suite to be more assertive about the expected query to prevent bugs like this from slipping by again.

cc @jgowdyelastic

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 778e8ee

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[pgayvallet]

Sorry, overlooked that too. So we were using [{ term: { type } }, {}] in bool.must?

• What was the exact consequences?
• I thought you added a FTR test in the initial PR. Why wasn't this catched by it? Should we add a new one?

[legrego]

Yeah that's what was happening. The consequence was a query parse exception:

{
  "name": "ResponseError",
  "meta": {
    "body": {
      "error": {
        "root_cause": [
          {
            "type": "x_content_parse_exception",
            "reason": "[1:120] [bool] failed to parse field [must]"
          }
        ],
        "type": "x_content_parse_exception",
        "reason": "[1:120] [bool] failed to parse field [filter]",
        "caused_by": {
          "type": "x_content_parse_exception",
          "reason": "[1:120] [bool] failed to parse field [should]",
          "caused_by": {
            "type": "x_content_parse_exception",
            "reason": "[1:120] [bool] failed to parse field [must]",
            "caused_by": {
              "type": "illegal_argument_exception",
              "reason": "query malformed, empty clause found at [1:120]"
            }
          }
        }
      },
      "status": 400
    },
    "statusCode": 400,
    "headers": {
      "content-type": "application/json; charset=UTF-8",
      "content-length": "541"
    },
    "meta": {
      "context": null,
      "request": {
        "params": {
          "method": "POST",
          "path": "/.kibana/_search",
          "body": "{\"seq_no_primary_term\":true,\"query\":{\"bool\":{\"filter\":[{\"bool\":{\"should\":[{\"bool\":{\"must\":[{\"term\":{\"type\":\"ml-job\"}},{}],\"must_not\":[{\"exists\":{\"field\":\"namespace\"}}]}}],\"minimum_should_match\":1}}]}}}",
          "querystring": "size=1&from=0&rest_total_hits_as_int=true",
          "headers": {
            "user-agent": "elasticsearch-js/7.10.0-rc.1 (darwin 19.6.0-x64; Node.js v10.22.1)",
            "x-elastic-product-origin": "kibana",
            "content-type": "application/json",
            "content-length": "201"
          },
          "timeout": 30000
        },
        "options": {
          "maxRetries": 0,
          "ignore": [
            404
          ]
        },
        "id": 20
      },
      "name": "elasticsearch-js",
      "connection": {
        "url": "http://localhost:9200/",
        "id": "http://localhost:9200/",
        "headers": {},
        "deadCount": 0,
        "resurrectTimeout": 0,
        "_openRequests": 4,
        "status": "alive",
        "roles": {
          "master": true,
          "data": true,
          "ingest": true,
          "ml": false
        }
      },
      "attempts": 0,
      "aborted": false
    }
  },
  "isBoom": true,
  "isServer": false,
  "data": null,
  "output": {
    "statusCode": 400,
    "payload": {
      "message": "[1:120] [bool] failed to parse field [filter]: x_content_parse_exception",
      "statusCode": 400,
      "error": "Bad Request"
    },
    "headers": {}
  }
}

I thought you added a FTR test in the initial PR. Why wasn't this catched by it? Should we add a new one?

The FTR test verified single namespace types, not multi-namespace types. I can't organically exercise this via the FTR because OSS doesn't have any multi-namespace types, and the default distribution won't allow the * namespace to make its way to the repository.

I'd either have to create an FTR config without spaces or security enabled in the default distribution, or create a test plugin which exposed a route that we could call in test to exercise this path directly. Both options seemed overkill to me, but I'm happy to revisit if you think it'd be worthwhile.

[pgayvallet]

It's 'amazing' that we don't have a SO FTR testsuite in x-pack/test/api_integration/apis tbh... I guess :thisisfine:, ml should introduce 'proper' coverage when they'll leverage the feature.