-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[console] Deprecate "proxyFilter" and "proxyConfig" on 8.x #113555
[console] Deprecate "proxyFilter" and "proxyConfig" on 8.x #113555
Conversation
@@ -5,14 +5,17 @@ | |||
* in compliance with, at your election, the Elastic License 2.0 or the Server | |||
* Side Public License, v 1. | |||
*/ | |||
import { SemVer } from 'semver'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turn on "Hide whitespace changes" in Github to review this file
…oxyConfig-proxyFilter-support
Pinging @elastic/kibana-stack-management (Team:Stack Management) |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for patching this up @sebelga! Code LGTM and was able to test everything and can confirm it works as expected.
I know its not on us, but I was expecting that when starting up the server with the deprecated settings and MAJOR_VERSION=8.0.0
I would see an error saying that certain setting was deprecated. But instead I got the following which didn't seem very user friendly:
erver log [16:19:33.479] [fatal][root] ValidationError: [config validation of [console].proxyFilter]: definition for this key is missing
at ObjectType.validate (/Users/ignacio/Projects/elastic/kibana/node_modules/@kbn/config-schema/target_node/types/type.js:95:13)
at ConfigService.validateAtPath (/Users/ignacio/Projects/elastic/kibana/node_modules/@kbn/config/target_node/config_service.js:244:19)
at MapSubscriber.project (/Users/ignacio/Projects/elastic/kibana/node_modules/@kbn/config/target_node/config_service.js:252:169)
at MapSubscriber._next (/Users/ignacio/Projects/elastic/kibana/node_modules/rxjs/src/internal/operators/map.ts:84:29)
at MapSubscriber.Subscriber.next (/Users/ignacio/Projects/elastic/kibana/node_modules/rxjs/src/internal/Subscriber.ts:99:12)
at DistinctUntilChangedSubscriber._next (/Users/ignacio/Projects/elastic/kibana/node_modules/rxjs/src/internal/operators/distinctUntilChanged.ts:121:24)
at DistinctUntilChangedSubscriber.Subscriber.next (/Users/ignacio/Projects/elastic/kibana/node_modules/rxjs/src/internal/Subscriber.ts:99:12)
at MapSubscriber._next (/Users/ignacio/Projects/elastic/kibana/node_modules/rxjs/src/internal/operators/map.ts:89:22)
at MapSubscriber.Subscriber.next (/Users/ignacio/Projects/elastic/kibana/node_modules/rxjs/src/internal/Subscriber.ts:99:12)
at ReplaySubject._subscribe (/Users/ignacio/Projects/elastic/kibana/node_modules/rxjs/src/internal/ReplaySubject.ts:82:20) {
cause: SchemaTypeError: definition for this key is missing
at ObjectType.onError (/Users/ignacio/Projects/elastic/kibana/node_modules/@kbn/config-schema/target_node/types/type.js:137:14)
at Object.error (/Users/ignacio/Projects/elastic/kibana/node_modules/@kbn/config-schema/target_node/types/type.js:79:47)
at Object.internals.finalize (/Users/ignacio/Projects/elastic/kibana/node_modules/joi/lib/validator.js:472:36)
at Object.exports.validate (/Users/ignacio/Projects/elastic/kibana/node_modules/joi/lib/validator.js:333:38)
at Object.internals.entry (/Users/ignacio/Projects/elastic/kibana/node_modules/joi/lib/validator.js:142:28)
at Object.exports.entry (/Users/ignacio/Projects/elastic/kibana/node_modules/joi/lib/validator.js:27:30)
at internals.Base.validate (/Users/ignacio/Projects/elastic/kibana/node_modules/joi/lib/base.js:548:26)
at ObjectType.validate (/Users/ignacio/Projects/elastic/kibana/node_modules/@kbn/config-schema/target_node/types/type.js:89:29)
at ConfigService.validateAtPath (/Users/ignacio/Projects/elastic/kibana/node_modules/@kbn/config/target_node/config_service.js:244:19)
at MapSubscriber.project (/Users/ignacio/Projects/elastic/kibana/node_modules/@kbn/config/target_node/config_service.js:252:169) {
cause: undefined,
path: [ 'proxyFilter' ]
}
}
FATAL Error: [config validation of [console].proxyFilter]: definition for this key is missing
I'm not sure if this issue should be raised to another team or if it's somehow expected 🤔
@@ -1,12 +1,14 @@ | |||
{ | |||
"id": "console", | |||
"version": "kibana", | |||
"version": "8.0.0", | |||
"kibanaVersion": "kibana", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious about this. I see in the docs that its the version of kibana the plugin is compatible with, so by specifying "kibana" it means that is compatible with all versions? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't given it too much thoughts to be honest. I just followed what all the other plugins are doing 😄 "kibana"
seems to be the kibana version declared everywhere.
Thanks for the review @sabarasaba !
Yes it is expected. But that's why we have the Upgrade assistant 😊 A user should not have any deprecated settings anymore before migrating. |
Indeed! I just felt that the error message could be improved to say something like "this setting was deprecated in x.y.z" rather than "unknown setting". I'll check if there's an issue created for that already and if not I can create one! |
That would be great indeed @sabarasaba 👍 |
Remove support for the "proxyFilter" and "proxyConfig" Kibana setting in console from
8.x
.How to test
Add the following in your
kibana.yml
The Kibana server should throw an error mentioning that those settings are unknown.
Now change the
MAJOR_VERSION
constant insrc/plugins/console/common/constants/plugin.ts
to7.16.0
. The server will restart and won't throw an error for those settings anymore.Test the deprecation in Upgrade Assistant:
x-pack/plugins/upgrade_assistant/public/plugin.ts
const readonly = false;
Release note
We removed support for the
console.proxyFilter
andconsole.proxyConfig
Kibana setting in favour of the standardserver.ssl.*
andelasticsearch.ssl.keyPassphrase
settings.Fixes #39494