elastic / kibana

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

[Fleet] Updating integrations does not trigger rollover anymore #158242

Open P1llus opened 1 year ago

P1llus commented 1 year ago

Running on the current 8.9.0-SNAPSHOT, when upgrading a fleet integration that includes either:

  1. Removal of field mappings.
  2. Updating the type of existing fields.
  3. Adding dynamic templates.

Does not trigger a rollover anymore. While more testing is still required, I added this issue to discuss.

When for example changing the type of an existing field, when upgrading, all the index templates and component templates are correctly updated, however a rollover is never initiated. Currently trying to find out when this was introduced, as I believe it might be related to ignore_malformed When I tried to ingest data with the field that changed type (but did not rollover), the field is rather marked as ignored now.

        "_index": ".ds-logs-logs-ti_abusech.malware-2023.05.23-000001",
        "_id": "UDrtR4gBnqajF9EFJlKe",
        "_score": 1,
        "_ignored": [
          "abusech.malware.signature"
        ], 

Steps to reproduce:

  1. Start up the stack elastic-package stack up -d -v --version=8.9.0-SNAPSHOT
  2. Browse to integration, click on any of them, click Settings, click install assets/integration.
  3. Go to Kibana Devtools, and POST a document to your integration, for example (example doc is not complete):
    POST logs-logs-ti_abusech.malware/_doc
    {
    "abusech": { "malware": {"signature": "test"}},
    }
  4. Modify the integration package by changing any of the field types in any of the datastreams (that you used in your POST test).
  5. Bump the version of the package and build it: elastic-package build && elastic-package format && elastic-package lint && elastic-package check && elastic-package build
  6. restart package-registry: elastic-package stack up -s package-registry -v -d --version=8.9.0-SNAPSHOT
  7. Browse to the integration page, settings, click update integration.
  8. Try to POST another document, using the same field, but the type you changed to (from long to keyword for example).

Expected behavior is that the upgrade should have triggered a rollover, so that the index now has the new field type, but instead it only updates the index/component templates, and never rolls over, the "error" is simply ignored. Looking at the Elasticsearch logs from the container, no rollover is ever initiated.

elasticmachine commented 1 year ago

Pinging @elastic/fleet (Team:Fleet)

joshdover commented 1 year ago

@P1llus can you confirm if this is was working on the 8.8 branch or earlier releases?

Relevant code: https://github.com/elastic/kibana/blob/84d070d2f7ef0ec774946d61b8a2f4637d747047/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts#L688

P1llus commented 1 year ago

Tested on 8.8.0 and 8.9.0, though there was some inconsistencies.

For some reason, when we create document entries manually in the datastream, so by only installing the integration assets, then POST'ing a document to the datastream alias, then upgrading the integration (without actually using a policy and elastic agent), then the rollover does not seem to happen.

Can it be because of how we POST to the datastream using POST logs-logs-ti_abusech.url-default/_doc rather than ingesting data from the agent? I wonder how this affects other components outside of agent that uses integration assets as well, like the Elastic Serverless forwarder.

Seems like its kind of a false alarm, but I still want to understand what the difference in behavior is.

joshdover commented 1 year ago

then upgrading the integration (without actually using a policy and elastic agent), then the rollover does not seem to happen.

This definitely sounds like a bug. Our package manager should be agnostic to whether or not the integration has an integration policy attached to it.

I wonder if some of the changes around input packages a few versions back have anything to do with this.

@juliaElastic @jlind23 I think we should prio this in the next few sprints. Luckily for now there is a workaround (rollover manually) and the impact is limited to non-Agent usages.

CohenIdo commented 1 year ago

I conducted a test on a live agent that injects data to verify the rollover functionality. Unfortunately, the rollover process did not execute as intended.

TL;DR: I applied the CSPM package policy to an active agent, performed a package upgrade, and attempted to index a field based on the new index mapping. However, the field did not get mapped, and a new index was not created as expected.

  1. Start up the stack elastic-package stack up -d -v --version=8.9.0-SNAPSHOT.
  2. Choose the agent policy which used by the running agent and add the CSPM package.
  3. Verify data is indexed in the data stream - logs-cloud_security_posture.findings-default
  4. Checkout the integration branch with the relevant changes (auto import for ecs fields) and build the package.
  5. Build the package with elastic-package build
  6. Restart package-registry: elastic-package stack up -s package-registry -v -d --version=8.9.0-SNAPSHOT
  7. Verify the auto upgrade was applied and CSPM version was updated.
  8. Post new document to the data stream with a new ecs field observer.hostname
  9. Check the index mapping of the data stream

Result:

ruflin commented 1 year ago

the impact is limited to non-Agent usages

Why is that? How is any other shipper different from standalone Elastic Agent?

P1llus commented 1 year ago

the impact is limited to non-Agent usages

Why is that? How is any other shipper different from standalone Elastic Agent?

It was an assumption based on the output from my testing, where manual tests did not trigger a rollover, but the exact same scenario using an active agent with the integration attached to a policy triggered a rollover on the exact same tests.

However I see now that Cohen also had issues, with an active agent, so the reason seems a bit uncertain @ruflin ..

nchaulet commented 1 year ago

@P1llus Actually it's the expected behavior to not trigger a rollover if we are able to update the existing index mappings, in your tests does the existing write indices where updated? I did a few tests and it seems to worked, but we may are missing something here.

nchaulet commented 1 year ago

Currently the logic we have is to try to update the existing mapping on the datastreams write index, and if there is any errors happening here we rollover.

However there is a few case (related to mappings, there is more related to settings too) that are not correctly handled with that approach

Moving that logic to ES as suggested here will probably be best longterm solution https://github.com/elastic/elasticsearch/issues/96521

@juliaElastic @jlind23 In the meantime should we try to handle those scenarios? try to detect the type of mappings change and trigger rollover.

jlind23 commented 1 year ago

@nchaulet It would be best to have this fixed but I agree with you that I would rather rely on https://github.com/elastic/elasticsearch/issues/96521 instead of delivering some thrown away code.

juliaElastic commented 1 year ago

@nchaulet My concern is whether this is a regression, sounds like it from the issue description. If I understand correctly, for example changing the type of an existing mapping should throw an error and trigger a rollover, are you saying it is working as expected?

nchaulet commented 1 year ago

@nchaulet My concern is whether this is a regression, sounds like it from the issue description. If I understand correctly, for example changing the type of an existing mapping should throw an error and trigger a rollover, are you saying it is working as expected?

I do not think it's a regression changing a type of a mapping seems to work, but some case like deleting a mapping or changing a dynamic are not and it seems they never worked

jlind23 commented 1 year ago

I would consider this issue as blocked for now until https://github.com/elastic/elasticsearch/issues/96521 is implemented.