Allow the default space to be accessed via /s/default

[legrego]

Summary

Allows the default space to be accessed via /s/default as well as /.

Resolves #62053

[legrego]

I don't love that this function now has two responsibilities, but this function is called in some hot paths, and I didn't want to have to do regex matching twice when I'm already doing the work here to extract the space identifier.

[legrego]

No functional changes: extracted for readability

[legrego]

This is the actual change which enables the default space to be accessed via /s/default

[elasticmachine]

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

[jportner]

LGTM!

It might be worthwhile to simplify this reporting code as well (CC @tsullivan):

kibana/x-pack/plugins/reporting/server/core.ts

Lines 223 to 226 in d10d70b

	if (spaceId && spaceId !== DEFAULT_SPACE_ID) {
	logger.info(`Generating request for space: ${spaceId}`);
	this.getPluginSetupDeps().basePath.set(fakeRequest, `/s/${spaceId}`);
	}

[jportner]

Just from naively reading the tests, it's not clear that the default space can be used as an explicit space identifier.

Optional nit: what do you think about adding some additional test cases to make that more clear?

diff --git a/x-pack/plugins/spaces/common/lib/spaces_url_parser.test.ts b/x-pack/plugins/spaces/common/lib/spaces_url_parser.test.ts
index d563a0c86cd..c4379574f85 100644
--- a/x-pack/plugins/spaces/common/lib/spaces_url_parser.test.ts
+++ b/x-pack/plugins/spaces/common/lib/spaces_url_parser.test.ts
@@ -16,6 +16,14 @@ describe('getSpaceIdFromPath', () => {
       });
     });
 
+    test('it identifies the space url context with the default space', () => {
+      const basePath = `/s/${DEFAULT_SPACE_ID}`;
+      expect(getSpaceIdFromPath(basePath)).toEqual({
+        spaceId: DEFAULT_SPACE_ID,
+        pathHasExplicitSpaceIdentifier: true,
+      });
+    });
+
     test('ignores space identifiers in the middle of the path', () => {
       const basePath = `/this/is/a/crazy/path/s/my-awesome-space-lives-here`;
       expect(getSpaceIdFromPath(basePath)).toEqual({
@@ -42,6 +50,14 @@ describe('getSpaceIdFromPath', () => {
       });
     });
 
+    test('it identifies the space url context with the default space', () => {
+      const basePath = `/s/${DEFAULT_SPACE_ID}`;
+      expect(getSpaceIdFromPath(basePath)).toEqual({
+        spaceId: DEFAULT_SPACE_ID,
+        pathHasExplicitSpaceIdentifier: true,
+      });
+    });
+
     test('it identifies the space url context following the server base path', () => {
       const basePath = `/server-base-path-here/s/my-awesome-space-lives-here`;
       expect(getSpaceIdFromPath(basePath, '/server-base-path-here')).toEqual({
@@ -50,6 +66,14 @@ describe('getSpaceIdFromPath', () => {
       });
     });
 
+    test('it identifies the space url context with the default space following the server base path', () => {
+      const basePath = `/server-base-path-here/s/${DEFAULT_SPACE_ID}`;
+      expect(getSpaceIdFromPath(basePath, '/server-base-path-here')).toEqual({
+        spaceId: DEFAULT_SPACE_ID,
+        pathHasExplicitSpaceIdentifier: true,
+      });
+    });
+
     test('ignores space identifiers in the middle of the path', () => {
       const basePath = `/this/is/a/crazy/path/s/my-awesome-space-lives-here`;
       expect(getSpaceIdFromPath(basePath, '/this/is/a')).toEqual({

[legrego]

Thanks for the suggestion! Done in 2084f12 (#77109)

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 5ff8450

Metrics [docs]

page load bundle size

id	before	after	diff
spaces	364.2KB	364.4KB	+246.0B

History

• 💚 Build #76246 succeeded 64786c2
• 💔 Build #76188 failed d4a89e5
• 💔 Build #74897 failed cd783c1
• 💚 Build #73523 succeeded c020f07

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