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

[Ingest Manager] Support limiting integrations on an agent config #70542

Merged
merged 20 commits into from
Jul 6, 2020

Conversation

jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Jul 2, 2020

Summary

Resolves #64371. This PR supports the package definition config_templates.[0].multiple: false field that indicates whether or not a package is allowed to be added to an agent config more than once. Currently the only package that uses this field is endpoint.

Server-side changes:

  • Adds API endpoint GET /api/ingest_manager/epm/packages/limited that retrieves a list of package names which are limited
  • Adds a query parameter full to agent config list endpoint GET /api/ingest_manager/agent_configs?full=true to retrieve expanded package_configs information on each agent config
    • The default behavior (i.e. without the parameter), remains unchanged: list endpoint will return an array of SO IDs for package_configs field
  • Adjusts API endpoint POST /api/ingest_manager/package_configs to return an error if a package config is using a 1) limited package and 2) is being added to an agent config which already has another package config using the same limited package
    • In addition, some functionality which were being executed by the handler for this endpoint, have been moved to packageConfigService.create() instead

Client-side changes:

  • On Agent config > Add integration page, remove limited integrations which have already been added to that agent config from the list of available integrations
  • On Integration > Add [integration name page, for limited integrations, disable agent configs which already have that integration added

API integration changes:

  • Dockerized EPM tests were moved to their own directory: /ingest_manager_api_integration/apis/epm
  • New directory created for package config API integration tests: /ingest_manager_api_integration/apis/package_config
    • create.ts file created containing basic tests covering creating package configs
    • Note: a test was written to cover the erroring case for limited packages described above, but is skipped ATM due to the docker image not containing recent package registry changes needed for the test to succeed
      • Unskipped as the docker image was updated!

Testing

  • Testing can be done in the UI by opening Agent config > Add integration page in two tabs, selecting Endpoint in both and submitting the first tab. Attempting to submit the second tab should result in a toast error message.
  • Similar steps for the Integration flow.

Screenshots

Jul-02-2020 14-09-57

Jul-02-2020 14-10-11

image

@jen-huang jen-huang self-assigned this Jul 2, 2020
@jen-huang jen-huang marked this pull request as ready for review July 2, 2020 21:13
@jen-huang jen-huang requested a review from a team July 2, 2020 21:13
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jul 2, 2020
@elasticmachine
Copy link
Contributor

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

@jen-huang jen-huang added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0 labels Jul 2, 2020
@jen-huang jen-huang changed the title [WIP][Ingest Manager] Support limiting integrations on an agent config [Ingest Manager] Support limiting integrations on an agent config Jul 2, 2020
@hbharding
Copy link
Contributor

Thanks Jen. LGTM visually

@@ -3,6 +3,7 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { HttpFetchQuery } from 'src/core/public';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this? It appears unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bad merge :)

(pkg) => (pkg.status = InstallationStatus.installed)
);
const installedPackagesInfo = await Promise.all(
installedPackages.map(async (pkgInstall) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can drop async since we're not using await and getPackageInfo is already async

})
);
return installedPackagesInfo
.filter((pkgInfo) => isPackageLimited(pkgInfo))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: no need for the lambda

Suggested change
.filter((pkgInfo) => isPackageLimited(pkgInfo))
.filter(isPackageLimited)

