search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.72k stars 8.14k forks source link

[Fleet] Add option to toggle search of prerelease packages #122973

Closed jsoriano closed 1 year ago

jsoriano commented 2 years ago

In order to simplify the ways package developers can specify the stability of a package, we are going to rely completely on SemVer. This means that any package version over 1.0.0 and without a prerelease label (like 1.0.0-rc1) will be considered stable, 0.x.x versions or versions with prerelease labels, will be considered prereleases.

This means that we are going to deprecate the release labels in package manifests, and eventually, the different package registry stages (production, staging, snapshot...). More about this in https://github.com/elastic/package-spec/issues/225.

As part of this we are proposing the following changes for Fleet:

prerelease parameter is being added to the registry in https://github.com/elastic/package-registry/pull/785. To keep compatibility with current Kibana versions, experimental=true keeps current behaviour, including prerelease packages in search requests.

Demo

elasticmachine commented 2 years ago

Pinging @elastic/fleet (Team:Fleet)

jsoriano commented 2 years ago

Updated description after internal discussion about how to map prerelease tags to UI labels as replacement of current "Beta"/"Experimental" ones.

The recommendation would be to use the following convention to replace current labels with the following mappings:

cc @akshay-saraswat @mostlyjason.

mostlyjason commented 2 years ago

Add a setting to enable or disable the search of pre-release packages (if possible available both in UI and in configuration).

@jsoriano I don't see a UI feature today that offers this ability. Only a small subset of integrations are experimental so have you see it be a significant issue for users?

Replace current beta/experimental labels with the following mappings

This part sounds good. @jen-huang I think we'd need to coordinate Fleet UI with the EPR changes?

jsoriano commented 2 years ago

Add a setting to enable or disable the search of pre-release packages (if possible available both in UI and in configuration).

@jsoriano I don't see a UI feature today that offers this ability. Only a small subset of integrations are experimental so have you see it be a significant issue for users?

Correct, there is no feature today to disable the search of experimental packages, and they are always searched. We could continue like this by now, only moving from experimental to prerelease. If we add a configuration flag, we can keep it enabled by default by now if we want to keep this behaviour.

But I think that this will be more needed in the future.

Currently one reason to have few in-development packages in the public registry is that we have multiple registries (snapshot/staging), so not everything is promoted to the production one. We have plans to get rid of these promotion approach as part of the migration to the v2 storage, in that scenario the only way for developers to publish in-development packages will be using prerelease versions, and without a toggle, users won't be able to opt-out from these packages.

Another reason is that we want to expand the packages ecosystem to more developers and teams, possibly also out of Elastic, and we want to offer them a simple way to publish in-development versions of packages, instead of the ~three ways we have now.

Summarizing, I would suggest to implement the toggle, so this is ready when needed or requested by users. If we don't want it to be so used by now, we can keep it enabled by default, and only available through configuration files. We could postpone the decision of making it visible in the UI and/or disabling it by default.

jsoriano commented 2 years ago

All public instances of the registry support the new prerelease parameter now (experimental is supported too for backwards compatibility).

mostlyjason commented 2 years ago

Sounds good! I'd recommend keeping the same behavior in the UI by default for now, which means showing experimental and higher. I agree it'd be nice to offer users a UI feature to filter on version, but we should define/implement that when it becomes a priority for UX. We have several other UI features we are focusing on currently that are higher up. I'll leave it up the engineering team what is the MVP way to enable prerelease packages for development users.

we want to offer them a simple way to publish in-development versions of packages

@jsoriano Do you plan to distinguish in-development versions of packages intended only for internal team's use from "Technical Preview" packages that are intended for end-users?

jsoriano commented 2 years ago

@jsoriano Do you plan to distinguish in-development versions of packages intended only for internal team's use from "Technical Preview" packages that are intended for end-users?

We want to fully rely on semantic versioning for publicly available packages. If a development team wants to publish a package as technical preview, available to users, they must use a prerelease suffix (for example 1.2.0-preview1). Also, all 0.x versions are considered technical previews. Regarding the toggle, if we implement it, such packages would be available only to users with it enabled.

I agree with you that we would need to think this well from the UX perspective. Maybe another option, instead of a toggle, would be to always show all packages, but require an additional confirmation to install or upgrade to a prerelease version.

