-
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
[Stack management apps] Deprecate "enabled" Kibana setting #114768
[Stack management apps] Deprecate "enabled" Kibana setting #114768
Conversation
return completeConfig; | ||
} | ||
|
||
addDeprecation({ |
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.
In theory this does not make sense as if UA is disabled then it won't be able to surface its own deprecated settings. I decided to put it anyway to align with all the other apps in case there are, in the future, other consumers of the Deprecation service.
e53f549
to
338488a
Compare
Pinging @elastic/kibana-stack-management (Team:Stack Management) |
Devtools is indeed just a holder for our apps (Console, painless lab, grok debugger, search profiler).
I am confused as I thought you said that our apps could not be added to features control as they rely on ES privileges. Is console an exception? |
Dev Tools is a bit of a special case. Well, Console specifically (I don't know the other apps well enough). Console is effectively a friendly proxy to ES -- it doesn't require any specific ES privileges to run; It'll just use whatever privileges the current user has. We added kibana/src/plugins/console/server/routes/api/console/proxy/index.ts Lines 14 to 29 in 4584a8b
|
Thanks for the explanation @legrego. As we are very close to FF we are going to keep using the |
…belga/kibana into stack-managment-apps-deprecate-enable
I've added the missing doc for |
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.
app services src/plugins/inspector/public/views/requests/components/details/req_code_viewer.tsx
lgtm
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.
Hi @sebelga, thank you so much for adding ui.enabled
setting to all of our apps, impressive job! I tested locally and haven't noticed any problems with plugins.
The slm_ui.enabled
config in Snapshot & Restore disables snapshot policies tab and api routes, but I couldn't find it in the docs. I'm wondering now that we have the ui.enabled
config if the slm_ui.enabled
config should either be deleted or added to the docs as an alternative to ui.enabled
, wdyt? But that's non-blocking for this PR.
* Fix copy * Configure level
* Clarify distinction between Dev Tools and Console.
d5abd6f
to
596c605
Compare
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 covering the edge cases in which other plugins link to Console! I didn't test those locally but the code and screenshots LGTM.
I pushed a few changes to copy, docs, and comments. Regarding slm_ui.enabled
, I'm not sure if it's worth documenting but I don't think we should remove it. Removing it won't offer any benefits to the user and it will incur the cost of another deprecation, so not worth it IMO.
@sebelga Once you merge this, please ping me for a review of the backport so I can test this in the 7.16 Upgrade Assistant UI, which is significantly different from the UI on master. I'd like to take responsibility for merging the backport once it's gone green.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
Thanks for the review @cjcenizal @yuliacech and @sabarasaba !
ok 👍 |
…14768) # Conflicts: # docs/dev-tools/console/console.asciidoc # docs/setup/settings.asciidoc # x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/index.ts # x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/setup_environment.tsx # x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/overview.test.tsx # x-pack/plugins/upgrade_assistant/common/config.ts # x-pack/plugins/upgrade_assistant/common/constants.ts # x-pack/plugins/upgrade_assistant/public/application/components/es_deprecations/deprecation_types/reindex/flyout/warning_step.test.tsx # x-pack/plugins/upgrade_assistant/public/plugin.ts # x-pack/plugins/upgrade_assistant/public/types.ts
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/ml/jobs/categorization_field_examples·ts.apis Machine Learning jobs Categorization example endpoint - invalid, too many tokens.Standard Out
Stack Trace
Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
…4768) (#115541) * [Stack management apps] Deprecate "enabled" Kibana setting (#114768) # Conflicts: # docs/dev-tools/console/console.asciidoc # docs/setup/settings.asciidoc # x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/index.ts # x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/setup_environment.tsx # x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/overview.test.tsx # x-pack/plugins/upgrade_assistant/common/config.ts # x-pack/plugins/upgrade_assistant/common/constants.ts # x-pack/plugins/upgrade_assistant/public/application/components/es_deprecations/deprecation_types/reindex/flyout/warning_step.test.tsx # x-pack/plugins/upgrade_assistant/public/plugin.ts # x-pack/plugins/upgrade_assistant/public/types.ts * Fix doc issue * [CCR] Update "MAJOR_VERSION" constant * [ILM] Update "MAJOR_VERSION" constant * [License Management] Update "MAJOR_VERSION" constant * [Remote Clusters] Update "MAJOR_VERSION" constant * [Rollup] Update "MAJOR_VERSION" constant * [S&R] Update "MAJOR_VERSION" constant * [UA] Update "MAJOR_VERSION" constant * Fix default value for "readonly" in 7.16.0
In
8.0
we deprecate theenabled
Kibana settings for the apps. As we want to offer an alternative way for our users to disable the UI we had 2 options: add the stack management app to Features control or use another Kibana setting (ui.enabled
) for that purpose. The former would be the ideal way as it is the easiest and can be done through our UI. Unfortunately the Features control does not support yet disabling apps that depend on ES privileges so the security team has opened an issue to solve that.In the meantime we are going to rely on the
ui.enabled
Kibana setting to let our users disable the UI of our apps.Some of our apps already use this setting (CCR, ILM, license management, remote clusters) so for those ones I've only added the deprecation notice for UA. For the rest of our apps I have added the
ui.enabled
setting to the plugin config.Notes on implementation
As I mentioned several times I like the idea of having the same code base in
master
and7.x
. The reasons are:master
and can test different behaviours on7.x
by changing 1 constant7.x
tomaster
(when working against7.x
), like we had to do in Partial forward-port of dacf0c2a6cabf088b0f17f4eca57e01c466286c0. #113672 and [Upgrade Assistant] Forwardport from 7.x #114966yarn kbn bootstrap
andyarn start
)7.x
and modify a component we won't ever be able to automatically backport that component frommaster
after that. We'll need to manually backport the change and remember why7.x
is different and be sure to keep the change in place. Sure tests should have our back but it is not a fun process. Specially if the component is big and is not in our domain expertise.7.x
and what is in other majors.How to test
Note I have aligned all apps to the same naming convention (using the
MAJOR_VERSION
const) and file structure (expose that constant from thecommon
folder, handle all the plugin config from inside the./server/config.ts
file).enabled
(e.g.xpack.index_management.enabled: false
) setting inkibana.yml
and check that the server throws an errorui.enabled
. The server restarts and the UI is not thereMAJOR_VERSION
const (in thecommon
folder) to7.16.0
(the minor does not matter).ui.enabled
setting toenabled
. This time the server should not throw (we are simulating being on 7.16.0) and the UI should be disabledx-pack/plugins/upgrade_assistant/public/plugin.ts
and change L64 (replacereadonly
withfalse
)enabled
setting.Test Console
Console expose a URL locator to other apps to link to it. If console UI is disabled the apps linking to it should not display the console button.
console.ui.enabled: false
in thekibana.yml
src/plugins/kibana_overview/public/components/overview/overview.tsx
and change L212 (solutions.length ?
tofalse
).Improve console.proxyConfig and
console.proxyFilter
warningAs we are very close to FF I've added 6813c6a to improve the deprecation message for the
console.proxyConfig
andconsole.proxyFilter
settings.Fixes #112605
Fixes #113810