elastic / kibana

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

[Fleet] Automatically add tags on assets for security integration packages #152814

Closed P1llus closed 1 year ago

P1llus commented 1 year ago

Fleet should support top level saved object tags provided by a package manifest. This would look something like

# integration/manifest.yml
tags:
- text: SecurityExternalIntegrations
  asset_types:
  - dashboard
- text: ThreatDetectionDashboard
  asset_ids:
  - dashboard3
  - dashboard4

These saved object tags should be relatable to either all assets of a given type, or to individual assets based on their IDs.

When Fleet installs a package that includes these top-level tags, it should generate the tag + asset references using a unique ID for the tag object. This ID should be a unique identifier based on the downcased text value to avoid collisions and duplications. For example, if two packages define a SecurityExternalIntegrations and securityexternalintegrations tag, these would result in only one tag object being created in Kibana.

There is a special case tag for the SecuritySolution tag that exists today. Currently, the Kibana security solution app generates this tag on startup if it does not exist. In order to prevent duplication and ensure backwards compatibility, when Fleet encounters a SecuritySolution tag from a given package manifest, it should instead defer to using the existing SecuritySolution tag if one exists in that cluster.

Tags should also be assigned a color at random, and the possible set of colors should be sourced from EUI theme colors to guarantee acceptable contrast and allow users to differentiate between them.


Original description Currently Fleet adds 2 tags to integration package dashboards, the package name and the "Managed" tag. We would like to add another tag, called `Security Solution` to all packages and datastreams owned by us: ``` owner: github: elastic/security-external-integrations ``` We could also do another approach which might be easier, for integrations with multiple owners for example, where we simply have a list of the Dashboard ID's, and the tag that we want in the manifest.yml? The tag itself can have an autogenerated ID, but the text itself would need to stay the same. However the tag might already exist, and is it possible to reuse that then? The tag itself should never be removed, as it would belong to multiple integrations The usecase for this is multiple, but the main is that the Security UI Dashboard section list all dashboards with specific tags, and we are unable to add these tags to packages manually in each package, as they would end up being duplicated. Currently most packages that are owned by SEI is tagged with the owner above, however certain packages have multiple owners (mostly AWS and Azure), and the only small issue we need to get around is how we make sure we only tag our own dashboards in large multi-owner packages like that.
elasticmachine commented 1 year ago

Pinging @elastic/fleet (Team:Fleet)

semd commented 1 year ago

Hey @P1llus, would it be possible to add the Security Solution tag to the integration dashboards imported as well? Using the same logic as for Security Integrations:

The reason is that after talking about it with @paulewing we realized it would be more simple for the user to retrieve dashboards using only the Security Solution tag in the dashboards table, like we are doing now, instead of using two different tags. The Security Integration tag will be also helpful for filtering, but won't be used for the initial dashboard retrieval on the Security Solution dashboards page.

What do you think?

P1llus commented 1 year ago

@semd I don't see anything against that, seeing as we are adding the tags to help support the UI effort. I don't believe the issue has been started yet.

Do we really want two tags though? We could just go with the "Security Solutions" one? All dashboards will also have the "Managed" tag, that way we could differentiate between your different usecases using that?

The only issue I see is that there will be duplicates of the solution tag if you already created it, as I dont believe the current logic for tags are checking for this. But thats up to @kpollich I believe.

jen-huang commented 1 year ago

The usecase for this is multiple, but the main is that the Security UI Dashboard section list all dashboards with specific tags, and we are unable to add these tags to packages manually in each package, as they would end up being duplicated.

@hop-dev I'd like to have your thoughts on how we can handle Kibana asset tagging that:

  1. Supports packages declaring their own tags for assets like dashboards
  2. Doesn't duplicate the tag object if the same tag is used in multiple packages (https://github.com/elastic/elastic-package/issues/1060#issuecomment-1408460337)

There could be other options, such as exposing a server-side "tagging" extension point in Fleet that other solutions can hook into. Each extension could return a list of tags to add based on their own conditions derived from the package info + asset info.

WDYT?

P1llus commented 1 year ago

@jen-huang @hop-dev thats actually a great way to think about it, if we could define somewhere in a package which tags we want, and the tags then being created by fleet then that also works.

That way we dont need any new logic to find owners as we can add them in manually.

We would need some way to specify which assets to tag in packages that has multiple owners maybe? Azure/aws for example.

semd commented 1 year ago

