[Fleet] Support browsing granular integrations by jen-huang · Pull Request #99866 · elastic/kibana

jen-huang · 2021-05-11T23:14:50Z

Summary

Part of #93315. This PR has two main parts:

Allow users to browse "granular integrations"

Screenshots

Granular AWS integration tiles on browse page

Integration specific detail page

Installed integrations page, single AWS tile

What this means: packages can now export multiple integrations by exporting multiple policy_templates in their package definition. Previously, all packages exported 1 policy template. Now, we have changes coming to the AWS package (Adjust aws package to use input groups integrations#767) so that it exports multiple AWS integrations: AWS CloudWatch, AWS DynamoDB, etc.
Each one of these integrations need to show as a separate icon/tile in Integrations > All integrations. The search functionality should work normally with these new tiles. Clicking one of these tiles will bring you to the integration detail page that shows that integration's specific icon, screenshots, and README content.
In Integrations > Installed integrations, only the top-level package tile should show (aka only AWS tile) for now.
All changes should be BWC with previous packages (aka packages that only export 1 policy template).

Support package-level vars (user configurable variables)

Screenshots

Policy editor "Integrations settings" area expanded to include package-level vars

Package-level vars are used properly while compiling final agent YAML

Previously we only supported vars at the input- and stream- level. Now, packages can start using package-level vars. Fleet will use these package-level vars when compiling the final agent YAML.

How to test

TLDR Build in-flight AWS package and load it into your local package registry

Get elastic-package https://github.com/elastic/elastic-package
Pull down Adjust aws package to use input groups integrations#767
From that PR, cd into <integrations>packages/aws
Run elastic-package build, make note of the built aws/0.6.0 package at <integrations>/build
Pull down and build package-registry https://github.com/elastic/package-registry
Copy the built 0.6.0 package to <package-registry>/build/package-storage/packages/aws
Run the package registry go run .
Edit kibana.dev.yml to use your local package registry by adding: xpack.fleet.registryUrl: http://localhost:8080
Start Kibana and observe the above changes to Integrations UI and policy editor. Play around to make sure I didn't break any other packages

What's next

You'll notice that the policy editor for the AWS package only displays "Collect billing metrics". This is expected for now. The second part of the work to fully support all the requirements in #93315 is to adjust the policy editor to support multiple policy_templates too. Originally I tried putting everything in one PR but that second part is a bit more complex and it was holding up the rest of the work from going in 🙃

…evel vars

…page

elasticmachine · 2021-05-11T23:14:52Z

Pinging @elastic/fleet (Team:Fleet)

jen-huang · 2021-05-11T23:35:22Z

@kpollich You might be interested in reviewing this PR and running through the above testing steps as it relates to what you've been working on. I apologize in advance that you might need to resolve even more conflicts with the top-level Integration UI work if this PR makes it in before that 😅

kpollich · 2021-05-13T15:41:26Z

@kpollich You might be interested in reviewing this PR and running through the above testing steps as it relates to what you've been working on. I apologize in advance that you might need to resolve even more conflicts with the top-level Integration UI work if this PR makes it in before that 😅

Thanks, @jen-huang - definitely touches a lot of the same files that will be moved as part of #99848. I think it definitely makes the most sense to aim for landing this PR first before we move integrations to a separate app. It will be easier to fix conflicts by just "re-moving" files around instead of recreating your changes on a bunch of newly relocated/renamed files.

…ration-list

jen-huang · 2021-05-20T00:02:27Z

@elasticmachine merge upstream

jen-huang · 2021-05-24T18:47:30Z

@elasticmachine merge upstream

kpollich

I ran through the local testing instructions in the PR description and everything seems good on my end. This should get a second review from someone with more experience on the Fleet codebase, though.

kpollich · 2021-05-25T13:44:38Z

...s/fleet/sections/agent_policy/create_package_policy_page/services/validate_package_policy.ts

+    validationResults.vars = packageVars.reduce((results, [name, varEntry]) => {
+      results[name] = validatePackagePolicyConfig(varEntry, packageVarsByName[name]);
+      return results;
+    }, {} as ValidationEntry);


Minor thing, but it seems like elsewhere in the fleet codebase we use the reduce's generic - e.g. packageVars.reduce<ValidationEntry>(...)- instead of a type assertion like this. Although looking more it seems inconsistent file-to-file. Is this something we should or could make consistent for all our uses of reduce?

yeah our usage has been inconsistent, I will address this in the next PR for granularity support in order to get this one in now. thanks!

kpollich · 2021-05-25T13:46:47Z

x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/home/index.tsx

-function InstalledPackages() {
+});
+
+// Packages can export multiple integrations, aka `policy_templates`


Super helpful comment - thank you!

jen-huang · 2021-05-26T14:44:34Z

@elasticmachine merge upstream

kibanamachine · 2021-05-26T16:29:20Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`fleet`	455	456	+1

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`	972	980	+8

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`fleet`	731.8KB	736.9KB	+5.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`fleet`	346.3KB	347.5KB	+1.2KB

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id	before	after	diff
`ingest-package-policies`	31	32	+1

Unknown metric groups

API count

id	before	after	diff
`fleet`	1062	1070	+8

References to deprecated APIs

id	before	after	diff
`canvas`	29	25	-4
`crossClusterReplication`	8	6	-2
`fleet`	22	20	-2
`globalSearch`	4	2	-2
`indexManagement`	12	7	-5
`infra`	261	149	-112
`lens`	67	45	-22
`licensing`	18	15	-3
`lists`	239	236	-3
`maps`	286	208	-78
`ml`	121	115	-6
`monitoring`	109	56	-53
`securitySolution`	386	342	-44
`stackAlerts`	101	95	-6
total			-342

History

💚 Build #126955 succeeded 716068a
💚 Build #126733 succeeded 67867be
💔 Build #126654 failed e569fe1
💔 Build #126653 failed 8dd44fa
💔 Build #125451 failed e15d6f0

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

cc @jen-huang

* Manual cherry pick of work to support integration tiles and package-level vars * Fix types * Remove registry input group typings * Show integration-specific readme, title, and icon in package details page * Revert unnecessary changes * Add package-level `vars` field to package policy SO mappings * Fix types * Fix test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-05-26T17:02:07Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

* Manual cherry pick of work to support integration tiles and package-level vars * Fix types * Remove registry input group typings * Show integration-specific readme, title, and icon in package details page * Revert unnecessary changes * Add package-level `vars` field to package policy SO mappings * Fix types * Fix test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jen Huang <its.jenetic@gmail.com>

mdelapenya · 2021-05-28T11:30:49Z

Hi, we have found a regression in the e2e tests, probably caused by this PR.

We are observing changes in how an integration is added to the policy. Before these changes, we were able to hit the /api/fleet/package_policies endpoint passing a payload for a PackageDataStream (https://github.com/elastic/e2e-testing/blob/master/internal/kibana/policies.go#L132-L142), but now we receive a HTTP 400 error code:

Error: could not add package to policy; API status code = 400; response body = {"statusCode":400,"error":"Bad Request","message":"[request body.package.id]: definition for this key is missing"}

And this code hasn’t changed in the test framework.

I've checked the git history of Kibana for the x-pack/plugins/fleet path, and the below list contains the possible culprit commits:

I've diff'ed #99866, and the code in there seems a feasible candidate to cause the regression. I'm currently running the tests for the previous PR (#100476) using the dedicated pipeline we created to run e2e tests for Kibana: https://beats-ci.elastic.co/blue/organizations/jenkins/e2e-tests%2Fe2e-testing-mbp/detail/master/922/pipeline.

At the same time, I've run the e2e tests for #99866: https://beats-ci.elastic.co/blue/organizations/jenkins/e2e-tests%2Fe2e-testing-mbp/detail/master/924/pipeline

In both cases we:

have built the docker image for each PR first, using "Deploy my Kibana" tool, so they are available in the Docker registry: docker.elastic.co/observability-ci/kibana:pr100476
Once built (~50mins) run the e2e tests using pr$ID as KIBANA_VERSION.(pr99866 and pr 100476)

It's possible to reproduce this locally:

Before this changes, the tests adding integration to policies pass:

 TAGS="linux_integration" TIMEOUT_FACTOR=3 LOG_LEVEL=TRACE DEVELOPER_MODE=true KIBANA_VERSION=pr100476 make -C e2e/_suites/fleet functional-test

Passing:

2 scenarios (2 passed)
10 steps (10 passed)
3m0.812610611s

With this changes, the tests adding integration to policies fail:

 TAGS="linux_integration" TIMEOUT_FACTOR=3 LOG_LEVEL=TRACE DEVELOPER_MODE=true KIBANA_VERSION=pr99866 make -C e2e/_suites/fleet functional-test

Failing with:

--- Failed steps:

  Scenario Outline: Adding the Linux Integration to an Agent ... # features/linux_integration.feature:6
    When the "Linux" integration is "added" in the policy # features/linux_integration.feature:9
      Error: could not add package to policy; API status code = 400; response body = {"statusCode":400,"error":"Bad Request","message":"[request body.package.id]: definition for this key is missing"}

  Scenario Outline: Adding the Linux Integration to an Agent ... # features/linux_integration.feature:6
    When the "Linux" integration is "added" in the policy # features/linux_integration.feature:9
      Error: could not add package to policy; API status code = 400; response body = {"statusCode":400,"error":"Bad Request","message":"[request body.package.id]: definition for this key is missing"}


2 scenarios (2 failed)
10 steps (4 passed, 2 failed, 4 skipped)
2m44.62434826s

ruflin · 2021-05-28T11:38:30Z

Thanks for the all the investigation @mdelapenya . @nchaulet @kpollich I assume we introduce a breaking change during the introduction of the granular integration implementation. Can you please have a look?

kpollich · 2021-05-28T12:40:46Z

@mdelapenya it looks like the issue here is that the package object in the request body includes an id field, which the fleet codebase doesn't expect.

Pasting the error message from above just for convenience:

{"statusCode":400,"error":"Bad Request","message":"[request body.package.id]: definition for this key is missing"}

The PackageDataStream struct in https://github.com/elastic/e2e-testing/blob/master/internal/kibana/policies.go#L132-L142 defines the package field as another struct IntegrationPackage here: https://github.com/elastic/e2e-testing/blob/888ecfec924124d6359e2dadf999acc347c12477/internal/kibana/integrations.go#L14-L19

This struct defines the package object as such:

{
  package: { 
    id: string,
    name: string,
    title: string,
    version: string
  }
}

Fleet's expected schema for this object is defined here:

kibana/x-pack/plugins/fleet/server/types/models/package_policy.ts

Lines 37 to 43 in a4f6d43

    
           package: schema.maybe( 
        
             schema.object({ 
        
               name: schema.string(), 
        
               title: schema.string(), 
        
               version: schema.string(), 
        
             }) 
        
           ),

I was able to reproduce this exact error locally by forcing an id field into this package object when we create a package policy in the Fleet UI:

Screenshots

This PR didn't alter anything around this schema that I can see, the only changes made to this particular request structure were to allow for the vars field at the package level to support granular integrations here:

kibana/x-pack/plugins/fleet/server/types/models/package_policy.ts

Line 80 in a4f6d43

vars: schema.maybe(ConfigRecordSchema),

I'm new to the project (and Elastic in general 🙂), so I'm not 100% certain on how to proceed towards a resolution, but maybe someone else could chime in on a few questions:

Should Fleet support this id field when creating package policies?
Is the Linux integration exporting this ID and causing this specific failure, or this an issue with the E2E tests?

mdelapenya · 2021-05-28T13:53:36Z

As far as I know, the package.id field was already there and the tests did pass. What I understand from this analysis is that now the schema contract is more strict. Maybe in the past that new field was simple skipped and now a strict contract is enforced.

kpollich · 2021-05-28T14:03:34Z

As far as I know, the package.id field was already there and the tests did pass. What I understand from this analysis is that now the schema contract is more strict. Maybe in the past that new field was simple skipped and now a strict contract is enforced.

That could definitely be the case. I'll surface this with the rest of the Fleet team and take a look at how we enforce this request schema.

mdelapenya · 2021-05-28T14:12:41Z

We simple retrieve the integration hitting /epm/packages?experimental=true, and then filter them by packageName (i.e. "linux"), and with that JSON response we build the struct discussed above, so it very likely includes package ID. We could discard that field on our side, but IMHO it would be like a workaround

kpollich · 2021-05-28T14:23:44Z

We simple retrieve the integration hitting /epm/packages?experimental=true, and then filter them by packageName (i.e. "linux"), and with that JSON response we build the struct discussed above, so it very likely includes package ID. We could discard that field on our side, but IMHO it would be like a workaround

Yes I agree. I just filed an issue (#100897) to permit the id field in create requests to unblock the E2E tests. I'll take this on and ping you once a PR is up.

mdelapenya · 2021-05-28T14:27:02Z

Cool thank you for the fast response! If we decide to not accept the id field in the payload, I've just removed the ID in a local branch of the e2e tests, and it works, just in case.

jen-huang added 6 commits May 11, 2021 14:50

Manual cherry pick of work to support integration tiles and package-l…

36550ab

…evel vars

Fix types

f3aa43b

Remove registry input group typings

a59c76e

Show integration-specific readme, title, and icon in package details …

7fb49f5

…page

Revert unnecessary changes

49cd8f2

Add package-level vars field to package policy SO mappings

e15d6f0

jen-huang added release_note:enhancement v8.0.0 Team:Fleet Team label for Observability Data Collection Fleet team v7.14.0 labels May 11, 2021

jen-huang self-assigned this May 11, 2021

jen-huang requested a review from a team as a code owner May 11, 2021 23:14

jen-huang added 2 commits May 19, 2021 16:23

Merge remote-tracking branch 'upstream/master' into granularity-integ…

e8f26a1

…ration-list

Fix types

8dd44fa

jen-huang requested a review from a team as a code owner May 19, 2021 23:46

jen-huang changed the title ~~[WIP][Fleet] Support browsing granular integrations~~ [Fleet] Support browsing granular integrations May 19, 2021

kibanamachine and others added 2 commits May 19, 2021 20:02

Merge branch 'master' into granularity-integration-list

e569fe1

Fix test

67867be

Merge branch 'master' into granularity-integration-list

716068a

kaiyan-sheng mentioned this pull request May 24, 2021

Adjust aws package to use input groups elastic/integrations#767

Merged

3 tasks

ChrsMark mentioned this pull request May 25, 2021

Introduce kubernetes package granularity elastic/integrations#1018

Merged

4 tasks

nchaulet self-requested a review May 25, 2021 12:58

kpollich approved these changes May 25, 2021

View reviewed changes

Merge branch 'master' into granularity-integration-list

edfd5cd

nchaulet approved these changes May 26, 2021

View reviewed changes

jen-huang enabled auto-merge (squash) May 26, 2021 16:58

jen-huang disabled auto-merge May 26, 2021 16:59

jen-huang merged commit ba7c027 into elastic:master May 26, 2021

jen-huang deleted the granularity-integration-list branch May 26, 2021 16:59

jen-huang added the auto-backport Deprecated - use backport:version if exact versions are needed label May 26, 2021

kibanamachine mentioned this pull request May 26, 2021

[7.x] [Fleet] Support browsing granular integrations (#99866) #100707

Merged

kpollich mentioned this pull request May 28, 2021

[Fleet] Allow Package ID in Create Package Policy requests #100897

Closed

kpollich mentioned this pull request May 28, 2021

[Fleet] Include ID field in packages on get/create #100908

Merged

This was referenced Jun 7, 2021

[Fleet] Support granular integrations in policy editor #101531

Merged

Meta: enable input groups elastic/package-spec#144

Closed

Conversation

jen-huang commented May 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Allow users to browse "granular integrations"

Support package-level vars (user configurable variables)

How to test

What's next

Uh oh!

elasticmachine commented May 11, 2021

Uh oh!

jen-huang commented May 11, 2021

Uh oh!

kpollich commented May 13, 2021

Uh oh!

jen-huang commented May 20, 2021

Uh oh!

jen-huang commented May 24, 2021

Uh oh!

kpollich left a comment

Choose a reason for hiding this comment

Uh oh!

kpollich May 25, 2021

Choose a reason for hiding this comment

Uh oh!

jen-huang May 26, 2021

Choose a reason for hiding this comment

Uh oh!

kpollich May 25, 2021

Choose a reason for hiding this comment

Uh oh!

jen-huang commented May 26, 2021

Uh oh!

kibanamachine commented May 26, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

Saved Objects .kibana field count

API count

References to deprecated APIs

History

Uh oh!

kibanamachine commented May 26, 2021

💚 Backport successful

Uh oh!

mdelapenya commented May 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruflin commented May 28, 2021

Uh oh!

kpollich commented May 28, 2021

Uh oh!

mdelapenya commented May 28, 2021

Uh oh!

kpollich commented May 28, 2021

Uh oh!

mdelapenya commented May 28, 2021

Uh oh!

kpollich commented May 28, 2021

Uh oh!

mdelapenya commented May 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jen-huang commented May 11, 2021 •

edited

Loading

mdelapenya commented May 28, 2021 •

edited

Loading