elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.72k stars 8.13k forks source link

[Console] Only the first configured ES host receives request body #160380

Open jloleysens opened 1 year ago

jloleysens commented 1 year ago

Describe the bug:

Console has the ability to try multiple ES hosts (from elasticsearch.hosts). If a request gets hung up or connection refused it will try the next host.

The issue is that when a connection times out (i.e., some time passes) the proxied request body (stream) may have emitted end in which case it cannot be re-used in the subsequent request in which case we send the "same" request but without a body :(. This can lead to very weird issues depending on the request being made.

Steps to reproduce:

Apply this diff to current main to "fake" a failed response and get Console to retry sending the request to the next host:

diff --git i/src/plugins/console/server/routes/api/console/proxy/create_handler.ts w/src/plugins/console/server/routes/api/console/proxy/create_handler.ts
index 1a2825dcbd7..830f396f0a0 100644
--- i/src/plugins/console/server/routes/api/console/proxy/create_handler.ts
+++ w/src/plugins/console/server/routes/api/console/proxy/create_handler.ts
@@ -119,9 +119,11 @@ export const createHandler =
     }

     const legacyConfig = await readLegacyESConfig();
-    const { hosts } = legacyConfig;
+    // const { hosts } = legacyConfig;
+    const hosts = ['http://localhost:9200/', 'http://localhost:9200/'];
     let esIncomingMessage: IncomingMessage;

+    let failed = false;
     for (let idx = 0; idx < hosts.length; ++idx) {
       const host = hosts[idx];
       try {
@@ -157,6 +159,11 @@ export const createHandler =
           agent,
         });

+        if (!failed) {
+          failed = true;
+          continue;
+        }
+
         break;
       } catch (e) {
         // If we reached here it means we hit a lower level network issue than just, for e.g., a 500.

Send a request with a body, e.g.:

PUT my-index-21
{
  "mappings": {
    "properties": {
      "test": {
        "type": "text"
      },  
      "test1": {
        "type": "text"
      },
      "test2": {
        "type": "text"
      }
    }
  }
}

See that the above returns 400 -- clearly we did send the correct body!

Expected behavior:

Console should retry the full request against each ES host OR should only try against a single host and return the appropriate error instead of this confusing response.

I think we have two options:

(1) Create a re-readable stream (something like this). The risk is large requests putting additional pressure on Node.js memory. To mitigate we could choose a max proxy-able request size.

(2) Remove the fallback functionality, although it will work “correctly” for requests without a body, rationale being we consider accumulating Console request bodies in memory too risky and so Console just works with the first host specified (as before) and we remove the possibility of this “different results for the same requests” bug that way

CC @alisonelizabeth

elasticmachine commented 1 year ago

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

MakoWish commented 1 year ago

Thank you for opening this one, Allison. This has been a strange one, for sure! It all started with support asking me to run a query and submit the results. I put the results in the ticket, and I was told, "that's not possible!" Glad you guys figured out the issue!

Eric

iamgd67 commented 4 months ago

(1) Create a re-readable stream (something like this). The risk is large requests putting additional pressure on Node.js memory. To mitigate we could choose a max proxy-able request size.

This solution sounds good. As for the memory pressure, the console is mainly for debugging and testing, so the request body should not be very large, such as more than 1MB, and the concurrency should also be low.

elasticmachine commented 1 day ago

Pinging @elastic/kibana-management (Team:Kibana Management)