Do we really want two tags though? We could just go with the "Security Solutions" one? All dashboards will also have the "Managed" tag, that way we could differentiate between your different usecases using that?

@P1llus Yes, that would also work. Users will be able to filter by Security Solution + Managed tags, so there's no need for Security Integrations.

Doesn't duplicate the tag object if the same tag is used in multiple packages

@jen-huang The Security Solution tag is also created automatically by the UI, only if it does not exist. So it is very likely the tag is already created. There is also a corner case where we may find multiple tags with this exact same name (tags are not unique by name). We currently use the first one the query returned to create new dashboards, then the UI uses all tags named Security Solution to display the dashboard table. Just FYI

hop-dev commented 1 year ago

@jen-huang @P1llus I think defining these in the package is the most appealing to me as it allows for more use cases. As I see it the requirements are:

My proposed solution:

We already allow tags to be added as part of a package, so we will have to define these tags in a separate place, maybe the manifest or a new file?

Here is an example of the package yml:

---
tags:
- text: Some Text
  asset_ids:
  - dashboard1
  - dashboard2
- text: Some Other Text
  asset_ids:
  - dashboard3
  - dashboard4

I do think this is prone to package devs forgetting to update the IDs list so we may consider allowing a type to be specified?

e.g here Some Text is added to all dashboard assets:

---
tags:
- text: Some Text
  asset_types:
  - dashboard
- text: Some Other Text
  asset_ids:
  - dashboard3
  - dashboard4

This would then create tags with ID fleet-shared-tag-552e21cd4cd9918678e3c1a0df491bc3-space1 and fleet-shared-tag-2932cd2ffddeec2f06f0682391253d6f-space1 where the hash is the MD5 hash of the tag text lowercased

P1llus commented 1 year ago

I like the idea alot @hop-dev ! Maybe we could add a new section in the root level manifest.yml, since we do not have kibana assets per datastream? Its also good that we can indeed configure which ID's should be tagged like you mentioned, so packages with multiple owners will still work out 👍

hop-dev commented 1 year ago

@mrodm how do you feel about us adding this new kind of tagging to the manifest?

I think logically they are aligned with assets but at the minute the assets folder is quite straight forward raw assets to be imported, whereas this will be slightly different because Fleet is creating the assets and managing the IDs.

hop-dev commented 1 year ago

@P1llus another point is colour 😄 Do you care what colour these tags are? I think in my implementation all fleet created tags will be the same colour

P1llus commented 1 year ago

@P1llus another point is colour 😄 Do you care what colour these tags are? I think in my implementation all fleet created tags will be the same colour

I don't think that matters, though what I did notice is all tags created by fleet is white, which is really hard to see when you have a white background as well

jsoriano commented 1 year ago

Related: https://github.com/elastic/package-spec/issues/396

mrodm commented 1 year ago

As @jsoriano mentioned, this is quite similar to https://github.com/elastic/package-spec/issues/396. The difference would be to be able to add tags to some specific dashboards inside a package. And probably, color could not be specified as it is mentioned in that issue... if they are going to be defined in different packages.

Regarding the example, I would try to avoid tags as the key for the first level to avoid confusions if they are tags to the package itself (like the categories).

Probably, there is another issue related to elastic-package. Would it be necessary to add that tag (or pattern) into the tags that are filtered when exporting dashboards? Like it was done for the current fleet managed tags https://github.com/elastic/elastic-package/issues/1060 (https://github.com/elastic/elastic-package/pull/1206).

Looking into the patterns described in https://github.com/elastic/kibana/issues/152814#issuecomment-1516053663 , currently elastic-package just filters the tags for the default space.

semd commented 1 year ago

@mrodm @hop-dev How are we going to align this approach with the tag creation from Security Solution? We have some workflows in the UI where we need to create the SecuritySolution tag directly from the app, if it does not exist. Currently we only care about the tag name, we do not specify ID, it is auto-generated. We should be able to reuse the same tag, regardles of where it was created.

Would it make sense for you to expose some api from fleet to create the tag? so you can set your own generated ID.

kpollich commented 1 year ago

How are we going to align this approach with the tag creation from Security Solution? We have some workflows in the UI where we need to create the SecuritySolution tag directly from the app, if it does not exist.

If I'm understanding @hop-dev's proposal above, part of the implementation here would be predictable ID's based on the MD5 hash of a given tag's name, or a seeded UUID or something of that nature. Generating this consistent ID is important because it will guarantee we don't generate multiple tags when the same tag is defined by multiple integrations. We could rely on the name as a unique identifier, but I fear we'd run into cases like

