Remove Space Selector from Serverless Top Navigation #164461

SiddharthMantri · 2023-08-22T14:00:50Z

Summary

For Serverless projects, there will be only a single default space, with no way to create/select additional spaces. For this reason, the spaces navigation control has been removed for serverless only

Changes Made

Added isServerless boolean flag to depend on the plugin context buildFlavor in the Serverless plugin
Based on the flag
- Disabled rendering of spaces nav control
- Disabled the space management route
- Disabled adding spaces to feature catalogue
Updated plugin tests to check for serverless or traditional build flavors

UI Changes

Before	After

Release Notes

Security
Removed the space selector from the Serverless UI. #163024

SiddharthMantri · 2023-08-22T14:09:17Z

@azasypkin @thomheymann I wonder if it's better to do it the way i've done in this PR or pass a isSpaceSelectorDisabled prop to this function

kibana/x-pack/plugins/spaces/public/nav_control/nav_control.tsx

Line 17 in 54eb873

    
           export function initSpacesNavControl(spacesManager: SpacesManager, core: CoreStart) {

x-pack/plugins/spaces/public/plugin.tsx

kc13greiner

Looking good - just a few questions, don't hesitate to reach out if my comments aren't clear 😅

x-pack/plugins/spaces/public/plugin.test.ts

kc13greiner · 2023-08-25T03:01:55Z

x-pack/plugins/spaces/public/plugin.tsx

      plugins.home.featureCatalogue.register(createSpacesFeatureCatalogueEntry());
    }