mostlyjason commented 2 years ago

In that case it sounds like we need to show all packages by default. I think that is already the case today. We need to continue showing experimental packages because some users may have already installed those, and it'd be unexpected if they disappeared. We have a label on each card on the browse page, and a label right next to the add button so hopefully users are aware that its a technical preview. Before adding confirmation steps, perhaps we wait to see if we get any feedback that users are confused by this?

image

jsoriano commented 2 years ago

@mostlyjason yes, SGTM to show all packages by default in searches, this will help discovering non-GA features.

I still think that we will need at least something to prevent users of GA packages to upgrade to unintentionally upgrade to a non-GA version. If we consider the labels are enough, then no need to add any setting or confirmation dialog, we can wait to see if users find this necessary.

Then as part of this task we would only need to add the mappings for the new labels, and the use of the prerelease parameter instead of the deprecated experimental.

mostlyjason commented 2 years ago

I still think that we will need at least something to prevent users of GA packages to upgrade to unintentionally upgrade to a non-GA version

Are you saying a package could upgrade from GA to non-GA? I thought the version for a given package could only increase, not decrease? Wouldn't that be a breaking change to remove a GA feature? Or are you saying that a user who uses GA releases normally, would unintentionally add a non-GA release of a different package?

jsoriano commented 2 years ago

I still think that we will need at least something to prevent users of GA packages to upgrade to unintentionally upgrade to a non-GA version

Are you saying a package could upgrade from GA to non-GA?

It is the same as with any other Elastic product, for example Elasticsearch is GA, but before releasing 8.0, there were prerelease versions like 8.0.0-beta1 or 8.0.0-rc2. Maintainers of the Apache package could decide to release a 1.4.0-rc1 prerelease version, this version would be offered to users of the GA Apache 1.3.2 if we don't implement any safeguard.

I thought the version for a given package could only increase, not decrease?

This is true, but there can be prereleases of a greater version (1.4.0-rc1 > 1.3.2).

And this doesn't change with the removal of the release tag, with the release tag it was possible to release 1.4.0 with release: beta after having released 1.3.2 with release: ga.

Wouldn't that be a breaking change to remove a GA feature?

We wouldn't be removing the GA feature, the same way as when we released Elasticsearch 8.0.0-beta1 we were not removing previous Elasticsearch. GA versions continue being available.

Or are you saying that a user who uses GA releases normally, would unintentionally add a non-GA release of a different package?

This is a different case, this would be the case of someone using only GA features, that decides to install a non-GA package, maybe to try something that hasn't been released yet as GA, as could be the case of the synthetics package.

juliaElastic commented 2 years ago

@jsoriano @mostlyjason @joshdover As part of the release tag change, Integrations UI has to be changed to show package badges based on semver instead of release tag, right? I'm not seeing this feature mentioned in this issue. Currently the UI shows no badge/beta/experimental coming from the package's release tag field. This has to change to derive from semver I think and to use the new labeling (ga/technical preview/beta).

jsoriano commented 2 years ago

@juliaElastic yes, this would be the part mentioned in the description as "Replace current beta/experimental labels with the following mappings: ...". Or are you referring to a different thing?

juliaElastic commented 2 years ago

yes, this would be the part mentioned in the description as "Replace current beta/experimental labels with the following mappings: ...". Or are you referring to a different thing?

I see, I guess I wasn't sure if that refers the Integration UI labels, thanks for confirming. Is the expectation for Fleet to do the mapping from semver to GA/Technical Preview/Beta for the purpose of using as UI badges?

jsoriano commented 2 years ago

Is the expectation for Fleet to do the mapping from semver to GA/Technical Preview/Beta for the purpose of using as UI badges?

Yes, I think this should replace current mappings based on the release tag.

The only other place where we have identified the needing of this mapping has been in elastic-package status, implementation can be found here.

It'd be nice if we could reuse this logic between components, but I don't expect this to change frequently.

juliaElastic commented 2 years ago

Note to Fleet UI team: the previous release tag values are duplicated in a few places, it would be good to move to an enum and rename: https://github.com/elastic/kibana/search?q=%22%27beta%27+%7C+%27experimental%27+%7C+%27ga%27%22

juliaElastic commented 2 years ago

