[discuss] Allow to ignore specific deprecations

[pgayvallet]

Currently, all cloud-related information are exposed from the cloud xpack plugin. This has two main limitationd limitations:

• Being exposed as a plugin, there's currently no way to access these info from core itself.
• The cloud plugin having dependencies toward usageCollection, home and security, it's not possible to access the cloud config from the dependency plugins without introducing a cyclic dependency (that would force to split the cloud plugin into multiple pieces)

This question come back often. The two most recent example being:

• Accessing isCloudEnabled from the home plugin: Self-managed telemetry Privacy Statement notice shown in Cloud environment #110638
• Accessing isCloudEnabled from core to toggle some legacy logging deprecations

With cloud being a first-class citizen of our 'customers', and with our current focus on cloud, I suspect that this kind of need will be more and more frequent. Which is why I'm opening this issue to discuss the possibility to move and expose the cloud config from core itself.

We could do that this way:

• deprecates xpack.cloud.* in favor of either cloud.* or core.cloud.* (with a proper rename deprecation for BWC)
• introduce a new cloud core service, both on the server and client-side
  • it would expose the 'basic' cloud config: cloudId, deploymentId, isCloudEnabled
• have the cloud plugin consume this config instead of using its own plugin config
  • not sure if we want 'specialized' config props (such as profileUrl, snapshotsUrl) to be exposed from the core service or from the cloud plugin

cc @kobelb @stacey-gammon is there any licensing concern on having the base cloud config living inside core instead of an xpack plugin?

[elasticmachine]

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

[kobelb]

cc @kobelb @stacey-gammon is there any licensing concern on having the base cloud config living inside core instead of an xpack plugin?

From the licensing perspective given what the cloud plugin currently does, no.

However, I'm curious about the following reasoning:

The cloud plugin having dependencies toward usageCollection, home and security, it's not possible to access the cloud config from the dependency plugins without introducing a cyclic dependency (that would force to split the cloud plugin into multiple pieces)

During previous discussions, we decided that it's acceptable to have OSS plugins have optional dependencies on x-pack plugins. This makes it possible for us to make all of these plugins have dependencies on the cloud plugin, where previously we couldn't.

This just leaves us with core. Do we really think that core should be inseparable from "cloud"? Conceptually, this doesn't make much sense to me since we will continue to ship an on-prem version of Kibana for the foreseeable future.

Only moving the cloud-config to core seems equally awkward. Is it really the only option that we have here?

[pgayvallet]

During previous discussions, we decided that it's acceptable to have OSS plugins have optional dependencies on x-pack plugins. This makes it possible for us to make all of these plugins have dependencies on the cloud plugin, where previously we couldn't.

True. However we can't have bi-lateral dependencies, and cloud has a few dependency (home for example). The current plugins architecture/hierarchy means that home can't require cloud for any reason (such as displaying something specific when on cloud). Of course, this could be solved by splitting cloud in two, cloud-api and cloud-app, but given that core may also need to access this info at some point, this doesn't solve the problem as a whole.

Do we really think that core should be inseparable from "cloud"? Conceptually, this doesn't make much sense to me since we will continue to ship an on-prem version of Kibana for the foreseeable future.

I don't think core should be inseparable from cloud, but I feel like core should be aware of if it's running on cloud or not, one way or another.

Only moving the cloud-config to core seems equally awkward. Is it really the only option that we have here?

That's definitely the easiest one, as it would require changes only in the Kibana codebase. But if we think this is not acceptable (and we do want core to be aware of if it's running on cloud or not), we could open the discussion with the cloud team to seek potential other options.

[kobelb]

Thinking long-term, I don't think that we want the Cloud config to be part of core. I'm having a hard time thinking of a future situation where we'd need core itself to know whether or not it's running on Cloud, outside of the current predicament that we're in because ESS can't use the new logging format yet.

If we're just talking about code that would be in 7.16 to accommodate for the logging issue, do we feel any differently about accessing the xpack.cloud config directly from core? Could we do so safely and only have this code exist in the 7.16 branch?

[pgayvallet]

Thinking long-term, I don't think that we want the Cloud config to be part of core. I'm having a hard time thinking of a future situation where we'd need core itself to know whether or not it's running on Cloud

You probably have a way better long-term vision than I do regarding this, so I will trust you on this.

(I will still !remindme 6month though 😉)

, do we feel any differently about accessing the xpack.cloud config directly from core? Could we do so safely and only have this code exist in the 7.16 branch?

