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.

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

Expose current branch and version to config deprecation providers #113600

Merged
merged 11 commits into from
Oct 6, 2021

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Oct 1, 2021

Summary

Fix #112221

Introduce a new ConfigDeprecationContext type containing the current branch and version, and expose it as an additional parameter to ConfigDeprecation.

Example

const someDeprecation: ConfigDeprecation = (settings, fromPath, addDeprecation, { branch }) => {
  if (settings.foo?.deprecated) {
    addDeprecation({
      documentationUrl: `https://www.elastic.co/guide/en/kibana/${branch}/foo.html`,
      message: '"foo.deprecated" has been deprecated and will be removed in 8.0.',
      correctiveActions: {
        manualSteps: [`Remove "foo.deprecated" from your kibana configs.`],
      },
    });
  }
};

Checklist

@pgayvallet pgayvallet changed the title Expose current branch and version to deprecation providers Expose current branch and version to config deprecation providers Oct 1, 2021
@pgayvallet pgayvallet added release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.16.0 v8.0.0 labels Oct 1, 2021
@pgayvallet
Copy link
Contributor Author

retest

@pgayvallet pgayvallet marked this pull request as ready for review October 4, 2021 09:08
@pgayvallet pgayvallet requested review from a team as code owners October 4, 2021 09:08
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

Alerting changes LGTM

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Oct 4, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Fleet changes LGTM

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

Changes in Security/Spaces deprecation tests LGTM.

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

AppServices test mock changes LGTM

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Adding the context here is going to make life so much easier down the road if and when we need to hunt down when something was deprecated. Nice work!
LGTM 😄

@@ -229,4 +248,5 @@ export interface ConfigDeprecationFactory {
export interface ConfigDeprecationWithContext {
deprecation: ConfigDeprecation;
path: string;
context: ConfigDeprecationContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

The context is going to make our lives so much easier down the road!


const createDeprecationContext = (env: Env): ConfigDeprecationContext => {
return {
branch: env.packageInfo.branch,
Copy link
Contributor

Choose a reason for hiding this comment

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

I was looking at the type changes and started thinking about validation immediately. Now it all makes sense! 😄

@pgayvallet pgayvallet added the auto-backport Deprecated - use backport:version if exact versions are needed label Oct 6, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

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
core 1020 1019 -1
Unknown metric groups

API count

id before after diff
core 2293 2300 +7

History

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

@pgayvallet pgayvallet merged commit 58bab91 into elastic:master Oct 6, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 6, 2021
…elastic#113600)

* Expose deprecation context to config deprecations

* fix import

* add correct doc annotations

* fix another test file

* update generated doc

* fix yet another test file

* fix more types

* add proper mock

* fix import
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

pgayvallet added a commit to pgayvallet/kibana that referenced this pull request Oct 6, 2021
…elastic#113600)

* Expose deprecation context to config deprecations

* fix import

* add correct doc annotations

* fix another test file

* update generated doc

* fix yet another test file

* fix more types

* add proper mock

* fix import
pgayvallet added a commit that referenced this pull request Oct 6, 2021
…viders (#113600) (#114067)

* Expose current `branch` and `version` to config deprecation providers (#113600)

* Expose deprecation context to config deprecations

* fix import

* add correct doc annotations

* fix another test file

* update generated doc

* fix yet another test file

* fix more types

* add proper mock

* fix import

* lint ?

* restore changes to files removed in master
watson added a commit to watson/kibana that referenced this pull request Oct 7, 2021
watson added a commit to watson/kibana that referenced this pull request Oct 7, 2021
@watson
Copy link
Contributor

watson commented Oct 7, 2021

Created two PRs to use this in 7.x and master: #114263 + #114264

watson added a commit that referenced this pull request Oct 7, 2021
watson added a commit that referenced this pull request Oct 12, 2021
@cauemarcondes
Copy link
Contributor

@pgayvallet on APM I use core.deprecations.registerDeprecations to register a deprecation. I do it because I need to fetch some information on fleet and cloud plugins. Is there a way to get the branch variable in this case?

@lukeelmers
Copy link
Member

@cauemarcondes We have a separate issue to track adding the branch info in registerDeprecations, but it hasn't been addressed yet: #114065

In the meantime, if you are using registerDeprecations, you can get this as part of the env info that's provided in the plugin initializer context: initializerContext.env.packageInfo.branch. More info on that in this discussion: #112221 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Fleet Team label for Observability Data Collection Fleet team v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow ConfigDeprecationProvider functions to access current "branch"