-    if (plugins.management) {
+    if (plugins.management && !this.isServerless) {


questions: Should the spaceSelectorApp.create(...) call below also be in a !isServerless block?

@kc13greiner I think you're right. I added the check just below this

Awesome! Let's double check w/ @thomheymann

One additional nit: could the !isServerless check be pulled out into a top level if statement that wraps these blocks/statements?

@kc13greiner Absolutely! Just updated!

The spaces selector app exposes a route that can be accessed to view all spaces and to select a different space.

This is only linked to from the space selector and when deleting the current space as far as I can see so I think this can be safely disabled on serverless.

…ss in spaces plugin

…fix'

elasticmachine · 2023-08-29T10:57:12Z

Pinging @elastic/kibana-security (Team:Security)

kc13greiner

LGTM!

jeramysoucy

Overall looks great. Just two things.

I was confused about the tests regarding the spaces API. Don't these test just check whether the space selector application is registered?

~~2. I think we should augment the serverless test suite to confirm the UI layout does not include the space selector. Maybe in x-pack/test_serverless/functional/page_objects/svl_common_page.ts~~ As discussed in our sync, we don't really need this - too short term, and does not add value. Sorry about that.

jeramysoucy · 2023-08-29T13:23:10Z

x-pack/plugins/spaces/public/plugin.test.ts

@@ -33,7 +37,23 @@ describe('Spaces plugin', () => {
      );
    });

-    it('should register the management and feature catalogue sections when the management and home plugins are both available', () => {
+    it('should not register the spaces API and the space selector app when buildFlavor is serverless', () => {


What is meant by spaces API here? Doesn't this test confirm only the space selector app registration?

@jeramysoucy You're right! I changed the label to make it clear. It should say spaces routes and not API.

@SiddharthMantri The same wording is used on line 20.

x-pack/plugins/spaces/public/plugin.test.ts

TinaHeiligers · 2023-08-29T18:22:13Z

packages/core/plugins/core-plugins-browser-mocks/src/plugins_service.mock.ts

-const createPluginInitializerContextMock = (config: unknown = {}) => {
+const createPluginInitializerContextMock = (
+  config: unknown = {},
+  { buildFlavor = 'serverless' }: { buildFlavor?: BuildFlavor } = {}


For UI-only changes, using the config should be ok although core now has an elasticsearch capabilities API that gets info from the cluster directly.

The new API is better than using a config flag because KIbana's sometimes run against a stateful ES instance. @pgayvallet is the capabilities API meant to be used instead?

Yea, this is fine.

TinaHeiligers

LGTM

TinaHeiligers · 2023-08-29T19:42:20Z

packages/core/plugins/core-plugins-browser-mocks/src/plugins_service.mock.ts

-const createPluginInitializerContextMock = (config: unknown = {}) => {
+const createPluginInitializerContextMock = (
+  config: unknown = {},
+  { buildFlavor = 'serverless' }: { buildFlavor?: BuildFlavor } = {}


Yea, this is fine.

kibana-ci · 2023-08-30T08:48:54Z

💚 Build Succeeded

Buildkite Build
Commit: 45655b1

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
`@kbn/core-plugins-browser-mocks`	6	7	+1

Page load bundle

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

id	before	after	diff
`spaces`	24.3KB	24.4KB	+157.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-plugins-browser-mocks`	6	7	+1

History

💔 Build #154636 failed 28c6bb6
💚 Build #154542 succeeded 4cedff5
💛 Build #154457 was flaky 8ea4765
💚 Build #154411 succeeded ade6b03
💔 Build #154098 failed ec9e5d3

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

SiddharthMantri and others added 2 commits August 22, 2023 15:59

Use buildflavor to disable space selector and route

607cee4

Merge branch 'main' into disable-space-selector-serverless

54eb873

SiddharthMantri commented Aug 22, 2023

View reviewed changes

x-pack/plugins/spaces/public/plugin.tsx Outdated Show resolved Hide resolved

SiddharthMantri and others added 4 commits August 23, 2023 09:52

Merge branch 'main' into disable-space-selector-serverless

a5532f9

Use isServerless boolean flag and update plugin test

1ab5558

Merge branch 'main' into disable-space-selector-serverless

ba0a842

Merge branch 'main' into disable-space-selector-serverless

65f4017

SiddharthMantri marked this pull request as ready for review August 23, 2023 11:47

Merge branch 'main' into disable-space-selector-serverless

2f4df52

SiddharthMantri requested a review from a team as a code owner August 23, 2023 11:47

SiddharthMantri requested review from thomheymann and azasypkin August 23, 2023 11:47

SiddharthMantri added 3 commits August 23, 2023 20:12

Merge branch 'main' into disable-space-selector-serverless

41f5fa1

Merge branch 'main' into disable-space-selector-serverless

fd7e40e

Merge branch 'main' into disable-space-selector-serverless

56a3fd0

kc13greiner self-requested a review August 24, 2023 19:57

kc13greiner reviewed Aug 25, 2023

View reviewed changes

Fixed tests and added serverless check for spaces app

ec9e5d3

SiddharthMantri requested a review from kc13greiner August 28, 2023 13:23

Update signature of mocking function and wrapped check for isServerle…

6698bb8

…ss in spaces plugin

SiddharthMantri requested a review from a team as a code owner August 29, 2023 08:42

SiddharthMantri and others added 3 commits August 29, 2023 10:44

Merge branch 'main' into disable-space-selector-serverless

7c1f14e

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

ade6b03

…fix'

Merge branch 'main' into disable-space-selector-serverless

8ea4765

SiddharthMantri added the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Aug 29, 2023

SiddharthMantri added the backport:skip This commit does not require backporting label Aug 29, 2023

kc13greiner added release_note:enhancement Feature:Security/Spaces Platform Security - Spaces feature labels Aug 29, 2023

kc13greiner approved these changes Aug 29, 2023

View reviewed changes

jeramysoucy requested changes Aug 29, 2023

View reviewed changes

change test label from API to routes

4cedff5

SiddharthMantri added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Aug 29, 2023

jeramysoucy reviewed Aug 29, 2023

View reviewed changes

x-pack/plugins/spaces/public/plugin.test.ts Outdated Show resolved Hide resolved

SiddharthMantri added 3 commits August 29, 2023 19:11

Update plugin.test.ts with correct test label

d07844e

Update plugin.test.ts

0f82e23

Merge branch 'main' into disable-space-selector-serverless

28c6bb6

SiddharthMantri requested a review from jeramysoucy August 29, 2023 17:19

jeramysoucy approved these changes Aug 29, 2023

View reviewed changes

SiddharthMantri requested a review from pgayvallet August 29, 2023 17:47

jeramysoucy added the v8.11.0 label Aug 29, 2023

TinaHeiligers reviewed Aug 29, 2023

View reviewed changes

TinaHeiligers self-requested a review August 29, 2023 19:41

TinaHeiligers approved these changes Aug 29, 2023

View reviewed changes

Merge branch 'main' into disable-space-selector-serverless

45655b1

pgayvallet approved these changes Aug 30, 2023

View reviewed changes

SiddharthMantri merged commit 0c3d38b into elastic:main Aug 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Space Selector from Serverless Top Navigation #164461

Remove Space Selector from Serverless Top Navigation #164461

SiddharthMantri commented Aug 22, 2023 •

edited

Loading

SiddharthMantri commented Aug 22, 2023

kc13greiner left a comment

kc13greiner Aug 25, 2023

SiddharthMantri Aug 28, 2023

kc13greiner Aug 28, 2023

SiddharthMantri Aug 29, 2023

thomheymann Aug 29, 2023

elasticmachine commented Aug 29, 2023

kc13greiner left a comment

jeramysoucy left a comment •

edited

Loading

jeramysoucy Aug 29, 2023

SiddharthMantri Aug 29, 2023

jeramysoucy Aug 29, 2023 •

edited

Loading

TinaHeiligers Aug 29, 2023 •

edited

Loading

TinaHeiligers Aug 29, 2023

TinaHeiligers left a comment

TinaHeiligers Aug 29, 2023

kibana-ci commented Aug 30, 2023

API count

Remove Space Selector from Serverless Top Navigation #164461

Remove Space Selector from Serverless Top Navigation #164461

Conversation

SiddharthMantri commented Aug 22, 2023 • edited Loading

Summary

Changes Made

UI Changes

Release Notes

SiddharthMantri commented Aug 22, 2023

kc13greiner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Aug 29, 2023

kc13greiner left a comment

Choose a reason for hiding this comment

jeramysoucy left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeramysoucy Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

TinaHeiligers Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Aug 30, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Page load bundle

API count

History

SiddharthMantri commented Aug 22, 2023 •

edited

Loading

jeramysoucy left a comment •

edited

Loading

jeramysoucy Aug 29, 2023 •

edited

Loading

TinaHeiligers Aug 29, 2023 •

edited

Loading