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] Installation of hidden field #85703

Merged
merged 12 commits into from
Dec 14, 2020
Merged

[Fleet] Installation of hidden field #85703

merged 12 commits into from
Dec 14, 2020

Conversation

kevinlog
Copy link
Contributor

@kevinlog kevinlog commented Dec 11, 2020 •
edited
Loading

Summary

This PR adds the hidden field to package installation which can be used to indicate that a data_stream should be hidden.

Example package PR that will use this field: elastic/endpoint-package#118

See screenshot after installing a package with hidden set to true
image

Checklist

Delete any items that are not applicable to this PR.

@kevinlog kevinlog requested a review from a team December 11, 2020 18:04
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Dec 11, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@kevinlog kevinlog added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Dec 11, 2020
@kevinlog kevinlog changed the title Installation hidden field [Fleet] Installation hidden field Dec 11, 2020
@kevinlog kevinlog mentioned this pull request Dec 11, 2020
Merged
2 tasks
@kevinlog kevinlog changed the title [Fleet] Installation hidden field [Fleet] Installation of hidden field Dec 11, 2020
@kevinlog kevinlog mentioned this pull request Dec 11, 2020
Merged
@ruflin
Copy link
Member

ruflin commented Dec 14, 2020

This PR does not prefix hidden data streams with a . yet it seems. @tsg Should we do it directly?

@ruflin
Copy link
Member

ruflin commented Dec 14, 2020

@kevinlog I got confused at first by the title of the PR as it is about hidden data streams and not fields. I understand that it is a field in the manifest ;-)

@ruflin ruflin requested a review from skh December 14, 2020 08:21
@kevinlog
Copy link
Contributor Author

@elasticmachine merge upstream

skh
skh reviewed Dec 14, 2020
View reviewed changes
x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts Outdated
}: {
type: string;
templateName: string;
mappings: IndexTemplateMappings;
pipelineName?: string | undefined;
packageName: string;
composedOfTemplates: string[];
hidden?: boolean | false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't boolean | false make the field non-optional for TS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the field can still be optionally passed - take a look at this unit test case:
https://github.com/elastic/kibana/pull/85703/files#diff-f1bfa3ccb84f6e245a1f2a762eceeb3f2372bd454578568ce84a43c9c2c47fffR85

I'm thinking that we can drop the | false part or update it to | undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer dropping the | false then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in both places

skh
skh approved these changes Dec 14, 2020
View reviewed changes
Copy link
Contributor

@skh skh left a comment

Choose a reason for hiding this comment

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

Just one question about types. LGTM 👍

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 47129 47889 +760

History

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

jonathan-buttner
jonathan-buttner approved these changes Dec 14, 2020
View reviewed changes
Copy link
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

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

LGTM. Just a note, once you merge one of these you might cause conflicts in your other PR because I think we're touching a couple of the same functions. So merge them slowly and see if conflicts need to be resolved, otherwise we might break master.

@kevinlog kevinlog merged commit 06993c4 into elastic:master Dec 14, 2020
@kevinlog kevinlog deleted the installation-hidden-field branch December 14, 2020 18:46
@kevinlog kevinlog mentioned this pull request Dec 14, 2020
Merged
kevinlog added a commit that referenced this pull request Dec 14, 2020
@kevinlog @nnamdifrankie @kibanamachine
[Fleet] Installation of hidden field (#85703) (#85836)
cdbed54 
Co-authored-by: nnamdifrankie <franklin.ejoh@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: nnamdifrankie <franklin.ejoh@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kevinlog kevinlog mentioned this pull request Jan 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skh skh skh approved these changes

@jonathan-buttner jonathan-buttner jonathan-buttner approved these changes

@pzl pzl Awaiting requested review from pzl

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@kevinlog @elasticmachine @ruflin @kibanamachine @skh @jonathan-buttner @nnamdifrankie