As said during our sync discussion, that's not because we can access every parts of the config from within core that we should. I really do hate the idea of hacking our ways in the config just to disable some deprecations.

If we do decide that we don't want to move the (basic) cloud config in core, I'd say that's not reason enough to just go the hacky way. In that case, I would just either try to find another solution, or just let the legacy logging deprecations visible on cloud in 7.16. But that's only a personal opinion, let's see what the other @elastic/kibana-core members think about it.

Also, note that this is only one part of the problem. We also have a (more) concrete issue of cloud's dependency plugin needing to access the cloud API in #110638. What should we do about this one? Split the cloud plugin into two parts as suggested in #113734 (comment)?

[mshustov]

Thinking long-term, I don't think that we want the Cloud config to be part of core.

My ++. I don't really like the fact Kibana changes its behavior in the specific environment due to several reasons:

• Cloud business rules leak into Kibana. Let's take the example of logging? Why Kibana should how Cloud configures logging? How can we be sure that these rules have not been changed by the Cloud O11y team?
• Changing these business rules will require releasing a new Kibana version. Cloud and Kibana release cycles aren't synchronized.
• It's really hard to test these business rules. Ideally, Kibana devs should be able to adjust the config to test Cloud-specific behavior without waiting for a SNAPSHOT to be available on Cloud.
• There might be a plethora of different environments, putting env-specific logic in the code is not really scalable. How soon we might need ECE- or ECK-specific logic?

do we feel any differently about accessing the xpack.cloud config directly from core? Could we do so safely and only have this code exist in the 7.16 branch?

It might solve the problem in short term, just let the legacy logging deprecations visible on cloud in 7.16. is also a viable option then.

WDYT about my suggestion to extend UA with API allowing Cloud to suppress some of the Cloud-specific depreciations? Other Cloud-specific rules may follow the same pattern:

# cloud kibana.yml
logging: .....
x-pack.upgrade_assistant.mute: ['logging....']
home.ui.showSelfManagedTelemetry: false

[kobelb]

WDYT about my suggestion to extend UA with API allowing Cloud to suppress some of the Cloud-specific depreciations? Other Cloud-specific rules may follow the same pattern:

If it's technically feasible, it sounds good to me.

[lukeelmers]

I was originally in favor of having isCloudEnabled exposed from core, but I think these two points have changed my mind:

There might be a plethora of different environments, putting env-specific logic in the code is not really scalable. How soon we might need ECE- or ECK-specific logic?

This is probably the thing that would make me the most nervous about adding the logic into core.

During previous discussions, we decided that it's acceptable to have OSS plugins have optional dependencies on x-pack plugins.

