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.

Sign up for GitHub

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

Jump to bottom

[Fleet] Tag assets based on definitions in integration tag.yml #162643

Merged
merged 15 commits into from
Aug 2, 2023
Merged

[Fleet] Tag assets based on definitions in integration tag.yml #162643

merged 15 commits into from
Aug 2, 2023

Conversation

criamico
Copy link
Contributor

@criamico criamico commented Jul 27, 2023 •
edited
Loading

Closes #152814

Summary

Tag assets based on definitions in integrations file tag.yml following these guidelines:

  • Tags are not be duplicated if a fleet created tag already exists with that text in the same space (i.e two integrations using a tag with the same text installed in the same space should share a tag, not create two)
  • Tag ids follow the template fleet-shared-tag-${pkgName}-${uniqueId}-${spaceId}
  • When tag is SecuritySolution it generates a SecuritySolution tag to maintain compatibility with security requirements
  • The function that generates the unique tag ids is exported so other plugins can use it
  • The tag color is randomized from a set of known colors

Testing

To test it, I generated a customized version of a the AWS package that has a kibana/tags.yml file that follows this template, since currently there are no existing published packages of this type.
See related package-spec PR

- text: Foo
  asset_types:
    - dashboard
    - search
- text: Bar
  asset_ids:
    - id1
    - id2
- text: myCustomTag
  asset_types:
    - dashboard
    - map
  asset_ids:
    - id1

Note that manifest.yml needs to have at least:

  format_version: 2.10.0
  kibana.version: "^8.10.1"

I then verified that the tags are correctly generated and associated with correct assets:

Screenshot 2023-08-01 at 16 20 38

Checklist

@criamico criamico self-assigned this Jul 27, 2023
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@criamico criamico added the Team:Fleet Team label for Observability Data Collection Fleet team label Jul 27, 2023
@criamico
Copy link
Contributor Author

@elasticmachine merge upstream

@criamico
Copy link
Contributor Author

@elasticmachine merge upstream

@criamico criamico marked this pull request as ready for review August 1, 2023 14:25
@criamico criamico requested a review from a team as a code owner August 1, 2023 14:25
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@criamico criamico added v8.10.0 release_note:skip Skip the PR/issue when compiling release notes labels Aug 1, 2023
@criamico
Copy link
Contributor Author

criamico commented Aug 1, 2023

@elasticmachine merge upstream

hop-dev
hop-dev reviewed Aug 1, 2023
View reviewed changes
Copy link
Contributor

@hop-dev hop-dev left a comment

Choose a reason for hiding this comment

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

The code looks great, it would be good to have a functional test for making sure the tag isn't duplicated if it already exists, do you think that would be do-able?

@criamico
Copy link
Contributor Author

criamico commented Aug 1, 2023 •
edited
Loading

The code looks great, it would be good to have a functional test for making sure the tag isn't duplicated if it already exists, do you think that would be do-able?

Actually I forgot to add that test, I'll add it now

criamico
criamico commented Aug 1, 2023
View reviewed changes
x-pack/plugins/fleet/server/services/epm/kibana/assets/tag_assets.ts
return;
}
const packageSpecAssets = await getPackageSpecTags(taggableAssets, opts);
const groupedAssets = groupByAssetId(packageSpecAssets);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a lot of data manipulation here, trying to explain why.
I was previously trying to update call updateTagAssignments iterating directly on packageSpecAssets, calling by tagId, but for some reason the assets didn't get applied correctly. I suspect that calling the same assets on different ids asynchronously was overriding some of the assets.

So I decided to group by assetId instead, so every asset gets assigned to the tag only once.

@criamico
Copy link
Contributor Author

criamico commented Aug 2, 2023

@elasticmachine merge upstream

@criamico criamico requested a review from hop-dev August 2, 2023 08:48
@criamico criamico requested a review from juliaElastic August 2, 2023 08:48
juliaElastic
juliaElastic reviewed Aug 2, 2023
View reviewed changes
Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

Added a few comments, overall looks good, I suggested to make the tag ids more readable instead of using uuid, this would make the tests more readable too.

x-pack/plugins/fleet/server/services/epm/kibana/assets/tag_assets.test.ts Show resolved Hide resolved
x-pack/plugins/fleet/server/services/epm/kibana/assets/tag_assets.ts Show resolved Hide resolved
x-pack/plugins/fleet/server/services/epm/kibana/assets/tag_assets.test.ts Show resolved Hide resolved
juliaElastic
juliaElastic approved these changes Aug 2, 2023
View reviewed changes
Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes.

@criamico
Copy link
Contributor Author

criamico commented Aug 2, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 1067 1072 +5

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
fleet 40 41 +1
Unknown metric groups

API count

id before after diff
fleet 1183 1188 +5

History

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

cc @criamico

