Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] Make input IDs unique in agent policy yaml #127343

Merged
merged 5 commits into from
Mar 10, 2022

Conversation

jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Mar 9, 2022

Summary

Resolves #125844. This PR makes input IDs in the final generated agent policy unique by building it from the parent package policy ID + input type + the associated policy template name (if any).

When a package policy is built, its streams are grouped by input type+policy template, so that combination should be unique (and stable) on each package policy.

This screenshot shows an example of unique IDs being generated for AWS billing and Cloudtrail inputs:

image

Checklist

@jen-huang jen-huang self-assigned this Mar 9, 2022
@jen-huang jen-huang changed the title [Fleet] Make package policy input IDs unique [Fleet] Make input IDs unique in agent policy yaml Mar 9, 2022
@jen-huang jen-huang added release_note:fix Team:Fleet Team label for Observability Data Collection Fleet team v7.17.2 v8.0.2 v8.1.1 v8.2.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 9, 2022
@jen-huang jen-huang marked this pull request as ready for review March 9, 2022 23:47
@jen-huang jen-huang requested a review from a team as a code owner March 9, 2022 23:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fleet 110.2KB 110.3KB +55.0B

History

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

cc @jen-huang

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@jen-huang jen-huang merged commit 0e99e94 into elastic:main Mar 10, 2022
@jen-huang jen-huang deleted the fix/unique-input-ids branch March 10, 2022 16:21
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.1 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.1:
- [Security Solution] Unskip alerts related Cypress tests (#127187)
- [demo env] Skip docker cloud build (#127459)
- unskipping a11y painless lab test (#127082)
7.17 Backport failed because of merge conflicts

You might need to backport the following PRs to 7.17:
- unskipping a11y painless lab test (#127082)
8.0 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.0:
- unskipping a11y painless lab test (#127082)

Manual backport

To create the backport manually run:

node scripts/backport --pr 127343

Questions ?

Please refer to the Backport tool documentation

@kevinlog
Copy link
Contributor

@ph @jen-huang before backporting and releasing this, we need to resolve this downstream effect: #127570

More details on what we're seeing on our side in this this comment: #127570 (comment)

Essentially, the Endpoint is now adding the endpoint- prefix to all policy IDs which is breaking some of our UI. Let's figure out the right way to resolve this before it goes out.

cc @ferullo @paul-tavares

@nchaulet
Copy link
Member

@kevinlog, currently we do not persist that id, but if we persist that id on the package policy saved object, you will be able to query the package policy saved object with the id to retrieve the package policy id, it is something that will work for you?

We can expose a service getPackagePolicyForInputId() and the package policy api can support a query param ?input_id= and we will take care to support old input id too without the prefix.

@ph
Copy link
Contributor

ph commented Mar 14, 2022

@kevinlog Thanks for pointing this out, sorry for not having communicated that clearly with everyone. For the Agent, we will merge the changes because they currently affect Filebeat. We haven't changed the logic on the endpoint configuration. We are still giving the complete input block.

@nchaulet @jen-huang I haven't considered that, but would the following be true:

When a user upgrades they will still be using the old ids for input that aren't unique?

@nchaulet
Copy link
Member

When a user upgrades they will still be using the old ids for input that aren't unique?

Yes until they do something that trigger a policy change, (changing anything in the policy or a global change to outputs or settings)

@ph
Copy link
Contributor

ph commented Mar 14, 2022

@kvch @belimawr would that be a problem with the filestream input ?

@paul-tavares
Copy link
Contributor

paul-tavares commented Mar 14, 2022

@nchaulet

Re: using input_id to query fleet's package policies

I'm not sure yet of the full impact of the changes to the id on our side, so its hard to tell if what you suggest would work. FYI - I think we also use these IDs in some cases to query Agent Policies using KQL. We might also have impacts to our server code.

I would also like to request that the actual Package Policy ID be included in the YAML if the id is no longer going to be actual Package policy id. We have some other info about the package policy already (revision, name, type), so including the ID will help us ensure we are able to cross reference data back to fleet policies:

- id: endpoint-066fdf94-a6bc-4185-b27d-0245f96e3ab9
    name: Security
    revision: 4
    type: endpoint
    use_output: default
    meta:
      package:
        name: endpoint
        version: 1.5.0

We can then maybe see if we can get Endpoint to return that back to us in their messages and we can start to use it again.

@paul-tavares
Copy link
Contributor

Question:

Could this id remain unchanged and a new property be added that store the unique ID that is needed by you all?

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 14, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 127343 or prevent reminders by adding the backport:skip label.

jen-huang added a commit to jen-huang/kibana that referenced this pull request Mar 14, 2022
* Make package policy input IDs unique

* Lint

* Fix tests

* Add unit test

* Lint

(cherry picked from commit 0e99e94)

# Conflicts:
#	x-pack/plugins/fleet/server/integration_tests/__snapshots__/cloud_preconfiguration.test.ts.snap
jen-huang added a commit to jen-huang/kibana that referenced this pull request Mar 14, 2022
* Make package policy input IDs unique

* Lint

* Fix tests

* Add unit test

* Lint

(cherry picked from commit 0e99e94)

# Conflicts:
#	x-pack/plugins/fleet/server/integration_tests/__snapshots__/cloud_preconfiguration.test.ts.snap
jen-huang added a commit that referenced this pull request Mar 14, 2022
* Make package policy input IDs unique

* Lint

* Fix tests

* Add unit test

* Lint

(cherry picked from commit 0e99e94)

# Conflicts:
#	x-pack/plugins/fleet/server/integration_tests/__snapshots__/cloud_preconfiguration.test.ts.snap
@jen-huang jen-huang removed the v8.0.2 label Mar 15, 2022
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 15, 2022
jen-huang added a commit that referenced this pull request Mar 15, 2022
* Make package policy input IDs unique

* Lint

* Fix tests

* Add unit test

* Lint

(cherry picked from commit 0e99e94)

# Conflicts:
#	x-pack/plugins/fleet/server/integration_tests/__snapshots__/cloud_preconfiguration.test.ts.snap

Co-authored-by: Kibana Machine <[email protected]>
@joshdover joshdover added the QA:Ready for Testing Code is merged and ready for QA to validate label Mar 16, 2022
@belimawr
Copy link

@kvch @belimawr would that be a problem with the filestream input ?

The input IDs changing from a non-unique to a unique one? That will duplicate the events once.

If a filestream input that did not have an ID set gets an ID, we can migrate the data in the registry and not duplicate events.

My PR #30717 focus on adding IDs to filestream inputs that did not have one. Other cases have not been covered there.

@amolnater-qasource
Copy link

Hi @joshdover
We have revalidated this on latest 8.2 Snapshot and found it working fine now.

  • Unique ids are provided to integrations in agent policy yaml.
  • Even after doing changes unique id remains same.

Screenshots:
15
16
17

Build details:

VERSION: 8.2.0
BUILD: 51328
COMMIT: 2a93e80574f9c1052c541bc4083a440a623899e8

Hence marking this as QA:Validated.
Please let us know if anything else is required from our end.
Thanks

@amolnater-qasource amolnater-qasource added QA:Validated Issue has been validated by QA and removed QA:Ready for Testing Code is merged and ready for QA to validate labels Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed QA:Validated Issue has been validated by QA release_note:fix Team:Fleet Team label for Observability Data Collection Fleet team v7.17.2 v8.1.1 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] Agent policy inputs id are not unique