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

Compute AAD to encrypty/decrypt SO only if needed #75818

Merged
merged 2 commits into from
Aug 26, 2020

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Aug 24, 2020

Summary

While working on optimizing Fleet performance I noticed we compute the AAD for encrypting saved object even if we do not encrypt any attribute.

In Fleet this mean a lot of AAD get can computed for nothing, for example we are doing a periodical call to do a bulk update on every agent to set a timestamp last_checkin a field that is not encrypted.

I think we can avoid that call when not needed.

Screen Shot 2020-08-24 at 5 27 38 PM

TODO

  • Run this change against fleet load tests

This change seems to have a limited impact on fleet performance, but it still thinks looks like a small improvment

@nchaulet nchaulet requested a review from a team August 24, 2020 21:28
@nchaulet nchaulet requested a review from a team as a code owner August 24, 2020 21:28
@nchaulet nchaulet self-assigned this Aug 24, 2020
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Aug 24, 2020
@elasticmachine
Copy link
Contributor

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

@nchaulet nchaulet added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Aug 24, 2020
@nchaulet nchaulet force-pushed the feature-lazy-add-encryption branch from 022b17f to e189d1c Compare August 24, 2020 21:30
@nchaulet nchaulet changed the title Feature lazy add encryption Compute AAD to encrypty/decrypt SO only if needed Aug 24, 2020
@nchaulet nchaulet force-pushed the feature-lazy-add-encryption branch from e189d1c to 1674914 Compare August 25, 2020 01:38
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

  • 💔 Build #70164 failed e189d1c2312b33c2666ec87c3577258226aedfce

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

@nchaulet nchaulet requested a review from kobelb August 25, 2020 12:35
Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

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

This approach seems reasonable to me, I ultimately defer my approval to @elastic/kibana-security as they're more familiar with this code than I.

Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

LGTM

@nchaulet nchaulet merged commit 63265b6 into elastic:master Aug 26, 2020
@nchaulet nchaulet deleted the feature-lazy-add-encryption branch August 26, 2020 12:50
nchaulet added a commit to nchaulet/kibana that referenced this pull request Aug 26, 2020
nchaulet added a commit that referenced this pull request Aug 26, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants