Remove kibana.defaultAppId setting #109798

timroes · 2021-08-24T09:53:10Z

Summary

For release notes:

Details:
The deprecated kibana.yml setting kibana.defaultAppId (also available as kibana_legacy.defaultAppId) has been removed.

Impact:
When upgrading you need to remove this setting from you kibana.yml file if used. You should use the defaultRoute Advanced Setting (within the Kibana UI > Stack Management > Advanced Setting) to configure a default route for users when entering a space.

Fixes #54088

This PR removes the kibana.defaultAppId setting, which has been deprecated and replaced since a while with the defaultRoute advanced setting.

Changes

URLs like /app/discover#/visualize that worked beforehand and brought you despite the /app/discover URL to visualize. There should be no use-case for these URLs and we drop support for them via this PR.
/app/discover/#foobarOrSomethingNonExisting lead you to the defaultAppId. The behavior for those is now changing. They will lead you to Discover and use whatever Discover (or other corresponding apps) have setup as their "unknown route" behavior. Visualize and Dashboard already had reasonable default behaviors for unknown routes. For the Home application I added it in this PR (see inline comments). Discover we have some ongoing routing work, thus I would refer the change there to a separate PR.
/app/kibana#/discover Those URLs keep working as beforehand and will forward you to the corresponding app.
/app/kibana#/foobarOrSomethingNonExisting Those URLs earlier brought you to the kibana.defaultAppId and will now bring you to the defaultRoute of the current space instead.

Checklist

Delete any items that are not applicable to this PR.

~~[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
~~[ ] Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)~~
~~[ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)~~
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
~~[ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)~~
~~[ ] This was checked for cross-browser compatibility~~

timroes · 2021-08-30T09:51:42Z

@elasticmachine merge upstream

kertal

A note about Discover changes. I wonder if we should redirect the user to another app, or like it's currently handled in this PR, just show the error message:

I think the user expects to come to Discover, so he would need to click again to return to Discover. We could e.g. show the "main" view in Discover with the message on top (and ideally add info what was the invalid URL, this could be helpful for debugging).

timroes · 2021-08-30T16:12:22Z

@kertal I think we can def improve the default routing of Discover for those cases, though while we're not having a listing page in Discover I don't know what would be a good place to bring them (in the other apps they are brough to the listing page). I'd def see this outside of this PR, so maybe you could file a separate issue for better default routing where we can discuss the best way?

kertal · 2021-08-31T06:48:33Z

@kertal I think we can def improve the default routing of Discover for those cases, though while we're not having a listing page in Discover I don't know what would be a good place to bring them (in the other apps they are brough to the listing page). I'd def see this outside of this PR, so maybe you could file a separate issue for better default routing where we can discuss the best way?

Agreed, will open an issue

timroes · 2021-08-31T13:12:36Z

@elasticmachine merge upstream

timroes · 2021-08-31T13:14:27Z

src/plugins/home/public/application/components/home_app.js

