Allow ConfigDeprecationProvider functions to access current "branch"

[watson]

In the custom deprecation messages (created inside functions of type ConfigDeprecationProvider), it's possible to put a URL for a documentation page:

kibana/x-pack/plugins/security/server/config_deprecations.ts

Lines 39 to 40 in 4b33115

	documentationUrl:
	'https://www.elastic.co/guide/en/kibana/current/security-settings-kb.html#audit-logging-settings',

However currently, the deprecation code doesn't have access to the current release "branch" (e.g. 6.8, 7.15, 7.16) so that it can customize that part of the URL, e.g:

documentationUrl:
  `https://www.elastic.co/guide/en/kibana/${branch}/security-settings-kb.html`,

For example, say you're writing a deprecation message that's meant to go live in 7.16, and you want to link to a documentation URL in that version of the documentation. If 7.16 isn't released yet, it's documentation isn't live either. So if you hardcode 7.16 into the URL it will be a 404 until the documentation is released. Alternatively you can put current. But when 8.0.0 is released, current will now point to that and everybody who's still running 7.16 might now not see the right documentation or even get a 404 if that page has been removed in 8.0.0.

So we need some way to make these documentationUrl properties "dynamic".

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)

[lukeelmers]

Seems like this could be another use case for #95389, as the docLinks service is intended to help solve this problem.

Then instead of exposing a branch/version to the deprecation providers, we just inject the docLinks.

[pgayvallet]

Then instead of exposing a branch/version to the deprecation providers, we just inject the docLinks.

The docLinks are currently client-side only (and note that moving it elsewhere would require changes in the doc build CI job)

[lukeelmers]

The docLinks are currently client-side only (and note that moving it elsewhere would require changes in the doc build CI job)

Yeah, I know it is client-side only, I guess what I was saying is that if we addressed #95389 & moved it to the server, we could then inject it into the deprecation providers. This would require passing docLinks as a dependency to ConfigService (the only dependency docLinks needs is the branch name, so this should be doable).

However, I think the ideal solution would be moving docLinks to a package (and updating the doc build job) as you outlined in #95389 (comment).

If we wanted to inject the branch name, that would be fairly straightforward in the current implementation as ConfigService already takes in env as a dependency. But IMHO it feels a bit more hacky for folks to have to build their own links from this info, especially as this is the exact problem docLinks was designed to resolve.

[jportner]

Another use case for this is when consumers use the Deprecations Service to register deprecations (example: #110960).

[watson]

Btw, if possible, we need this for 7.16 as we want to make sure those running 7.16 once 8.0 comes around still sees the right deprecation URLs

[pgayvallet]

Btw, if possible, we need this for 7.16

With such short-time notice, imho our only option would be to go the hacky way and simply expose the version to the config deprecation provider, at least for now.

export type ConfigDeprecationProvider = (factory: ConfigDeprecationFactory) => ConfigDeprecation[];

could become

export type ConfigDeprecationProvider = (factory: ConfigDeprecationFactory, context: DeprecationContext) => ConfigDeprecation[];

interface DeprecationContext {
  version: string;
  branch: string; 
  // then, later, we could inject the docLinks here
}

which would also have the upside to not be breaking the existing usages of this type/API

[pgayvallet]

We discussed with the team during today's weekly, and decided to go with the quickest approach for 7.16, by either introducing a new DeprecationContext to the ConfigDeprecationProvider, or from the ConfigDeprecation type itself, and exposing the current version and branch from it.

We'll include this issue in our current sprint

[jportner]

We discussed with the team during today's weekly, and decided to go with the quickest approach for 7.16, by either introducing a new DeprecationContext to the ConfigDeprecationProvider, or from the ConfigDeprecation type itself, and exposing the current version and branch from it.

We'll include this issue in our current sprint

That's great, do you think we can expose those fields in the existing GetDeprecationsContext as well? (that's used to register feature deprecations)

kibana/src/core/server/deprecations/types.ts

Lines 81 to 84 in 249c5fb

	export interface GetDeprecationsContext {
	esClient: IScopedClusterClient;
	savedObjectsClient: SavedObjectsClientContract;
	}

[pgayvallet]

That's great, do you think we can expose those fields in the existing GetDeprecationsContext as well?

Would make sense, will look at it too

[pgayvallet]

@jportner After taking a look, I'm not sure about the need to add these info to GetDeprecationsContext.

Config deprecations are 'statically' registered via the PluginConfigDescriptor, which makes it impossible to access branch and version from Env.

Feature deprecations, on the other hand, are registered from the plugin's setup method, which already has access to env via the plugin initializer context provided in the constructor (e.g initializerContext.env.packageInfo.branch), so there's already a way to access these info.

Do you think this would significantly improve the DX to expose them directly from GetDeprecationsContext?

[Bamieh]

@pgayvallet i think it makes sense to expose them to both contexts especially since we might need this information for config deprecations. I believe it is possible to access this information in there.

[pgayvallet]

since we might need this information for config deprecations

This is handled by #113600. I was specifically asking about feature deprecations here (registered directly via coreSetup.deprecations.getRegistry(...).registerDeprecations(...)), as the info are already accessible for them (even if it do require to propagate info from the initializer context down to the logic in the plugin handling feature deprecations), so it's not, technically, blocking.

[jportner]

Feature deprecations, on the other hand, are registered from the plugin's setup method, which already has access to env via the plugin initializer context provided in the constructor (e.g initializerContext.env.packageInfo.branch), so there's already a way to access these info.

Do you think this would significantly improve the DX to expose them directly from GetDeprecationsContext?

Yeah, I was aware, it just seems like all consumers of feature deprecations will need to know the current branch, so we might as well expose that from the context if it's not too much trouble. If it's going to be a headache I don't think it's critical though.

[pgayvallet]

#113600 introduces the config deprecations context to address this issue.

I created #114065 to add the same enhancement to feature deprecations