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

[EPM] add/remove package in package settings page #63389

Merged
merged 5 commits into from
Apr 14, 2020
Merged

[EPM] add/remove package in package settings page #63389

merged 5 commits into from
Apr 14, 2020

Conversation

neptunian
Copy link
Contributor

@neptunian neptunian commented Apr 13, 2020 •
edited
Loading

#60699

  • adds ability to directly install a package
  • adds ability to directly uninstall a package
  • stops user from uninstalling a package if there are currently datasources associated with it
  • update confirm install/uninstall modal text

Install
Screen Shot 2020-04-13 at 12 31 16 PM

Install Confirmation
Screen Shot 2020-04-13 at 1 22 33 PM

Installing
Screen Shot 2020-04-13 at 11 44 38 AM

Able to uninstall (no datasources associated with this package exist)
Screen Shot 2020-04-13 at 11 44 10 AM

Uninstall confirmation
Screen Shot 2020-04-13 at 1 53 31 PM

Uninstalling
Screen Shot 2020-04-13 at 12 17 59 PM (2)

Not able to uninstall (datasources associated with this package exist)
Screen Shot 2020-04-13 at 11 42 41 AM

@neptunian neptunian added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project v7.8.0 Team:Fleet Team label for Observability Data Collection Fleet team labels Apr 13, 2020
@neptunian neptunian requested a review from a team April 13, 2020 18:58
@neptunian neptunian self-assigned this Apr 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Feature:EPM)

@jfsiii
Copy link
Contributor

jfsiii commented Apr 13, 2020

Before I read any code, 👏 👏 👏 for the screenshot-for-every-state (or so it seems) in the description.

paul-tavares
paul-tavares reviewed Apr 13, 2020
View reviewed changes
x-pack/plugins/ingest_manager/common/types/rest_spec/datasource.ts
@@ -16,6 +19,14 @@ export interface GetOneDatasourceRequest {
};
}

export interface GetDatasourcesResponse {
Copy link
Contributor

Choose a reason for hiding this comment

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

:) I just added this type to my PR as well - #63373

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jen-huang is working on datasources table and probably adding it as well!

Copy link
Contributor

Choose a reason for hiding this comment

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

@neptunian fwiw, I'm working on Datasources table in #57961

...st_manager/public/applications/ingest_manager/sections/epm/screens/detail/settings_panel.tsx
kuery: `datasources.package.name:${props.name}`,
});
const { name, title } = props;
const packageInstallStatus = getPackageInstallStatus(name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Q. Does Delete apply to Default installed packages? those defined via x-pack/plugins/ingest_manager/common/types/models/epm.ts:252:

export enum DefaultPackages {
  base = 'base',
  system = 'system',
  endpoint = 'endpoint',
}

My concern is the impact to Endpoint if uninstalled.
cc:/ @jonathan-buttner , @kevinlog

Copy link
Contributor

Choose a reason for hiding this comment

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

@paul-tavares they are default installed vs required or always installed, IMO. Perhaps base is always there, but I believe everything else can be removed.

Copy link
Contributor Author

@neptunian neptunian Apr 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@paul-tavares what @jfsiii said. Base can be removed if you hit the endpoint but we do not allow them to remove it through the UI. All other packages can be removed through the UI.

Copy link
Member

Choose a reason for hiding this comment

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

Do not want to block this PR, but I think not to be able to remove a package should not only be a UI feature but also be blocked through the API as otherwise it would mess up things.

jfsiii
jfsiii approved these changes Apr 13, 2020
View reviewed changes
Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

I ran this locally and everything worked great and as expected. Install, uninstall, re-install. etc.

✨Great work ✨

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

@neptunian neptunian merged commit 1a6398b into elastic:master Apr 14, 2020
@neptunian neptunian mentioned this pull request Apr 14, 2020
Merged
neptunian added a commit to neptunian/kibana that referenced this pull request Apr 14, 2020
@neptunian
[EPM] add/remove package in package settings page (elastic#63389)
8f11528 
* fix bug where assets were not being returned, use archive info for assets

* add settings page, add install/remove button and text

* check existence of datasources associated with this package

* add package title variable to text

* update modal text and rename to uninstall
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 15, 2020
@gmmorris
Merge branch 'alerting/alert-services-mock' into alerting/es-query
75fe882 
* alerting/alert-services-mock: (107 commits)
  removed unused import
  added alert services mock and use it in siem
  [Metrics UI] Refactor With* containers to hooks (elastic#59503)
  [NP] Migrate logstash server side code to NP (elastic#63135)
  Clicking cancel in saved query save modal doesn't close it (elastic#62774)
  [Lens] Migration from 7.7 (elastic#62879)
  [Lens] Fix bug where suggestions didn't use filters (elastic#63293)
  Task/linux events (elastic#63400)
  [Remote clusters] guard against usageCollection plugin if unav… (elastic#63284)
  [Uptime] Remove pings graphql (elastic#59392)
  Index Pattern Field class - factor out copy_field code for future typescripting (elastic#63083)
  [EPM] add/remove package in package settings page (elastic#63389)
  Adjust API authorization logging (elastic#63350)
  Revert FTR: add chromium-based Edge browser support (elastic#61684) (elastic#63448)
  [Event Log] Adds namespace into save objects (elastic#62974)
  document code splitting for client code (elastic#62593)
  Escape single quotes surrounded by double quotes (elastic#63229)
  [Endpoint] Update cli mapping to match endpoint package (elastic#63372)
  update in-app links to metricbeat configuration docs (elastic#63295)
  investigation notes field (documentation / metadata) (elastic#63386)
  ...
neptunian added a commit that referenced this pull request Apr 15, 2020
@neptunian
[EPM] add/remove package in package settings page (#63389) (#63460)
383ea04 
* fix bug where assets were not being returned, use archive info for assets

* add settings page, add install/remove button and text

* check existence of datasources associated with this package

* add package title variable to text

* update modal text and rename to uninstall
wayneseymour pushed a commit that referenced this pull request Apr 15, 2020
@neptunian @wayneseymour
[EPM] add/remove package in package settings page (#63389)
6567a30 
* fix bug where assets were not being returned, use archive info for assets

* add settings page, add install/remove button and text

* check existence of datasources associated with this package

* add package title variable to text

* update modal text and rename to uninstall
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paul-tavares paul-tavares paul-tavares left review comments

@ruflin ruflin ruflin left review comments

@jfsiii jfsiii jfsiii approved these changes

Assignees

@neptunian neptunian

Labels
Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@neptunian @elasticmachine @jfsiii @kibanamachine @ruflin @paul-tavares