# Integration 1
saved_object_tags:
  - my_tag

# Integration 2
saved_object_tags:
  - My_Tag

where the integrations are intended to generate assets tagged with the same tag, but human error around casing causes two almost-identical tags to be generated and for the asset references to be disjointed.

I think we'll need to hardcode a special case for the security solution tag in Fleet such that when a package is installed that includes a tag whose name matches SecuritySolution (case insensitive), Fleet will not generate a predictably identified tag at install time. Instead, the existing SecuritySolution tag will be used to tag all assets.

Ideally in the future, perhaps we'd consider removing this tag generation logic from the security solution in favor of using the package manifests as the sole source of truth. I don't think this it something we can immediately, however.

image


Alluded to above is @mrodm's suggestion to avoid the tags property as this property name already has meaning elsewhere in the package spec. Instead, I'd suggested using saved_object_tags in the top-level manifest to indicate the intent of these values and their difference from the categorization features for packages.

So, we'd have a manifest.yml that contains

---
tags:
- text: SecurityExternalIntegrations
  asset_types:
  - dashboard
- text: ThreatDetectionDashboard
  asset_ids:
  - dashboard3
  - dashboard4

That results in tags being generated along the lines of

[
  { 
    "name": "SecurityExternalIntegrations",
    "id": "fleet-shared-tag-0d72fb1dd3e666b5cf7b899c829d3964-space1",
    "description": "",
    "is_managed": true // I think tags have this flag?
    "color": "..."
  }
]

Some other thoughts

To summarize, I think we can handle this implementation entirely in Fleet's package installation and asset tagging code paths, but we'll likely need a special case around the security solution tag.

@hop-dev @P1llus @mrodm @semd - Eager for any other thoughts you might have. Otherwise, I think we have what we need to get started on this in an upcoming sprint.

P1llus commented 1 year ago

@kpollich one quick question that I remember now, would this be backwards compatible? Meaning that integrations running on older versions will simply ignore the settings? Or does the minimum version need to be bumped up as well?

For the Security Solution tag, it might not always exist, it would almost be easier for us to create the tag from fleet, but I will leave that up to @semd .

Will the assets also have the "managed" tag? So its easier to filter on custom vs managed assets.

In terms of the manifest naming that sounds good to me! If we want to hardcode some colors, or allow to specify them in the manifest does not matter too much to me specifically, however the default dark text on white background used for the current "Managed" tag is quite hard to read. We should default to some neutral color that adhere to the color design patterns for Kibana/EUI.

kpollich commented 1 year ago

one quick question that I remember now, would this be backwards compatible? Meaning that integrations running on older versions will simply ignore the settings? Or does the minimum version need to be bumped up as well?

Yes, I would expect older stack versions to simply ignore top-level saved_object_tags declared by a package. If a version of a package that includes those tags is compatible with say 8.6, they won't be created or referenced by the Fleet API running on that cluster. This is consistent with other "new new" package spec fields. As I see it, this constitutes backwards compatibility as older stack versions won't have tag collisions or anything like that.

For the Security Solution tag, it might not always exist, it would almost be easier for us to create the tag from fleet, but I will leave that up to @semd

I'm not familiar enough with the security solution code to track down where the tag logic lives, but I'd be interested to read through it and get a better sense for what's happening here. To me, it doesn't seem like we need to provide a Fleet API for creating tags "on the fly" if support tags as part of an integration's manifest, but I might be missing use cases or an accurate mental model around that.

Will the assets also have the "managed" tag? So its easier to filter on custom vs managed assets.

Yes this is definitely something we should include. The tags themselves should get metadata/managed flags making it clear that they were generated by Fleet/EPM.

We should default to some neutral color that adhere to the color design patterns for Kibana/EUI.

Yeah this sounds like the best approach to me. I think we have 12-15 known "good" colors to choose from at random that's probably best. There will be cases where two tags for different packages are the same color, but it's better than the all-white contrast issues we have today.

kpollich commented 1 year ago

I've updated the description with the implementation plan as I understand it. Please review at your convenience, everyone 🙏

P1llus commented 1 year ago

I've updated the description with the implementation plan as I understand it. Please review at your convenience, everyone 🙏

All looks excellent from my side, I believe we should try to not have any custom logic for our specific securitysolutions tag, but rather work with @semd to see if fleet could somehow manage it, or have they work together nicely

semd commented 1 year ago

Hi @kpollich

I'm not familiar enough with the security solution code to track down where the tag logic lives, but I'd be interested to read through it and get a better sense for what's happening here. To me, it doesn't seem like we need to provide a Fleet API for creating tags "on the fly" if support tags as part of an integration's manifest, but I might be missing use cases or an accurate mental model around that.

Here's where we create the tag. First we call a Security Solution route to find the tag by name (case sensitive), if it is not found call another route to create it: https://github.com/elastic/kibana/blob/973f4f36065c0aa10bcc324f19d761f4ca101574/x-pack/plugins/security_solution/public/dashboards/containers/use_fetch_security_tags.ts#L36-L39 This route uses savedObject client to create the tag from the parameters. As you see the ID is not passed so it is auto-generated. Right now the tag attributes are passed from the UI because those server routes are generic, but we could create a dedicated route for it, so all the attributes can live on the server side.

I believe we should try to not have any custom logic for our specific securitysolutions tag

I agree @P1llus, I think we can avoid that. Do you think Fleet could expose the function to generate the (consistent) ID from the tag name? if we could use it in Security I think that would be enough. Or if you prefer using a predefined ID, let us import it somehow. If we could find a way to create the tag using the same attributes and ID, we should be good. Right?

hop-dev commented 1 year ago

Do you think Fleet could expose the function to generate the (consistent) ID from the tag name?

@semd I think this is something we definitely can do, I think it is a nice solution 👍 just to confirm would you want to import it client side or server side?

Are we saying that you would be OK for the tag to be duplicated for deployments which have the existing tag with auto generated ID?

semd commented 1 year ago

@hop-dev it would be easier for us to import it on the client side, but if you think it is better to have it on the server side we can adapt the code.

Are we saying that you would be OK for the tag to be duplicated for deployments which have the existing tag with auto generated ID?

Yes, we are okay with that. We will create the new static (self-generated ID) tag if it does not exist, and use it for new custom dashboards users create. But our Security Dashboard list will continue to use all tags with SecuritySolution name to retrieve the dashboards, so users will still see dashboards referenced with legacy (auto-generated id) tags.

So, if Fleet could use the same exact SecuritySolution (case sensitive) name for the new tag, it would simplify the logic on the Security side as well.

nimarezainia commented 1 year ago

@jen-huang @kpollich what is the urgency in getting this done?

@P1llus what release does this need to b delivered in? I need to weigh it against other priorities we have. thx

P1llus commented 1 year ago

@nimarezainia we have been trying to get this into 8.9, as this also impacts our ability to embedd integration dashboards for serverless. We communicated the urgency very early on with @kpollich .

kpollich commented 1 year ago

Yes this has been pending for a while now and has been punted a few times already. Would greatly prefer to keep this prioritized for 8.9. I understand team bandwidth will likely make this challenging, but this has already slipped out of multiple sprints.

juliaElastic commented 1 year ago

@criamico The package-spec issue is https://github.com/elastic/package-spec/issues/396 that @mrodm has picked up now, so the kibana implementation can be done after.

nimarezainia commented 1 year ago

@criamico please let me know if this is a risk for this sprint, so we can figure out alternatives if needed. thank you.

jlind23 commented 1 year ago

@criamico - @mrodm PR is now merged so you should be good to move forward.

criamico commented 1 year ago

Reading through the past conversation, here is my summary of the current requirements.

Summary

For all packages that have file kibana/tags.yml as per https://github.com/elastic/package-spec/pull/567, Fleet will generate asset tags objects with following requirements:

Example

In case of package having kibana/tags.yml of type

- text: SecurityExternalIntegrations
  asset_types:
  - dashboard
- text: ThreatDetectionDashboard
  asset_ids:
  - dashboard3
  - dashboard4

Fleet should generate the following tags:

[
  { 
    "name": "SecurityExternalIntegrations",
    "id": "fleet-shared-tag-<unique_id_1>-space1",
    "description": "",
    "color": "..."
  },
   { 
    "name": "ThreatDetectionDashboard",
    "id": "fleet-shared-tag-<unique_id_2>-space1",
    "description": "",
    "color": "..."
  }
]

Please let me know if I'm misunderstanding/overlooking something. @juliaElastic @P1llus

criamico commented 1 year ago

Also, is my understanding correct that the tags are not displayed in Fleet UI but they are visible in the dashboards list?

