Skip to content

[Cases] Performance and RBAC improvements#101465

Merged
cnasikas merged 9 commits intoelastic:masterfrom
cnasikas:cases_rbac_improv
Jun 8, 2021
Merged

[Cases] Performance and RBAC improvements#101465
cnasikas merged 9 commits intoelastic:masterfrom
cnasikas:cases_rbac_improv

Conversation

@cnasikas
Copy link
Copy Markdown
Member

@cnasikas cnasikas commented Jun 7, 2021 •
edited
Loading

Summary

Resolves: #100804

For maintainers

@cnasikas cnasikas added v8.0.0 Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.14.0 Theme: rac label obsolete Feature:Cases-RAC-RBAC labels Jun 7, 2021
@cnasikas cnasikas self-assigned this Jun 7, 2021
@cnasikas cnasikas changed the base branch from master to cases-rbac-poc June 7, 2021 09:38
This was referenced Jun 7, 2021
Merged
Closed
@cnasikas cnasikas marked this pull request as ready for review June 7, 2021 13:16
@cnasikas cnasikas requested a review from a team as a code owner June 7, 2021 13:16
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jun 7, 2021

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jun 7, 2021

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@jonathan-buttner jonathan-buttner deleted the branch elastic:master June 7, 2021 13:37
@jonathan-buttner jonathan-buttner changed the base branch from cases-rbac-poc to master June 7, 2021 13:47
@jonathan-buttner jonathan-buttner requested review from a team as code owners June 7, 2021 13:47
@cnasikas cnasikas force-pushed the cases_rbac_improv branch from fda09f4 to 44bfa52 Compare June 7, 2021 13:51
@mshustov mshustov removed the request for review from a team June 7, 2021 15:53
@mshustov
Copy link
Copy Markdown
Contributor

mshustov commented Jun 7, 2021

Don't see any changes related to Core, but the approach LGTM.

@cnasikas cnasikas removed the request for review from a team June 8, 2021 06:55
@cnasikas cnasikas added the release_note:skip Skip the PR/issue when compiling release notes label Jun 8, 2021
@cnasikas
Copy link
Copy Markdown
Member Author

cnasikas commented Jun 8, 2021

Don't see any changes related to Core, but the approach LGTM.

Thank you @mshustov! This PR was based on another PR (the RBAC PR) that touched some of your files. The PR got merged and I switched the base to master and your team left as a reviewer. Sorry for the trouble.

@cnasikas
Copy link
Copy Markdown
Member Author

cnasikas commented Jun 8, 2021

@elasticmachine merge upstream

@cnasikas cnasikas force-pushed the cases_rbac_improv branch from 676403c to b06d2b6 Compare June 8, 2021 13:03
@cnasikas cnasikas force-pushed the cases_rbac_improv branch from b06d2b6 to 075364c Compare June 8, 2021 13:08
jonathan-buttner
jonathan-buttner reviewed Jun 8, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Looking good, just two more places I think we need the 10k perPage:
https://github.com/elastic/kibana/blob/master/x-pack/plugins/cases/server/services/cases/index.ts#L402

https://github.com/elastic/kibana/blob/master/x-pack/plugins/cases/server/common/models/commentable_case.ts#L328

x-pack/plugins/cases/server/client/cases/delete.ts
});

// Ensuring we don't too many concurrent get running.
const comments = await pMap(ids, getCommentsMapper, {
Copy link
Copy Markdown
Contributor

@jonathan-buttner jonathan-buttner Jun 8, 2021

Choose a reason for hiding this comment

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

nit: This isn't a change you made but if I understand the previous code correctly comments will be an array of saved object find responses (essentially an array of arrays [[1,2,3],[4,5,6]]). If we flattened it I wonder if it would make the deleteCommentsMapper a little easier to understand 🤷‍♂️ . That way we could avoid the pMap inside of the mapper.

Copy link
Copy Markdown
Member Author

@cnasikas cnasikas Jun 8, 2021

Choose a reason for hiding this comment

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

That's smart! As we gonna use search_after to iterate over all objects I will leave it for another PR and create a ticket to track it.

x-pack/plugins/cases/server/client/cases/find.ts

ensureSavedObjectsAreAuthorized([...cases.casesMap.values()]);

// casesStatuses are bounded by us. No need to limit concurrent calls.
Copy link
Copy Markdown
Contributor

@jonathan-buttner jonathan-buttner Jun 8, 2021

Choose a reason for hiding this comment

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

👍

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 8, 2021

💚 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
cases 382 381 -1

Page load bundle

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

id before after diff
cases 133.5KB 133.7KB +228.0B
Unknown metric groups

API count

id before after diff
cases 407 406 -1

History

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

cc @cnasikas

@cnasikas cnasikas merged commit 13e20e7 into elastic:master Jun 8, 2021
@cnasikas cnasikas deleted the cases_rbac_improv branch June 8, 2021 16:01
@cnasikas cnasikas mentioned this pull request Jun 8, 2021
Merged
cnasikas added a commit that referenced this pull request Jun 8, 2021
@cnasikas @kibanamachine
[Cases] Performance and RBAC improvements (#101465) (#101652)
f2a5c47 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Assignees

@cnasikas cnasikas

Labels

release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team Theme: rac label obsolete v7.14.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Cases] Performance and RBAC improvements

5 participants

@cnasikas @elasticmachine @mshustov @kibanamachine @jonathan-buttner