@@ -78,7 +69,7 @@ export function HomeApp({ directories, solutions }) {
              hasUserIndexPattern={() => indexPatternService.hasUserIndexPattern()}
            />
          </Route>
-          <Route path="*" exact={true} component={RedirectToDefaultApp} />
+          <Redirect to="/" />


ℹ️ This adds a redirect to the home page in case any unknown route has been entered. With the old logic the existing Route would have taken care of forwarding to the defaultAppId in this case.

Why not redirect to defaultRoute set in uiSettings?

The question is: why would only the Home page do that in this case, and not ALL apps? I feel it's a bit weird, that something like /app/home#/fooobar would bring you to the default app, but /app/dashboard#/foobar wouldn't. Or we'd basically need to set this as an expactation to all apps, that if they can't find the route in their app they should forward to the default app, which sounds like a larger change to me. I am happy to rediscuss this behavior, but I think we should not make a special case JUST for the home app and nothing else, and in this case rather let every app handle their "unknown route" internally to do something reasonable.

Or we'd basically need to set this as an expactation to all apps, that if they can't find the route in their app they should forward to the default app, which sounds like a larger change to me.

agree. that sounds like a high-level policy.

timroes · 2021-08-31T14:20:34Z

src/plugins/url_forwarding/public/forward_app/forward_app.ts

-      const [, , kibanaLegacyStart] = await core.getStartServices();
-      kibanaLegacyStart.navigateToDefaultApp();
+    const { navigated } = navigateToLegacyKibanaUrl(hash, forwards, basePath, application);
+    if (!navigated) {


ℹ️ This code is required to make sure that the following URLs keep working: /app/kibana#/foobarOrSomeOtherNonExistingNonesense Earlier the code navigated to the configured defaultAppId. With this change we'll just navigate to / in the current space (thus the basePath.get()), which will itself triggering the behavior looking for the configured defaultRoute and bringing us there.

elasticmachine · 2021-09-01T06:04:22Z

Pinging @elastic/kibana-vis-editors (Team:VisEditors)

timroes · 2021-09-01T06:13:29Z

test/functional/apps/home/_home.js

@@ -26,7 +26,7 @@ export default function ({ getService, getPageObjects }) {
    });

    it('clicking on console on homepage should take you to console app', async () => {
-      await PageObjects.common.navigateToUrl('home');
+      await PageObjects.common.navigateToApp('home');


ℹ️ This would actually not be needed with further changes I made to the PR, since those URLs are now handled properly, but imho that looks anyway nicer, so I just left it in :)

spalger

Operations: LGTM

mshustov · 2021-09-01T19:39:54Z

@timroes is the PR ready for review?

timroes · 2021-09-02T11:36:14Z

@mshustov I still wanted to provide tests for the url_forward function, but besides that it's done. I just thought I wait till I got time to finish the tests before pushing it into review... but since I anyway misclicked earlier and pinged everyone already, I think I can already set it to review know - pending some more incoming tests :D

mshustov

Ok for Home plugin changes

stratoula

Kibana-vis-editors team changes LGTM. I tested it locally in Chrome and works as expected!

@timroes the cloud PR to remove the whitelisted setting is not ready yet, right?

kibanamachine · 2021-09-03T11:50:42Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`home`	69	67	-2
`urlForwarding`	12	11	-1
total			-3

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
`kibanaLegacy`	66	64	-2
`urlForwarding`	15	12	-3
total			-5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`discover`	525.6KB	525.5KB	-75.0B
`home`	120.4KB	119.7KB	-775.0B
total			-850.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`home`	16.1KB	15.6KB	-523.0B
`kibanaLegacy`	45.9KB	45.7KB	-204.0B
`urlForwarding`	11.8KB	10.7KB	-1.1KB
total			-1.8KB

Unknown metric groups

API count

id	before	after	diff
`kibanaLegacy`	70	68	-2
`urlForwarding`	15	12	-3
total			-5

History

💛 Build #150075 was flaky 992c766
💚 Build #149812 succeeded 33071db
💚 Build #149798 succeeded 79a4db2
💚 Build #149766 succeeded 32c9bd2
💚 Build #149375 succeeded ad6d126

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

kertal

LGTM, code owner review. opened an issue to improve the changed behavior in Discover: #111172

kibanamachine · 2021-09-07T16:01:51Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 109798 or prevent reminders by adding the backport:skip label.

Remove kibana.defaultAppId setting

e9feb26

timroes added release_note:breaking Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 labels Aug 24, 2021

Tim Roes added 5 commits August 24, 2021 13:23

Fix typings

b1c95a5

Remove plugin dependency

83b2008

Merge branch 'master' into remove-defaultAppId

3bb62f3

Use proper navigation method to get to home

c092573

Default route for home

fa3ed0c

Tim Roes added 2 commits August 30, 2021 11:59

Merge branch 'master' into remove-defaultAppId

0684acf

Address discover new routing code

ad6d126

kertal reviewed Aug 30, 2021

View reviewed changes

Merge branch 'master' into remove-defaultAppId

32c9bd2

timroes commented Aug 31, 2021

View reviewed changes

Make non existing /kibana URLs working

af41c39

timroes commented Aug 31, 2021

View reviewed changes

Tim Roes added 5 commits August 31, 2021 16:20

Fix space awareness

79a4db2

Remove documentation

7016477

Remove the setting from docker file

5dc85d7

Make defaultRoute forward work properly

33071db

Merge branch 'master' into remove-defaultAppId

992c766

timroes marked this pull request as ready for review September 1, 2021 06:04

timroes requested review from a team as code owners September 1, 2021 06:04

timroes commented Sep 1, 2021

View reviewed changes

timroes marked this pull request as draft September 1, 2021 06:23

spalger approved these changes Sep 1, 2021

View reviewed changes

timroes marked this pull request as ready for review September 2, 2021 11:36

mshustov approved these changes Sep 2, 2021

View reviewed changes

Tim Roes added 2 commits September 3, 2021 11:01

Merge branch 'master' into remove-defaultAppId

fb3d950

Add forward_url tests

cf5f86f

stratoula approved these changes Sep 3, 2021

View reviewed changes

kertal mentioned this pull request Sep 3, 2021

[Discover] Improve page not found handling #111172

Closed

kertal approved these changes Sep 3, 2021

View reviewed changes

timroes merged commit b6ab15e into elastic:master Sep 3, 2021

timroes deleted the remove-defaultAppId branch September 3, 2021 16:00

This was referenced Sep 6, 2021

kibana.defaultAppId for valid app but with specific role creates endless redirect loop #52298

Closed

kibana.defaultAppId non-existent application, endless redirect loop #49495

Closed

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 7, 2021

timroes added the backport:skip This commit does not require backporting label Sep 7, 2021

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 7, 2021

timroes mentioned this pull request Sep 26, 2021

[Meta] 8.0 Kibana deprecations #109166

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove kibana.defaultAppId setting #109798

Remove kibana.defaultAppId setting #109798

timroes commented Aug 24, 2021 •

edited

Loading

timroes commented Aug 30, 2021

kertal left a comment

timroes commented Aug 30, 2021

kertal commented Aug 31, 2021

timroes commented Aug 31, 2021

timroes Aug 31, 2021

mshustov Sep 1, 2021

timroes Sep 1, 2021

mshustov Sep 1, 2021

timroes Aug 31, 2021

elasticmachine commented Sep 1, 2021

timroes Sep 1, 2021

spalger left a comment

mshustov commented Sep 1, 2021

timroes commented Sep 2, 2021

mshustov left a comment

stratoula left a comment

kibanamachine commented Sep 3, 2021

API count

kertal left a comment

kibanamachine commented Sep 7, 2021

Remove kibana.defaultAppId setting #109798

Remove kibana.defaultAppId setting #109798

Conversation

timroes commented Aug 24, 2021 • edited Loading

Summary

Changes

Checklist

timroes commented Aug 30, 2021

kertal left a comment

Choose a reason for hiding this comment

timroes commented Aug 30, 2021

kertal commented Aug 31, 2021

timroes commented Aug 31, 2021

timroes Aug 31, 2021

Choose a reason for hiding this comment

mshustov Sep 1, 2021

Choose a reason for hiding this comment

timroes Sep 1, 2021

Choose a reason for hiding this comment

mshustov Sep 1, 2021

Choose a reason for hiding this comment

timroes Aug 31, 2021

Choose a reason for hiding this comment

elasticmachine commented Sep 1, 2021

timroes Sep 1, 2021

Choose a reason for hiding this comment

spalger left a comment

Choose a reason for hiding this comment

mshustov commented Sep 1, 2021

timroes commented Sep 2, 2021

mshustov left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 3, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

kertal left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 7, 2021

timroes commented Aug 24, 2021 •

edited

Loading