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

[Ingest Manager] Send kibana version on requests to package registry #71443

Merged
merged 6 commits into from
Jul 14, 2020
Merged

[Ingest Manager] Send kibana version on requests to package registry #71443

merged 6 commits into from
Jul 14, 2020

Conversation

skh
Copy link
Contributor

@skh skh commented Jul 13, 2020 •
edited
Loading

Summary

Implements #64868

This change adds the parameter kibana.version to all requests to the package registry's /search endpoint. See also elastic/package-registry#518 for the query parameter.

How to test this

Unfortunately, there is no easy way to test this, other than verifying that the integrations in Ingest Manager still appear and are installable as expected.

To see how the new parameter affects the registry's response, visit the following links and play around with the package name and the version number in the query string.

For example, a package with this condition in its manifest.yml will not appear when the version sent is earlier than 7.9.0:

conditions:
  kibana:
    version: ">=7.9.0"

@skh skh added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.9.0 Team:Fleet Team label for Observability Data Collection Fleet team labels Jul 13, 2020
@skh skh requested a review from a team July 13, 2020 13:43
@skh skh self-assigned this Jul 13, 2020
@elasticmachine
Copy link
Contributor

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

@skh
Copy link
Contributor Author

skh commented Jul 13, 2020

@elasticmachine merge upstream

@skh
Copy link
Contributor Author

skh commented Jul 13, 2020

@elasticmachine merge upstream

@skh skh mentioned this pull request Jul 13, 2020
Closed
@skh
Copy link
Contributor Author

skh commented Jul 14, 2020

@ph It seems that a lot of the packages we have in https://epr-snapshot.ea-web.elastic.dev/ are excluded when kibana.version is 8.0.0, you see this if you compare https://epr-snapshot.ea-web.elastic.dev/search?kibana.version=8.0.0 and https://epr-snapshot.ea-web.elastic.dev/search?kibana.version=7.9.0

This would mean that if we merge this now, master would only have the packages system, endpoint, snapshot and log available. This is enough to setup Ingest Manager, but not enough for a lot of testing.

Please advise how to proceed. Thanks!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
enterpriseSearch 41 -21 62

History

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

@ph
Copy link
Contributor

ph commented Jul 14, 2020

After looking some packages, the issue is because version check are really specifics, see https://github.com/elastic/integrations/blob/master/packages/kafka/manifest.yml#L12

So I think we can do the following:

  • Let's merge this PR
  • Create an issue in the integrations repository.
  • Send an email to the google groups for the packages.

This was referenced Jul 14, 2020
Closed
Merged
@skh skh merged commit ef2a583 into elastic:master Jul 14, 2020
@skh skh mentioned this pull request Jul 14, 2020
Merged
skh added a commit to skh/kibana that referenced this pull request Jul 14, 2020
@skh @elasticmachine
[Ingest Manager] Send kibana version on requests to package registry (e…
6cd6f84 
…lastic#71443)

* Send kibana version on requests to registry search.

* Only use semver part of kibanaVersion.

* Adjust test.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
skh added a commit that referenced this pull request Jul 14, 2020
@skh @elasticmachine
[7.x] [Ingest Manager] Send kibana version on requests to package reg…
28f9a41 
…istry (#71443) (#71702)

* [Ingest Manager] Send kibana version on requests to package registry (#71443)

* Send kibana version on requests to registry search.

* Only use semver part of kibanaVersion.

* Adjust test.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* Adjust test to 7.x.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@skh skh deleted the 64868-send-kibana-version-to-registry branch July 14, 2020 21:31
@skh skh mentioned this pull request Jul 14, 2020
Closed
ruflin
ruflin reviewed Jul 23, 2020
View reviewed changes
x-pack/plugins/ingest_manager/server/services/epm/registry/index.ts
@@ -47,6 +48,10 @@ export async function fetchList(params?: SearchParams): Promise<RegistrySearchRe
url.searchParams.set('experimental', params.experimental.toString());
}
}
const kibanaVersion = appContextService.getKibanaVersion().split('-')[0]; // may be 8.0.0-SNAPSHOT
Copy link
Contributor

Choose a reason for hiding this comment

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

@skh Did you try if sending 8.0.0-SNAPSHOT works? I wonder if the registry should support this somehow?

@skh
Copy link
Contributor Author

skh commented Jul 23, 2020

Did you try if sending 8.0.0-SNAPSHOT works? I wonder if the registry should support this somehow?

The fact that it didn't work and broke CI (because CI sends -SNAPSHOT) is the reason for the change you're commenting on.

@ruflin
Copy link
Contributor

ruflin commented Jul 23, 2020

@skh Good to know. Will take a look at the registry to see if we should support it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin left review comments

@jfsiii jfsiii jfsiii approved these changes

Assignees

@skh skh

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@skh @elasticmachine @kibanamachine @ph @ruflin @jfsiii