@joshdover I noticed that on Integrations UI, some packages have empty release and version properties. These are the ones that are split from a package e.g. ActiveMQ Logs and Metrics. Is this a bug? E.g. if base package is beta, the integration card should have beta badge too, right?

image
joshdover commented 2 years ago

These are the ones that are split from a package e.g. ActiveMQ Logs and Metrics. Is this a bug? E.g. if base package is beta, the integration card should have beta badge too, right?

These are not packages, they're actually Beats tutorials that link off to a different app in Kibana. They're only visible when the corresponding package is not yet GA. You can grep around in the codebase for getReplacementCustomIntegrations to find how this replacement happens on the client side.

juliaElastic commented 2 years ago

Yes, I think this should replace current mappings based on the release tag.

The only other place where we have identified the needing of this mapping has been in elastic-package status, implementation can be found here.

It'd be nice if we could reuse this logic between components, but I don't expect this to change frequently.

@jsoriano we found another place where the release badges (beta/experimental) appear: https://docs.elastic.co/en/integrations

would it make sense to store the mapping logic centrally in registry? I'm not sure if kibana/fleet is the right place to store it, especially if it might be duplicated in other places.

jsoriano commented 2 years ago

@juliaElastic yes, if we start finding more places where this is needed we may need a central place to maintain this convention, I have created https://github.com/elastic/package-spec/issues/286 to discuss this.

I am not sure though if the registry is the best place, because there are situations where the registry won't be available, for example with bundled packages, or with packages installed without the registry (https://github.com/elastic/kibana/issues/70582). Also with elastic-package, when showing information about local, not published packages.

Other thing that makes me wonder if this logic should be centralized, is that in principle this is needed purely for UI reasons. Functionally (and regarding support) we should do the same with any non-GA package. With these mappings we are only providing additional information about what to expect, it is like a translation from a technical code to a human-friendly text. And this information can be different in different tools. For example in elastic-package we could decide to show the prerelease tag directly, without any mapping and this would be mostly ok for the target audience. While in Kibana we are also providing additional descriptions, and translations for them. If we bring this logic to the registry, we assume that this can be centrally changed there, but then, would the registry also serve the descriptions? and how would the translations be implemented?

I think we can go by now with specific implementations, and reevaluate later if we find that this is giving problems. We can in any case somehow fix the conventions in the spec as a result of https://github.com/elastic/package-spec/issues/286, so there is a central point where this convention can be referenced as base for the different implementations.

mostlyjason commented 2 years ago

Maintainers of the Apache package could decide to release a 1.4.0-rc1 prerelease version, this version would be offered to users of the GA Apache 1.3.2 if we don't implement any safeguard.

This is an interesting problem because we need to define a ranking algorithm for what we show in Integrations UI. It sounds like the existing algorithm is to show the highest package version? However, it should be to filter on the highest release type first (GA > Beta > Technical Preview), then filter on the highest version. So GA Apache 1.3.2 will outrank 1.4.0-rc1. Does that sound right?

There is a separate problem of how users can install prerelease versions after a GA release has been made. I'm not sure this needs to be part of the MVP release, but is a feature we could add later. I imagine in the API is a workaround that allows developers to install any version they want?

juliaElastic commented 2 years ago

Maintainers of the Apache package could decide to release a 1.4.0-rc1 prerelease version, this version would be offered to users of the GA Apache 1.3.2 if we don't implement any safeguard.

This is an interesting problem because we need to define a ranking algorithm for what we show in Integrations UI. It sounds like the existing algorithm is to show the highest package version? However, it should be to filter on the highest release type first (GA > Beta > Technical Preview), then filter on the highest version. So GA Apache 1.3.2 will outrank 1.4.0-rc1. Does that sound right?

There is a separate problem of how users can install prerelease versions after a GA release has been made. I'm not sure this needs to be part of the MVP release, but is a feature we could add later. I imagine in the API is a workaround that allows developers to install any version they want?

This could be done with a UI toggle or config flag to show only GA packages / show all. The toggle value could be used when checking for latest package versions as well, e.g. not showing Upgrade button when there are no new GA versions available, only prerelease.

mtojek commented 2 years ago

Hi @nimarezainia, according to what we discussed during the latest bi-weekly meeting... did you find some time to take a look at this issue?

dborodyansky commented 1 year ago