@criamico criamico merged commit 9942fdc into elastic:main Aug 2, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 2, 2023
@juliaElastic juliaElastic mentioned this pull request Aug 14, 2023
Closed
@P1llus P1llus mentioned this pull request Aug 22, 2023
Closed
@criamico criamico mentioned this pull request Aug 23, 2023
Merged
criamico added a commit that referenced this pull request Aug 24, 2023
@criamico
[Fleet] Fix security solution tag id (#164582)
da8d3b2 
Related to #152814

## Summary

Fix for a bug found in #162643 :
The security solution tag is actually created with "Security Solution"
name and "security-solution-default" id (instead of `SecuritySolution`):

```
   {
        "id": "security-solution-default",
        "name": "Security Solution",
        "description": "",
        "color": "#D36086"
    }
```

I found that in same cases the tag is duplicated, since[ is not created
with an unique
id](https://github.com/elastic/kibana/blob/dd0938bea3ebd745a49ac164a7a5f053ba6a138b/x-pack/plugins/security_solution/public/dashboards/containers/use_fetch_security_tags.ts#L44-L51):

<img width="3079" alt="Screenshot 2023-08-23 at 15 07 09"
src="https://github.com/elastic/kibana/assets/16084106/ef885d8f-2e68-4695-aa14-1adc2e326ab0">

I think it's acceptable to use the "security-solution-default", at least
it will create the correct tag name.

### Test
For testing I used a package built locally that uses the `Security
Solution` tag
The steps are the same as described in
#162643

- Additionally, check that the `tags` endpoint in the Tags page has name
and id as described above:
<img width="2521" alt="Screenshot 2023-08-23 at 14 55 05"
src="https://github.com/elastic/kibana/assets/16084106/48b3ddd0-3d04-4ff6-8a24-946902687b13">
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 24, 2023
@criamico
[Fleet] Fix security solution tag id (elastic#164582)
6079cb9 
Related to elastic#152814

## Summary

Fix for a bug found in elastic#162643 :
The security solution tag is actually created with "Security Solution"
name and "security-solution-default" id (instead of `SecuritySolution`):

```
   {
        "id": "security-solution-default",
        "name": "Security Solution",
        "description": "",
        "color": "#D36086"
    }
```

I found that in same cases the tag is duplicated, since[ is not created
with an unique
id](https://github.com/elastic/kibana/blob/dd0938bea3ebd745a49ac164a7a5f053ba6a138b/x-pack/plugins/security_solution/public/dashboards/containers/use_fetch_security_tags.ts#L44-L51):

<img width="3079" alt="Screenshot 2023-08-23 at 15 07 09"
src="https://github.com/elastic/kibana/assets/16084106/ef885d8f-2e68-4695-aa14-1adc2e326ab0">

I think it's acceptable to use the "security-solution-default", at least
it will create the correct tag name.

### Test
For testing I used a package built locally that uses the `Security
Solution` tag
The steps are the same as described in
elastic#162643

- Additionally, check that the `tags` endpoint in the Tags page has name
and id as described above:
<img width="2521" alt="Screenshot 2023-08-23 at 14 55 05"
src="https://github.com/elastic/kibana/assets/16084106/48b3ddd0-3d04-4ff6-8a24-946902687b13">

(cherry picked from commit da8d3b2)
kibanamachine added a commit that referenced this pull request Aug 24, 2023
@kibanamachine @criamico
[8.10] [Fleet] Fix security solution tag id (#164582) (#164664)
834fde2 
# Backport

This will backport the following commits from `main` to `8.10`:
- [[Fleet] Fix security solution tag id
(#164582)](#164582)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Cristina
Amico","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-24T07:20:13Z","message":"[Fleet]
Fix security solution tag id (#164582)\n\nRelated to
https://github.com/elastic/kibana/issues/152814\r\n\r\n##
Summary\r\n\r\nFix for a bug found in
#162643 :\r\nThe security solution
tag is actually created with \"Security Solution\"\r\nname and
\"security-solution-default\" id (instead of
`SecuritySolution`):\r\n\r\n```\r\n {\r\n \"id\":
\"security-solution-default\",\r\n \"name\": \"Security Solution\",\r\n
\"description\": \"\",\r\n \"color\": \"#D36086\"\r\n }\r\n```\r\n\r\nI
found that in same cases the tag is duplicated, since[ is not
created\r\nwith an
unique\r\nid](https://github.com/elastic/kibana/blob/dd0938bea3ebd745a49ac164a7a5f053ba6a138b/x-pack/plugins/security_solution/public/dashboards/containers/use_fetch_security_tags.ts#L44-L51):\r\n\r\n<img
width=\"3079\" alt=\"Screenshot 2023-08-23 at 15 07
09\"\r\nsrc=\"https://github.com/elastic/kibana/assets/16084106/ef885d8f-2e68-4695-aa14-1adc2e326ab0\">\r\n\r\nI
think it's acceptable to use the \"security-solution-default\", at
least\r\nit will create the correct tag name.\r\n\r\n### Test\r\nFor
testing I used a package built locally that uses the
`Security\r\nSolution` tag\r\nThe steps are the same as described
in\r\nhttps://github.com//pull/162643\r\n\r\n-
Additionally, check that the `tags` endpoint in the Tags page has
name\r\nand id as described above:\r\n<img width=\"2521\"
alt=\"Screenshot 2023-08-23 at 14 55
05\"\r\nsrc=\"https://github.com/elastic/kibana/assets/16084106/48b3ddd0-3d04-4ff6-8a24-946902687b13\">","sha":"da8d3b25c91deaba383a6d6a40d00522f7c6f463","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","backport:prev-minor","v8.11.0"],"number":164582,"url":"https://github.com/elastic/kibana/pull/164582","mergeCommit":{"message":"[Fleet]
Fix security solution tag id (#164582)\n\nRelated to
https://github.com/elastic/kibana/issues/152814\r\n\r\n##
Summary\r\n\r\nFix for a bug found in
#162643 :\r\nThe security solution
tag is actually created with \"Security Solution\"\r\nname and
\"security-solution-default\" id (instead of
`SecuritySolution`):\r\n\r\n```\r\n {\r\n \"id\":
\"security-solution-default\",\r\n \"name\": \"Security Solution\",\r\n
\"description\": \"\",\r\n \"color\": \"#D36086\"\r\n }\r\n```\r\n\r\nI
found that in same cases the tag is duplicated, since[ is not
created\r\nwith an
unique\r\nid](https://github.com/elastic/kibana/blob/dd0938bea3ebd745a49ac164a7a5f053ba6a138b/x-pack/plugins/security_solution/public/dashboards/containers/use_fetch_security_tags.ts#L44-L51):\r\n\r\n<img
width=\"3079\" alt=\"Screenshot 2023-08-23 at 15 07
09\"\r\nsrc=\"https://github.com/elastic/kibana/assets/16084106/ef885d8f-2e68-4695-aa14-1adc2e326ab0\">\r\n\r\nI
think it's acceptable to use the \"security-solution-default\", at
least\r\nit will create the correct tag name.\r\n\r\n### Test\r\nFor
testing I used a package built locally that uses the
`Security\r\nSolution` tag\r\nThe steps are the same as described
in\r\nhttps://github.com//pull/162643\r\n\r\n-
Additionally, check that the `tags` endpoint in the Tags page has
name\r\nand id as described above:\r\n<img width=\"2521\"
alt=\"Screenshot 2023-08-23 at 14 55
05\"\r\nsrc=\"https://github.com/elastic/kibana/assets/16084106/48b3ddd0-3d04-4ff6-8a24-946902687b13\">","sha":"da8d3b25c91deaba383a6d6a40d00522f7c6f463"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/164582","number":164582,"mergeCommit":{"message":"[Fleet]
Fix security solution tag id (#164582)\n\nRelated to
https://github.com/elastic/kibana/issues/152814\r\n\r\n##
Summary\r\n\r\nFix for a bug found in
#162643 :\r\nThe security solution
tag is actually created with \"Security Solution\"\r\nname and
\"security-solution-default\" id (instead of
`SecuritySolution`):\r\n\r\n```\r\n {\r\n \"id\":
\"security-solution-default\",\r\n \"name\": \"Security Solution\",\r\n
\"description\": \"\",\r\n \"color\": \"#D36086\"\r\n }\r\n```\r\n\r\nI
found that in same cases the tag is duplicated, since[ is not
created\r\nwith an
unique\r\nid](https://github.com/elastic/kibana/blob/dd0938bea3ebd745a49ac164a7a5f053ba6a138b/x-pack/plugins/security_solution/public/dashboards/containers/use_fetch_security_tags.ts#L44-L51):\r\n\r\n<img
width=\"3079\" alt=\"Screenshot 2023-08-23 at 15 07
09\"\r\nsrc=\"https://github.com/elastic/kibana/assets/16084106/ef885d8f-2e68-4695-aa14-1adc2e326ab0\">\r\n\r\nI
think it's acceptable to use the \"security-solution-default\", at
least\r\nit will create the correct tag name.\r\n\r\n### Test\r\nFor
testing I used a package built locally that uses the
`Security\r\nSolution` tag\r\nThe steps are the same as described
in\r\nhttps://github.com//pull/162643\r\n\r\n-
Additionally, check that the `tags` endpoint in the Tags page has
name\r\nand id as described above:\r\n<img width=\"2521\"
alt=\"Screenshot 2023-08-23 at 14 55
05\"\r\nsrc=\"https://github.com/elastic/kibana/assets/16084106/48b3ddd0-3d04-4ff6-8a24-946902687b13\">","sha":"da8d3b25c91deaba383a6d6a40d00522f7c6f463"}}]}]
BACKPORT-->

Co-authored-by: Cristina Amico <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliaElastic juliaElastic juliaElastic approved these changes

@hop-dev hop-dev hop-dev approved these changes

Assignees

@criamico criamico

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Fleet] Automatically add tags on assets for security integration packages
7 participants
@criamico @apmmachine @elasticmachine @kibana-ci @hop-dev @juliaElastic @kibanamachine