I also had forgotten about this discussion. If we are mostly doing this as a workaround for the issues linked in the description, then for now I am +1 on exploring other options (like the UA-specific API, and possibly splitting the cloud plugin if we think #110638 is important enough). After all, we can always add to core later if we change our minds. Much harder to subtract 🙂

[pgayvallet]

WDYT about my suggestion to extend UA with API allowing Cloud to suppress some of the Cloud-specific depreciations?

As it seems we don't want to 'ship' cloud into core, that looks like the most realistic alternative to work around the specific 'legacy logging deprecation showing in cloud 7.16 instances' issue.

However, note that UA does not have the info about the path of the setting being deprecated, as the DeprecationDetails type is generic for all kind of deprecations, so that x-pack.upgrade_assistant.mute: ['logging....'] syntax wouldn't be usable without additional changes in core (in addition to changes in UA I mean)

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

Lines 18 to 76 in 249c5fb

	export interface DeprecationsDetails {
	/**
	* The title of the deprecation.
	* Check the README for writing deprecations in `src/core/server/deprecations/README.mdx`
	*/
	title: string;
	/**
	* The description message to be displayed for the deprecation.
	* Check the README for writing deprecations in `src/core/server/deprecations/README.mdx`
	*/
	message: string;
	/**
	* levels:
	* - warning: will not break deployment upon upgrade
	* - critical: needs to be addressed before upgrade.
	* - fetch_error: Deprecations service failed to grab the deprecation details for the domain.
	*/
	level: 'warning' | 'critical' | 'fetch_error';
	/**
	* (optional) Used to identify between different deprecation types.
	* Example use case: in Upgrade Assistant, we may want to allow the user to sort by
	* deprecation type or show each type in a separate tab.
	*
	* Feel free to add new types if necessary.
	* Predefined types are necessary to reduce having similar definitions with different keywords
	* across kibana deprecations.
	*/
	deprecationType?: 'config' | 'feature';
	/* (optional) link to the documentation for more details on the deprecation. */
	documentationUrl?: string;
	/* (optional) specify the fix for this deprecation requires a full kibana restart. */
	requireRestart?: boolean;
	/* corrective action needed to fix this deprecation. */
	correctiveActions: {
	/**
	* (optional) The api to be called to automatically fix the deprecation
	* Each domain should implement a POST/PUT route for their plugin to
	* handle their deprecations.
	*/
	api?: {
	/* Kibana route path. Passing a query string is allowed */
	path: string;
	/* Kibana route method: 'POST' or 'PUT'. */
	method: 'POST' | 'PUT';
	/* Additional details to be passed to the route. */
	body?: {
	[key: string]: any;
	};
	};
	/**
	* Specify a list of manual steps users need to follow to
	* fix the deprecation before upgrade. Required even if an API
	* corrective action is set in case the API fails.
	* Check the README for writing deprecations in `src/core/server/deprecations/README.mdx`
	*/
	manualSteps: string[];
	};
	}

Given the timeline, would we be able to perform those changes before 7.16FF ? If not, do we think this is important enough to try and merge it after FF?

[mshustov]

If not, do we think this is important enough to try and merge it after FF?

That's a product requirement IMO. cc @thesmallestduck

syntax wouldn't be usable without additional changes in core (in addition to changes in UA I mean)

Sure, it will. @cjcenizal Do you agree on extending UA API?

[cjcenizal]

@mshustov @pgayvallet and others, just so I'm clear on this: the proposal is to offer a new setting in kibana.yml that would allow the administrator or orchestration service (Cloud) to blocklist a set of deprecation warnings that have been registered with the Kibana Deprecations API, correct?

If so I'm +1 on this proposal.

I just want to voice a couple concerns:

• Are you expecting UA to apply this blocklist itself? I'd expect the Kibana Deprecations API to apply the blocklist, so any blocked deprecations are automatically excluded from the responses to requests that retrieve the deprecations. This seems the most ergonomic to me -- does this sound right to you?
• @jakelandis Is this approach analogous to how y'all are approaching similar functionality on the ES side with the Deprecation Info API?

[jakelandis]

@cjcenizal - Elasticsearch has just introduced a new setting deprecation.skip_deprecated_settings such that Cloud (or ECE providers) can set this in the elasticsearch.yml to essentially ignore some deprecated settings. Once set the deprecation info API will no longer emit these deprecation (elastic/elasticsearch#78725) and soon the deprecation logs will also read this setting.

The intent is that we will work with Cloud for the 7.x last version to identify the small handful of settings that they set that are deprecated (like the transport client settings) but the user can/should not change. Then when the cluster is upgraded to the 7.x last this new setting will be added to the cluster so that when the user starts to look at the upgrade asssistant (which reads from the deprecation info API) they will not see these specific deprecation's.

Also, we (ES) limited our setting to just skip specific deprecated settings not any random deprecations. This is due to a lack of unique identifier for deprecations (settings have a natural unique identifier) and we don't have any known deprecrations outside of settings that we want to skip.

[pgayvallet]

I'd expect the Kibana Deprecations API to apply the blocklist, so any blocked deprecations are automatically excluded from the responses to requests that retrieve the deprecations

Imho you're right, that seems to be a better design than having UA filter the deprecations itself.

Also, we (ES) limited our setting to just skip specific deprecated settings not any random deprecations. This is due to a lack of unique identifier for deprecations (settings have a natural unique identifier) and we don't have any known deprecrations outside of settings that we want to skip.

That was going to be my next point. We also don't have unique identifiers for deprecations. However in our case, we unfortunately don't (necessarily) have the setting's path for config deprecations, e.g

const destLoggingDeprecation: ConfigDeprecation = (settings, fromPath, addDeprecation) => {
  if (settings.logging?.dest) {
    addDeprecation({
      documentationUrl:
        'https://github.com/elastic/kibana/blob/master/src/core/server/logging/README.mdx#loggingdest',
      title: i18n.translate('core.deprecations.loggingDest.deprecationTitle', {
        defaultMessage: `Setting "logging.dest" is deprecated`,
      }),
      message: i18n.translate('core.deprecations.loggingDest.deprecationMessage', {
        defaultMessage:
          '"logging.dest" has been deprecated and will be removed ' +
          'in 8.0. To set the destination moving forward, you can use the "console" appender ' +
          'in your logging configuration or define a custom one. For more details, see ' +
          'https://github.com/elastic/kibana/blob/master/src/core/server/logging/README.mdx',
      }),
      correctiveActions: {
        ...
      },
    });
  }
};

So we will need to be able to identify them somehow.

My draft proposal would be:

• Add a new id property to DeprecatedConfigDetails
  • I don't really like it, but to avoid changing all the current deprecation registrations (and given this would only be used for this niche feature atm), this property would be optional?
• Introduce a new deprecations config prefix
  • deprecations would imho be better because it reflects the name of the associated core service, but do we want to have a deprecation (singular) prefix instead to be consistent with the ES naming?
• Add a new deprecation(s).ignore config property (is ignore fine? Does anyone have a better suggestion?)
  • would be an array
  • would ideally support * wildcard, such as logging.* (or do we think this is dangerous?)
• Use this new config property in the deprecations service to filter/ignore matching deprecation

Once this is implemented, for our specific example, the required changes in the config would be:

deprecations:
  ignore:
    - 'logging.*'

(and of course, also add the id property to the logging deprecations:)

const destLoggingDeprecation: ConfigDeprecation = (settings, fromPath, addDeprecation) => {
  if (settings.logging?.dest) {
    addDeprecation({
      id: 'logging.dest',
      documentationUrl: '...',
      title: '...',
      // ...
    });
  }
};

Note: for the specific legacy logging deprecations, if we do want to use/allow the * wildcard, we could identify the deprecations with logging.legacy.dest instead of just logging.dest, to avoid ignoring potential non-legacy deprecations (even if that's unlikely to be a problem as we're only targeting 7.16 here for these specific deprecations)

@mshustov @cjcenizal WDYT?

[mshustov]

@mshustov @pgayvallet and others, just so I'm clear on this: the proposal is to offer a new setting in kibana.yml that would allow the administrator or orchestration service (Cloud) to blocklist a set of deprecation warnings that have been registered with the Kibana Deprecations API, correct?

Correct. In the case of the Cloud, domain owners will be responsible to update these settings. For example, the Kibana Core team will add logging settings in the blocklist.

I'd expect the Kibana Deprecations API to apply the blocklist, so any blocked deprecations are automatically excluded from the responses to requests that retrieve the deprecations

Agree, it sounds like a deprecation service responsibility. I'd expect Kibana not to log a deprecation warning if a deprecation of a setting is "blocked".

Add a new id property to DeprecatedConfigDetails

The most practical solution is to add any unique identifier, so my +1. There are 40 deprecations, so the number is manageable to update manually.

Add a new deprecation(s).ignore config property (is ignore fine? Does anyone have a better suggestion?)

For the sake of consistency and better UX, I'd suggest using the same naming pattern that Elasticsearch config does: deprecation.skip_deprecated_settings.

would ideally support * wildcard, such as logging.* (or do we think this is dangerous?)

The same approach is applicable: if Elasticsearch config doesn't support a wildcard, Kibana shouldn't do so.

[pgayvallet]

For the sake of consistency and better UX, I'd suggest using the same naming pattern that Elasticsearch config does: deprecation.skip_deprecated_settings.

Fine with me.

Do you think we'll also want to only allow to skip/ignore config type deprecations, as it is what ES is doing (would be fine to our usecase here)

[mshustov]

Do you think we'll also want to only allow to skip/ignore config type deprecations, as it is what ES is doing (would be fine to our usecase here)

I can imagine that we might want to deprecate other Kibana API (an endpoint, for example), but IMO we should align the deprecation approach across the Stack. Should we deprecate other entities later, we can extend functionality: for example, for an endpoint, we can introduce deprecation.skip_deprecated_endpoint setting for the whole Stack.
Right now, let's do what Elasticsearch does.

[cjcenizal]

I'm pleased with where y'all have landed, @mshustov and @pgayvallet. Thank you for driving this forward!

[pgayvallet]

FWIW, Looking at the ES implementation, the skipped settings are based on their config path, not on an arbitrary ID. Given that we decided that Kibana will also only support skipping config-type deprecation (for now at least), I will add a configPath property to config deprecations instead of an id, to better reflect that their (non-unique) identifier for skipping will be their path in the config.

[pgayvallet]

Closed by #114751