Please find design proposal based on discussion and share any questions or concerns. Thank you. (@kpollich @nimarezainia @jen-huang @mukeshelastic)

juliaElastic commented 1 year ago

Please find design proposal based on discussion and share any questions or concerns. Thank you. (@kpollich @nimarezainia @jen-huang @mukeshelastic)

@dborodyansky A few questions:

image
jsoriano commented 1 year ago
  • The design says "upgrade workflows remain as is" - Does this mean no change required on integration policies view, where we show an Upgrade button? Meaning that the Upgrade button will be visible even if only a prerelease version is available. Additionally, the auto upgrade integration policies functionality would upgrade to available prerelease versions too.

I think that users of GA versions with the prerelease toggle disabled shouldn't be offered prerelease upgrades, this would defeat the purpose of this feature.

The exception may be if a user is already using a prerelease version, then it could be ok to upgrade to another prerelease.

  • Should the prerelease toggle state persist as a UI setting? So once the user sets it, the value is persisted between page navigation and reloads.

Yes, I would say so. There should be also a way to control this toggle using configuration and/or the API, so it can be controlled in development environments.

  • Do we want to show all GA versions in a dropdown, not just the latest?

It may be nice if we support downgrading, otherwise not sure.

I am not sure if we have an API that lists all available versions of a package.

Adding all=true to any search request lists all versions that match the rest of conditions.

juliaElastic commented 1 year ago

I think that users of GA versions with the prerelease toggle disabled shouldn't be offered prerelease upgrades, this would defeat the purpose of this feature.

I agree with you, my only concern is on the scope of this issue. Can we do the upgrade changes later in a follow up issue, or best effort include in this one? I don't think the original estimate included this.

Yes, I would say so. There should be also a way to control this toggle using configuration and/or the API, so it can be controlled in development environments.

An API would be nice, though the same comment here about the scope. Can we start with a UI setting and move it to a configuration stored in elasticsearch with an API in a follow up issue? Though to prevent auto upgrades to prerelease versions, we definitely need a setting on the backend.

Do we want to show all GA versions in a dropdown, not just the latest?

It may be nice if we support downgrading, otherwise not sure.

I don't know if we want to encourage users to downgrade, they can do that now the the API using a force flag. In my opinion it is more important to focus on the upgrade use case first rather than spend time on displaying older versions in a dropdown.

cc @kpollich

jsoriano commented 1 year ago

Sure, this can be done as follow ups if it is out of the scope of the original estimation. It'd be nice though to have it ready for 8.6.

+1 to focus on upgrade-to-latest use cases, and fallback to the API for special cases.

kpollich commented 1 year ago

I agree with you, my only concern is on the scope of this issue. Can we do the upgrade changes later in a follow up issue, or best effort include in this one? I don't think the original estimate included this.

By "upgrade changes" here we mean preventing prerelease versions from being included in the "upgrade available" calculation when this setting is disabled, correct? I think this is important and should be included in the scope of this issue.

We should be able to update our _fetchFindLatestPackage method to honor the persisted value of this toggle:

https://github.com/elastic/kibana/blob/main/x-pack/plugins/fleet/server/services/epm/registry/index.ts#L87-L129

An API would be nice, though the same comment here about the scope. Can we start with a UI setting and move it to a configuration stored in elasticsearch with an API in a follow up issue? Though to prevent auto upgrades to prerelease versions, we definitely need a setting on the backend.

You are correct about needing a setting on the backend to honor this setting during the "latest available version" check for package updates. We'll need an API of some sort to persist this setting either way. Could we do this through the existing settings API perhaps?

I don't know if we want to encourage users to downgrade, they can do that now the the API using a force flag. In my opinion it is more important to focus on the upgrade use case first rather than spend time on displaying older versions in a dropdown.

+1 to this. Downgrades are "supported" but not encouraged. Let's not add any work to make downgrading part of the UI consideration here.

juliaElastic commented 1 year ago

By "upgrade changes" here we mean preventing prerelease versions from being included in the "upgrade available" calculation when this setting is disabled, correct? I think this is important and should be included in the scope of this issue.

Yes. Thanks or the suggestion, sounds like we have a straightforward way to solve for the upgrades.

Could we do this through the existing settings API perhaps?