packageConfig: NewPackageConfig,
options?: { id?: string; user?: AuthenticatedUser }
): Promise<PackageConfig> {
// Make sure the associated package is installed
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do these requests in parallel?

Do we have to wait for ensureInstalledPackage to complete before calling getPackageInfo?


// EPM
loadTestFile(require.resolve('./epm/list'));
loadTestFile(require.resolve('./epm/list'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this should be changed to import a different file or removed

@jen-huang
Copy link
Contributor Author

jen-huang commented Jul 6, 2020

Updated PR with review feedback. I also re-enabled the skipped API integration test as the docker image was updated in #70716 🎉

}): Promise<string[]> {
const { savedObjectsClient } = options;
const allPackages = await getPackages({ savedObjectsClient });
const installedPackages = allPackages.filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can skip the get & filter by calling getPackageSavedObjects. I don't know if it'll be a drop-in replacement, but it will start with the installed packages instead of getting all from the registry & filtering.

Not a blocker, just a comment

Copy link
Contributor Author

@jen-huang jen-huang Jul 6, 2020

Choose a reason for hiding this comment

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

It will end up being the same number of calls because getPackageSavedObjects just returns the saved objects, which do not contain the necessary PackageInfo fields (from the registry) to detect if the package is limited or not

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
ingestManager 358 +1 357

History

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

@jen-huang jen-huang merged commit 7debf4d into elastic:master Jul 6, 2020
@jen-huang jen-huang deleted the ingest/limit-integrations branch July 6, 2020 21:12
@jen-huang
Copy link
Contributor Author

Holding off on backporting until backport #70862 is merged, to avoid conflicts.

jen-huang added a commit that referenced this pull request Jul 7, 2020
…0542) (#70895)

* Add API endpoint and hook for retrieving restricted packages

* Filter out restricted packages already in use from list of integrations available for an agent config

* Allow list agent configs to optionally return expanded package configs, re

* Filter out agent configs which already use the restricted package already from list of agent configs available for an integration

* Allow more than 20 agent configs to be shown

* Rename restricted to limited; add some common methods to DRY

* Add limited package check on server side

* Adjust copy wording

* Fix typings

* Add some package config api integration tests, update es archive mappings

* Move test to dockerized integation tests directory; move existing epm tests to their own directory

* Remove extra assignPackageConfigs() - already handled in packageConfigService.create()

* Review fixes

* Fix type, reenabled skipped test

* Move new EPM integration test file
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 7, 2020
* master: (53 commits)
  [Composable template] Details panel + delete functionality (elastic#70814)
  [Uptime] Ping list body scroll (elastic#70781)
  moving indexPattern.delete() to indexPatterns.delete(indexPattern) (elastic#70430)
  Adapt expected response of advanced settings feature control for cloud tests (elastic#70793)
  skip flaky suite (elastic#70885)
  skip flaky suite (elastic#67814)
  skip flaky suite (elastic#70906)
  Revert "reenable regression and classification functional tests (elastic#70661)" (elastic#70908)
  Added UI validation when creating a Webhook connector with invalid URL (elastic#70025)
  [Security Solution] Change default index pattern (elastic#70797)
  ServiceNow push to Incident generic implementation (supporting both Case specific and generic Alerts) (elastic#68464)
  add button link to ingest (elastic#70142)
  reenable regression and classification functional tests (elastic#70661)
  [Component templates] Form wizard (elastic#69732)
  [Ingest Manager] Copy changes (elastic#70828)
  Adding test user to maps functional tests - PR 1 (elastic#70649)
  [Ingest Manager] Support limiting integrations on an agent config (elastic#70542)
  skip flaky suite (elastic#70880)
  [Metrics UI] Fix a bug in Metric Threshold query filter construction (elastic#70672)
  upgrade caniuse-lite database (elastic#70833)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 7, 2020
* master: (46 commits)
  [Composable template] Details panel + delete functionality (elastic#70814)
  [Uptime] Ping list body scroll (elastic#70781)
  moving indexPattern.delete() to indexPatterns.delete(indexPattern) (elastic#70430)
  Adapt expected response of advanced settings feature control for cloud tests (elastic#70793)
  skip flaky suite (elastic#70885)
  skip flaky suite (elastic#67814)
  skip flaky suite (elastic#70906)
  Revert "reenable regression and classification functional tests (elastic#70661)" (elastic#70908)
  Added UI validation when creating a Webhook connector with invalid URL (elastic#70025)
  [Security Solution] Change default index pattern (elastic#70797)
  ServiceNow push to Incident generic implementation (supporting both Case specific and generic Alerts) (elastic#68464)
  add button link to ingest (elastic#70142)
  reenable regression and classification functional tests (elastic#70661)
  [Component templates] Form wizard (elastic#69732)
  [Ingest Manager] Copy changes (elastic#70828)
  Adding test user to maps functional tests - PR 1 (elastic#70649)
  [Ingest Manager] Support limiting integrations on an agent config (elastic#70542)
  skip flaky suite (elastic#70880)
  [Metrics UI] Fix a bug in Metric Threshold query filter construction (elastic#70672)
  upgrade caniuse-lite database (elastic#70833)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 7, 2020
* actions/feature: (46 commits)
  [Composable template] Details panel + delete functionality (elastic#70814)
  [Uptime] Ping list body scroll (elastic#70781)
  moving indexPattern.delete() to indexPatterns.delete(indexPattern) (elastic#70430)
  Adapt expected response of advanced settings feature control for cloud tests (elastic#70793)
  skip flaky suite (elastic#70885)
  skip flaky suite (elastic#67814)
  skip flaky suite (elastic#70906)
  Revert "reenable regression and classification functional tests (elastic#70661)" (elastic#70908)
  Added UI validation when creating a Webhook connector with invalid URL (elastic#70025)
  [Security Solution] Change default index pattern (elastic#70797)
  ServiceNow push to Incident generic implementation (supporting both Case specific and generic Alerts) (elastic#68464)
  add button link to ingest (elastic#70142)
  reenable regression and classification functional tests (elastic#70661)
  [Component templates] Form wizard (elastic#69732)
  [Ingest Manager] Copy changes (elastic#70828)
  Adding test user to maps functional tests - PR 1 (elastic#70649)
  [Ingest Manager] Support limiting integrations on an agent config (elastic#70542)
  skip flaky suite (elastic#70880)
  [Metrics UI] Fix a bug in Metric Threshold query filter construction (elastic#70672)
  upgrade caniuse-lite database (elastic#70833)
  ...
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.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Ingest Manager] Support limiting number of data sources
5 participants