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

[Security Solution] Initiate endpoint package upgrade from security app #77498

Merged

Conversation

jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Sep 15, 2020

This PR will check to see if the endpoint package is installed and up to date when a user navigates to the security solution. It will perform the check on all of the tabs within the app and will run when a user navigates between the tabs.

There's no notification to the user as that work was postponed for now.

Before attempting to get information about the endpoint package, the component will check the current user's permissions to see if they are privileged enough to do the installation.

This PR is waiting for: #77827 to be merged and then a all the ingest manager will go away.

Endpoint Package Upgrade GIF

endpoint_upgrade

@jonathan-buttner jonathan-buttner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver v7.10.0 labels Sep 15, 2020
@@ -30,6 +31,7 @@ export const renderApp = ({
}: RenderAppProps) => {
render(
<SecurityApp apolloClient={apolloClient} history={history} services={services} store={store}>
<UpgradeEndpointPackage />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oatkiller @paul-tavares is this the right place to do this if we want the functionality to run on any of the tabs? We'll probably want to the upgrade check to run as often as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also @XavierM any thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually having it run when a user navigates between tab seems a bit excessive. If you have ideas for limiting this so it only runs when the user initially navigates to any page on the security app that'd be helpful. That way we don't have to perform the api requests when we navigate between tabs.

Copy link
Contributor

@XavierM XavierM Sep 23, 2020

Choose a reason for hiding this comment

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

Yes, I think we can add that in the start plugin.js, and we can store it in the reducer store and then have a component reading this value to show something or not. what do you think?

I have PR which going to merge in master soon and can show you what we did there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@XavierM will that cause the code to run less frequently?
@jonathan-buttner Why is it important that the code runs less frequently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After discussing this with Rob, I think we're just going to stick with doing it when any of the plugins are started because it's more simple.

* @param packageKey a string in the form of <package name>-<package version>
* @param options an object containing options for the request
*/
const sendGetPackageInfo = async (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oatkiller @paul-tavares where should these functions live? The management and policy pages have some similar functions and are within a services directory under those pages https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/public/management/pages/policy/store/policy_list/services/ingest.ts#L137

Copy link
Contributor

Choose a reason for hiding this comment

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

I would defer to @XavierM or @spong for this. I don't know about the structure of this part of the app.

x-pack/plugins/security_solution/public/app/upgrade.ts Outdated Show resolved Hide resolved
@jonathan-buttner jonathan-buttner marked this pull request as ready for review September 24, 2020 14:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility)

@XavierM
Copy link
Contributor

XavierM commented Sep 24, 2020

I am just wondering if we need to add telemetry over there, to know if we are doing or not. It might be already done on the back end.

@jonathan-buttner
Copy link
Contributor Author

jonathan-buttner commented Sep 24, 2020

I am just wondering if we need to add telemetry over there, to know if we are doing or not. It might be already done on the back end.

Oh good point. @michaelolo24 @skh do you recall if we have telemetry on the installed packages via the ingest manager? So when a package is installed/upgraded will that be reflected in the telemetry?

@@ -58,6 +59,8 @@ const HomePageComponent: React.FC<HomePageProps> = ({ children }) => {
const [showTimeline] = useShowTimeline();

const { browserFields, indexPattern, indicesExist } = useSourcererScope();
// side effect: this will attempt to upgrade the endpoint package if it is not up to date
Copy link
Contributor

Choose a reason for hiding this comment

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

❔ Elaborating to explain when/how often that will happen and why may be helpful to the reader.

const abortController = new AbortController();

// cancel any ongoing requests
const cleanup = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

cleanup as a name suggests this should always be done, which is a little confusing because this sends an abort through the connection. Rename or clarify, perhaps?

Copy link
Contributor

@bkimmel bkimmel left a comment

Choose a reason for hiding this comment

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

Left a few questions

@jonathan-buttner
Copy link
Contributor Author

I am just wondering if we need to add telemetry over there, to know if we are doing or not. It might be already done on the back end.

Oh good point. @michaelolo24 @skh do you recall if we have telemetry on the installed packages via the ingest manager? So when a package is installed/upgraded will that be reflected in the telemetry?

@XavierM The ingest manager app handles collecting telemetry on the installed packages: https://github.com/elastic/kibana/blob/master/x-pack/plugins/ingest_manager/server/collectors/package_collectors.ts

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id value diff baseline
securitySolution 1970 +1 1969

async chunks size

id value diff baseline
securitySolution 10.2MB +4.3KB 10.2MB

History

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

@jonathan-buttner jonathan-buttner merged commit 768ae21 into elastic:master Sep 28, 2020
@jonathan-buttner jonathan-buttner deleted the upgrade-endpoint-package branch September 28, 2020 18:18
jonathan-buttner added a commit to jonathan-buttner/kibana that referenced this pull request Sep 28, 2020
…pp (elastic#77498)

* Working on package update functionality

* Correctly installing package

* Moving upgrade component and working upgrade

* Doing permissions check

* Cleaning up imports

* Adding bulk upgrade api

* Addressing comments

* Removing todo

* Changing body field

* Adding helper for getting the bulk install route

* Adding request spec

* Using bulk install endpoint from ingest

* Moving component to a hook

* Addressing feedback
scottybollinger pushed a commit to scottybollinger/kibana that referenced this pull request Sep 28, 2020
…pp (elastic#77498)

* Working on package update functionality

* Correctly installing package

* Moving upgrade component and working upgrade

* Doing permissions check

* Cleaning up imports

* Adding bulk upgrade api

* Addressing comments

* Removing todo

* Changing body field

* Adding helper for getting the bulk install route

* Adding request spec

* Using bulk install endpoint from ingest

* Moving component to a hook

* Addressing feedback
jonathan-buttner added a commit that referenced this pull request Sep 28, 2020
…pp (#77498) (#78660)

* Working on package update functionality

* Correctly installing package

* Moving upgrade component and working upgrade

* Doing permissions check

* Cleaning up imports

* Adding bulk upgrade api

* Addressing comments

* Removing todo

* Changing body field

* Adding helper for getting the bulk install route

* Adding request spec

* Using bulk install endpoint from ingest

* Moving component to a hook

* Addressing feedback
phillipb added a commit to phillipb/kibana that referenced this pull request Sep 29, 2020
…a into add-anomalies-to-timeline

* 'add-anomalies-to-timeline' of github.com:phillipb/kibana: (89 commits)
  Aligns several module versions across the repository (elastic#78327)
  Empty prompt and loading spinner for service map (elastic#78382)
  Change progress bar to spinner (elastic#78460)
  [QA][Code Coverage] Coverage teams lookup w/o Additional Config (elastic#77111)
  Slim down core bundle (elastic#75912)
  [Alerting] retry internal OCC calls within alertsClient (elastic#77838)
  [kbn/optimizer] only build xpack examples when building xpack plugins (elastic#78656)
  [Ingest Manager] Ingest setup upgrade (elastic#78081)
  [Ingest Manager] Surface saved object client 10,000 limitation to bulk actions UI (elastic#78520)
  fix name without a category or if field end with .text (elastic#78655)
  [Security Solution] [Detections] Log message enhancements (elastic#78429)
  [ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url (elastic#78303)
  [Enterprise Search] Remove all instances of KibanaContext to Kea store (elastic#78513)
  [ML] DF Analytics creation: ensure job did not fail to start before showing results link (elastic#78200)
  fix createAppNavigationHandler to use `navigateToUrl` (elastic#78583)
  Fixing a11y test failure on discover app (elastic#59975) (elastic#77614)
  [Security Solution] Initiate endpoint package upgrade from security app (elastic#77498)
  [kbn/es] use a basic build process (elastic#78090)
  [kbn/optimizer] fix .json extension handling (elastic#78524)
  Fix APM lodash imports (elastic#78438)
  ...
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:Endpoint Data Visibility Team managing the endpoint resolver v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants