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] Add packagePoliceDelete callback #148509

Merged
merged 1 commit into from
Jan 10, 2023
Merged

[Fleet] Add packagePoliceDelete callback #148509

merged 1 commit into from
Jan 10, 2023

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Jan 7, 2023 •
edited
Loading

Required by: #147650

This PR adds a callback that runs just before a package policy is deleted. This is needed for the APM package (and possible other use cases) where API keys embedded in the package policy will need to be deleted/invalidated so they don't linger in the system after the policy has been removed.

The new callback is called packagePoliceDelete. A delete callback already exists, however, this callback is called after the policy is deleted.

Renaming in this PR:

  • postPackagePolicyDelete -> packagePolicyPostDelete
  • DeletePackagePoliciesResponse -> PostDeletePackagePoliciesResponse
  • PostPackagePolicyDeleteCallback -> PostPackagePolicyPostDeleteCallback

@sorenlouv sorenlouv requested review from a team as code owners January 7, 2023 13:36
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jan 7, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@sorenlouv sorenlouv added release_note:skip Skip the PR/issue when compiling release notes v8.7.0 labels Jan 7, 2023
@sorenlouv sorenlouv changed the title [Fleet] Add packagePolicyPostDelete and packagePoliceDelete callback [Fleet] Add packagePoliceDelete callback (rename existing packagePoliceDelete to packagePolicyPostDelete) Jan 7, 2023
@sorenlouv sorenlouv changed the title [Fleet] Add packagePoliceDelete callback (rename existing packagePoliceDelete to packagePolicyPostDelete) [Fleet] Add packagePoliceDelete callback Jan 7, 2023
@sorenlouv
Copy link
Member Author

sorenlouv commented Jan 8, 2023 •
edited
Loading

@dhurley14 Can you help me understand why the security tests fail, and how to fix them?

[job] [logs] Security Solution Tests #4 / Add exception using data views from rule details Creates an exception item from alert actions overflow menu
[job] [logs] Security Solution Tests #4 / Add exception using data views from rule details Creates an exception item from alert actions overflow menu

My changes are mostly type related. Only runtime impacting change is renaming 'postPackagePolicyDelete' to 'packagePolicyPostDelete'.

Thanks!

@sorenlouv sorenlouv mentioned this pull request Jan 9, 2023
Merged
ofiriro3
ofiriro3 reviewed Jan 9, 2023
View reviewed changes
Copy link
Contributor

@ofiriro3 ofiriro3 left a comment

Choose a reason for hiding this comment

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

LGTM - cloud security posture.

@orouz Would you like to double verify me?

@nchaulet nchaulet self-requested a review January 9, 2023 12:16
gergoabraham
gergoabraham approved these changes Jan 9, 2023
View reviewed changes
Copy link
Contributor

@gergoabraham gergoabraham left a comment

Choose a reason for hiding this comment

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

OLM changes look good to me 👍

@dhurley14
Copy link
Contributor

@sqren it seems like those cypress tests are failing on other PR's as well.. We will look into it.

@dhurley14
Copy link
Contributor

Should be resolved once this PR is merged: #148596

I think this is a flaky cypress test (the test has a rule executing every second which, on the CI container's limited resources, could prevent kibana from loading. This seems similar to the fix here: #147349)

@sorenlouv sorenlouv force-pushed the add-fleet-pre-delete-callback branch from d7afb4e to 0d18523 Compare January 9, 2023 21:47
@sorenlouv sorenlouv requested a review from a team as a code owner January 9, 2023 21:47
nchaulet
nchaulet approved these changes Jan 10, 2023
View reviewed changes
Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

Fleet changes looks good to me 🚀

@sorenlouv sorenlouv removed the request for review from joeypoon January 10, 2023 13:03
@sorenlouv sorenlouv force-pushed the add-fleet-pre-delete-callback branch from 922a2de to 8e763f9 Compare January 10, 2023 13:58
@sorenlouv sorenlouv removed the request for review from a team January 10, 2023 14:00
patrykkopycinski
patrykkopycinski approved these changes Jan 10, 2023
View reviewed changes
Copy link
Contributor

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

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

LGTM

@sorenlouv sorenlouv enabled auto-merge (squash) January 10, 2023 15:01
@dhurley14
Copy link
Contributor

dhurley14 commented Jan 10, 2023 •
edited
Loading

@sqren I think someone from @elastic/security-threat-hunting should be able to help investigate the flakiness of the other cypress tests. I'm looking into why the exceptions one is failing on your branch.

@sorenlouv
Copy link
Member Author

@sqren I think someone from @elastic/security-threat-hunting should be able to help investigate the flakiness of the other cypress tests. I'm looking into why the exceptions one is failing on your branch.

That would be great, thanks. I'll try to re-run the build but will post the failing tests here for visibility

[job] [logs] Security Solution Tests #4 / Add exception using data views from rule details Creates an exception item from alert actions overflow menu
[job] [logs] Security Solution Tests #1 / Detections : Page Filters Alert list is updated when the alerts are updated
[job] [logs] Security Solution Tests #3 / Timelines Creates a timeline by clicking untitled timeline from bottom bar can be added notes
[job] [logs] Security Solution Tests #3 / Timelines Creates a timeline by clicking untitled timeline from bottom bar can be added notes

@sorenlouv
Copy link
Member Author

Buildkite, test this

@sorenlouv
[Fleet] Add packagePolicyPostDelete and packagePoliceDelete
958b253 
Add tests

Rename `PackagePolicyPostCreateCallback` back to `PostPackagePolicyPostCreateCallback`

Rename `PackagePolicyCreateCallback` back to `PostPackagePolicyCreateCallback`

Add “Post” prefix

Use `packagePolicyService.getByIDs`
@sorenlouv sorenlouv force-pushed the add-fleet-pre-delete-callback branch from 8e763f9 to 958b253 Compare January 10, 2023 17:58
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #4 / Add exception using data views from rule details Creates an exception item from alert actions overflow menu
  • [job] [logs] FTR Configs #9 / alerting api integration security and spaces enabled Alerts - Group 1 alerts find public superuser at space1 should handle find alert request appropriately
  • [job] [logs] Security Solution Tests #1 / Detections : Page Filters Alert list is updated when the alerts are updated

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 927 929 +2

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 21 23 +2
Unknown metric groups

API count

id before after diff
fleet 1033 1035 +2

History

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

@sorenlouv sorenlouv merged commit bd095f5 into elastic:main Jan 10, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 10, 2023
@sorenlouv sorenlouv deleted the add-fleet-pre-delete-callback branch January 10, 2023 19:55
sorenlouv added a commit that referenced this pull request Jan 11, 2023
@sorenlouv
[APM] Add api key to APM package policies (#147650)
96b7629 
Closes #146368
Depends on: #148509

This adds an api key to every APM package policy. This will make it
possible for APM Server to query the source map index (
`.apm-source-map` ) and the agent config index
(`.apm-agent-configuration`)
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
@sorenlouv @jennypavlova
[Fleet] Add packagePoliceDelete callback (elastic#148509)
94cfa8d
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
@sorenlouv @jennypavlova
[APM] Add api key to APM package policies (elastic#147650)
5b07c84 
Closes elastic#146368
Depends on: elastic#148509

This adds an api key to every APM package policy. This will make it
possible for APM Server to query the source map index (
`.apm-source-map` ) and the agent config index
(`.apm-agent-configuration`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ofiriro3 ofiriro3 ofiriro3 left review comments

@nchaulet nchaulet nchaulet approved these changes

@patrykkopycinski patrykkopycinski patrykkopycinski approved these changes

@orouz orouz orouz approved these changes

@gergoabraham gergoabraham gergoabraham approved these changes

Assignees
No one assigned
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.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@sorenlouv @elasticmachine @dhurley14 @kibana-ci @nchaulet @patrykkopycinski @ofiriro3 @orouz @gergoabraham @kibanamachine