As discussed we will go ahead with adding the prerelease setting to Fleet settings to minimize boilerplate. We can move it to separate integration settings later if needed. Adding a kibana config option for this flag is also out of scope for now, we can decide later to add it as a preconfig.

juliaElastic commented 1 year ago

@dborodyansky @kpollich One more scenario to confirm:

e.g. /app/integrations/detail/apache_spark-0.2.1/overview

juliaElastic commented 1 year ago

@kpollich I ran into a new issue as I started making the changes in _fetchFindLatestPackage function to respect the prerelease flag, setting to false by default. The synthetics package is installed at kibana startup time, and it has only beta versions (latest 0.11.0), so I am getting an error with default settings, because the registry doesn't find the package with prerelease=false param.

I am wondering what would be the best approach here. I think we want to avoid depending on synthetics GA version for this change. We also want to keep the prerelease flag off by default to avoid auto upgrades to beta versions. Maybe we could do a workaround to return the beta version just for synthetics package, though it would be a hack.

kpollich commented 1 year ago

@elastic/uptime - Could you advise on the above? We are making changes to how prerelease packages interact with Kibana/Fleet and some input would be helpful.

@juliaElastic - It's probably not going to be pretty, but we may have to hardcode a list of exceptions for this behavior since we're shipping a prerelease version of synthetics to all clusters by default right now.

andrewvc commented 1 year ago

@kpollich our package is currently in beta. It seems like we would need to make it v 1.0.0 to appear by default. However, that's kinda at odds with the beta status. Is that the correct understanding?

As long as it's still clearly labeled beta I guess I'm fine with 1.0.0, but I'd like to check with @drewpost and @paulb-elastic specifically on that.

CC @dominiqueclarke as well

kpollich commented 1 year ago

our package is currently in beta. It seems like we would need to make it v 1.0.0 to appear by default. However, that's kinda at odds with the beta status. Is that the correct understanding?

Yes I'd say this is correct. cc @jsoriano for advice as well, but I think pursuing 1.0.0 makes the most sense. If we're shipping synthetics by default in production environments, it should not on a prerelease version in my opinion.

juliaElastic commented 1 year ago

For now I added an exception for synthetics to be visible even when prerelease packages are hidden (toggle is false).

I think the registry should already work by semver and derive the release label from it (meaning 1.0.0 is going to be GA). We are moving on the UI as well to this calculation, to ignore the release label.

andrewvc commented 1 year ago

@kpollich our timeline for 1.0 is circa 8.7. We can neither yank support for it now nor artificially move it forward.

I'm fine calling the number 1.0.0 as long as it's still clearly labeled beta.

juliaElastic commented 1 year ago

prerelease parameter is being added to the registry in https://github.com/elastic/package-registry/pull/785. To keep compatibility with current Kibana versions, experimental=true keeps current behaviour, including prerelease packages in search requests.

@jsoriano I discovered a difference in experimental and prerelease flags in the v2 dev epr repo (used by kibana CI). Is this expected?

https://epr-v2.ea-web.elastic.dev/search?package=endpoint&experimental=true
// -> returns endpoint.8.5.0
https://epr-v2.ea-web.elastic.dev/search?package=endpoint&prerelease=true
// -> returns endpoint.8.6.0-dev.0
andrewvc commented 1 year ago

Apologies, I'd somehow missed @juliaElastic 's comment. The exception is perfect for us.

mrodm commented 1 year ago

@jsoriano I discovered a difference in experimental and prerelease flags in the v2 dev epr repo (used by kibana CI). Is this expected?

https://epr-v2.ea-web.elastic.dev/search?package=endpoint&experimental=true
// -> returns endpoint.8.5.0
https://epr-v2.ea-web.elastic.dev/search?package=endpoint&prerelease=true
// -> returns endpoint.8.6.0-dev.0

@juliaElastic I would say this is the expected behavior. This was the implementation requested and it was implemented here. When the experimental flag is true, as there is a GA version of the package, other non-GA version are discarded.

In this case, for that search, there exists at least one GA version for endpoint, 8.5.0, so other prerelease versions are discarded, like 8.6.0-dev. With the prerelease flag, that behavior is also the expected, prerelease=true returns the prerelease versions.

juliaElastic commented 1 year ago

@mrodm thanks for the context. I am wondering when is the experimental flag expected to be removed from EPR API? We have a similar flag in Fleet packages API, which we are making deprecated. Should we keep the behavior in Fleet API that experimental flag is passed as-is to EPR API?

mrodm commented 1 year ago

I am wondering when is the experimental flag expected to be removed from EPR API? We have a similar flag in Fleet packages API, which we are making deprecated. Should we keep the behavior in Fleet API that experimental flag is passed as-is to EPR API?

I guess we would need to keep that experimental parameter in EPR for a long time, at least while there is any old Kibana version using it.

But, new Kibana versions (IIRC starting in 8.6) should not be using that parameter in requests against EPR.

juliaElastic commented 1 year ago

But, new Kibana versions (IIRC starting in 8.6) should not be using that parameter in requests against EPR.

Originally I wanted to replace usages of experimental flag to prerelease in Fleet, but now as it turns out they are not behaving the same way. Should we instead just remove experimental=true from the queries to EPR in Fleet?

The prerelease flag will be added to the queries when the setting (UI switch) is turned on.

mrodm commented 1 year ago

Should we instead just remove experimental=true from the queries to EPR in Fleet?

Yes, I think this is the expected change. experimental=true should be just used by older kibana versions (< 8.6.0)

The prerelease flag will be added to the queries when the setting (UI switch) is turned on.

:+1:

juliaElastic commented 1 year ago

@dborodyansky @kpollich I am wondering if we should add a confirmation modal or tooltip to the prerelease toggle, because enabling it has some implications that the user might not be aware of. Intuitively it looks like only a UI filter only, but enabling it impacts auto upgrades. Maybe this toggle could be moved to a more prominent settings page in the future.

kpollich commented 1 year ago

I am wondering if we should add a confirmation modal or tooltip to the prerelease toggle, because enabling it has some implications that the user might not be aware of

Yeah it probably makes sense to display some kind of info dialog about this. A confirmation modal might be a bit heavy, but maybe a tooltip would suffice? Something like this as a tooltip could work:

"Enabling this setting will enable the prerelease channel for all integrations. This will impact automatic integration updates and automatic policy upgrades."

Although upon thinking about this more, it seems a bit heavy-handed to perform auto updates for packages and auto upgrades for policies based on this setting. A user could very easily have a few hundred agents running on the latest stable release of system, enable this setting, then incur an immediate upgrade of all of their agents without realizing it.

Would it be possible to ignore this setting in the context of automatic updates and policy upgrades? I apologize if that creates a lot of rework or churn here, but it feels like the safest option. Happy to pull this into a follow-up issue that can be shipped during FF as well.

dborodyansky commented 1 year ago

The designed (prerelease toggle) UI element in question is intended as display toggle only, and would not be expected to have consequences beyond visibility. Coupling it with a persistent upgrade configuration setting would be problematic. If a configuration setting is required, we should redesign the UI to relocate it out of the browsing experience to where it would be logical for users. Please let me know if I misunderstood something or need to elaborate.

juliaElastic commented 1 year ago

Would it be possible to ignore this setting in the context of automatic updates and policy upgrades? I apologize if that creates a lot of rework or churn here, but it feels like the safest option. Happy to pull this into a follow-up issue that can be shipped during FF as well.

@kpollich I agree that we should avoid any unexpected upgrading to prerelease versions. I'll remove the code that checks the setting for auto and manual upgrades. A few things to clarify:

kpollich commented 1 year ago

When a GA package is installed (e.g. endpoint-8.5.0), and the prerelease toggle is on, do we still want to display the option to manually upgrade to a beta version? Otherwise users wouldn't see the beta versions, only if they uninstall the package first.

Yes, we should let users manually upgrade to prerelease/beta packages when the toggle is enabled.

We have a lot of tests that do "install latest version" on a mock package in beta version. For these we have to use the prerelease=true flag. This starts to become tricky to decide whether to use the prerelease flag without relying on the settings. I will keep testing to make sure there are no accidental auto-upgrades happening.

I think for now it makes the most sense to pass the prerelease parameter to fix the tests, but in the future it would probably be good to go through and update the test packages to align better with the new versioning constraints. I'll file an issue for this.

amolnater-qasource commented 1 year ago

Hi @juliaElastic

We have created 10 testcases for this feature under Fleet test suite at links:

Please let us know if we are missing any scenarios here. Thanks