Screenshot 2023-07-25 at 11 46 27

I'm guessing that the white labels @P1llus was referring to are the ones in the above screenshot.

juliaElastic commented 1 year ago

@criamico Yes, these are the existing managed tags. The new tags should show up in Dashboards or other places where assets can display tags. We don't show these tags in Fleet UI.

juliaElastic commented 1 year ago

@P1llus The changes are merged, please test on your side once the change is available in a next kibana snapshot.

criamico commented 1 year ago

Hi @P1llus @semd Could you let us know if the code correctly handles the existing SecuritySolution tag? In my PR I assumed the existing id is SecuritySolution but I'd like to have confirmation that is correct from your side.

criamico commented 1 year ago

I created a test package that has the following tags.yml:

- text: SecuritySolution
  asset_types:
    - dashboard
- text: OnlySomeAssetsTag
  asset_ids:
    - sample_dashboard
    - sample_search2
- text: MixedTypesTag
  asset_ids:
    - sample_visualization
  asset_types:
    - search

This package can be used to test locally: assets_with_tags-0.1.0.zip

Instructions to load the package:

curl -XPOST -H 'content-type: application/zip' -H 'kbn-xsrf: true' http://<YOUR_HOST>/api/fleet/epm/packages -u elastic:changeme --data-binary @assets_with_tags-0.1.0.zip

Will create these tags:

Screenshot 2023-08-07 at 15 31 25
harshitgupta-qasource commented 1 year ago

Hi Team,

We have created 06 test cases for this feature under the Fleet test suite at the links:

Please let us know if we are missing any scenarios to be covered here.

Thanks!

criamico commented 1 year ago

From a conversation with @marc-gr, it seems that the security solution tag is actually created with "Security Solution" name and "security-solution-default" id: image (8)

{
    "tags": [
        {
            "id": "fleet-shared-tag-1password-96314950-6e7c-52cf-bc33-5c044dc2c4e0-default",
            "name": "Security Solution",
            "description": "Tag defined in package-spec",
            "color": "#F583B7"
        },
        {
            "id": "fleet-managed-default",
            "name": "Managed",
            "description": "",
            "color": "#FFFFFF"
        },
        {
            "id": "security-solution-default",
            "name": "Security Solution",
            "description": "",
            "color": "#D36086"
        },
        {
            "id": "SecuritySolution",
            "name": "SecuritySolution",
            "description": "Tag defined in package-spec",
            "color": "#F04E98"
        },
        {
            "id": "fleet-pkg-1password-default",
            "name": "1Password",
            "description": "",
            "color": "#4DD2CA"
        },
        {
            "id": "fleet-pkg-system-default",
            "name": "System",
            "description": "",
            "color": "#4DD2CA"
        },
        {
            "id": "fleet-pkg-elastic_agent-default",
            "name": "Elastic Agent",
            "description": "",
            "color": "#4DD2CA"
        }
    ]
}

I couldn't repro locally, for some reason my local env doesn't create the "Security Solution" tag. However I can raise a quick PR to fix the wrong name and id.

criamico commented 1 year ago

I did some further testing before merging the above PR and its seems that the Security Solution tag is created without a unique id: https://github.com/elastic/kibana/blob/dd0938bea3ebd745a49ac164a7a5f053ba6a138b/x-pack/plugins/security_solution/public/dashboards/containers/use_fetch_security_tags.ts#L44-L51

That means that in some cases you can have multiple Security Solution tags, with different ids and same name:

Screenshot 2023-08-23 at 15 07 09

Do you think it's fine if Fleet creates the tag with id security-solution-default as I did in the above PR? The assets would still be displayed in the security dashboards. Of course it would be better if we agree on a unique id so that we can avoid this type of duplication, but this can be addressed later.

@P1llus @juliaElastic

P1llus commented 1 year ago

@P1llus @juliaElastic @marc-gr I think its okay to do this change for now, so that we can resolve it, but let's see if we can communicate with @semd once he is back, to see if there is a more permanent choice we want later on.

harshitgupta-qasource commented 1 year ago

Hi Team,

We have executed 06 testcases under the Feature test run for the 8.10.0 release at the link:

Status:

Build details: VERSION: 8.10.0 BC4 BUILD: 66329 COMMIT: 913dd8bc6679bec38486d44673ce27a1fb7a107b

As the testing is completed on this feature, we are marking this as QA:Validated.

Please let us know if anything else is required from our end. Thanks