Feat: setup reserved workspaces when calling list workspaces API #178

SuZhou-Joe · 2023-09-18T10:27:10Z

Description

Issues Resolved

Screenshot

Testing the changes

Check List

codecov-commenter · 2023-09-18T10:45:12Z

Codecov Report

Merging #178 (ec7c21d) into workspace (e296a5d) will increase coverage by 0.48%.
Report is 1 commits behind head on workspace.
The diff coverage is 100.00%.

@@              Coverage Diff              @@
##           workspace     #178      +/-   ##
=============================================
+ Coverage      61.39%   61.87%   +0.48%     
=============================================
  Files           2982     2117     -865     
  Lines          57792    40889   -16903     
  Branches        9360     7470    -1890     
=============================================
- Hits           35481    25300   -10181     
+ Misses         20224    13897    -6327     
+ Partials        2087     1692     -395

Flag	Coverage Δ
Linux_1	`?`
Linux_2	`55.05% <100.00%> (+<0.01%)`	⬆️
Linux_3	`44.18% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/core/server/index.ts	`100.00% <ø> (ø)`
src/core/utils/constants.ts	`100.00% <100.00%> (ø)`

... and 937 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

yuye-aws · 2023-09-19T02:44:29Z

src/plugins/workspace/server/workspace_client.ts

@@ -125,6 +227,52 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
          type: WORKSPACE_TYPE,
        }
      );
+      const scopedClientWithoutPermissionCheck = this.getScopeClientWithoutPermisson(requestDetail);


There has been a PR fixing typos, please change the function name to getScopedClientWithoutPermission.

Fixed and thanks

yuye-aws · 2023-09-19T02:49:11Z

src/plugins/workspace/server/workspace_client.ts

+    const managementWorkspaceACL = new ACL().addPermission([WorkspacePermissionMode.Management], {
+      users: ['*'],
+    });


For management workspace, are we going to grant WorkspacePermissionMode.Management to all users?

Yes, the conclusion is that we will make public and management workspace manageable by anyone, and the first one can change the permission if they want to limit the permission.

yuye-aws · 2023-09-19T02:49:33Z

src/plugins/workspace/server/workspace_client.ts

+    const publicWorkspaceACL = new ACL().addPermission([WorkspacePermissionMode.Management], {
+      users: ['*'],
+    });


For public workspace, are we going to grant WorkspacePermissionMode.Management to all users?

Yes, the conclusion is that we will make public and management workspace manageable by anyone, and the first one can change the permission if they want to limit the permission.

yuye-aws · 2023-09-19T02:57:29Z

I found that reserved workspaces were not automatically created from the left menu header. When workspace feature is enabled, the header obtain the workspace list like the following:

src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx
const workspaceList = useObservable(observables.workspaceList$, []);

Instead of simply get workspaceList$ observable, we need to call workspace _list to make this PR effective.

SuZhou-Joe · 2023-09-19T04:33:01Z

I found that reserved workspaces were not automatically created from the left menu header. When workspace feature is enabled, the header obtain the workspace list like the following:
src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx
const workspaceList = useObservable(observables.workspaceList$, []);
Instead of simply get workspaceList$ observable, we need to call workspace _list to make this PR effective.

There should be a place to call the workspace _list or the workspaceList$ will always be empty.

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

ruanyl · 2023-09-19T03:30:19Z

src/plugins/workspace/server/permission_control/client.ts

-  }
-
-  public getPrincipalsFromRequest(request: OpenSearchDashboardsRequest): Principals {
+  static getPrincipalsFromRequest(request: OpenSearchDashboardsRequest): Principals {


Nit: I might just move this out of the class and make it a pure util function

Agree, changed that.

ruanyl · 2023-09-19T04:49:43Z

src/plugins/workspace/server/workspace_client.ts

+      PUBLIC_WORKSPACE_ID,
+      {
+        name: i18n.translate('workspaces.public.workspace.default.name', {
+          defaultMessage: 'public',


Would be nice to tweak a bit

Suggested change

defaultMessage: 'public',

defaultMessage: 'Public',

Thanks for this catch. According to mockup, it should be named "Global workspace", changed to that.

ruanyl · 2023-09-19T04:56:22Z

src/plugins/workspace/server/workspace_client.ts

+      /**
+       * Setup public workspace if public workspace can not be found
+       */
+      const hasPublicWorkspace = savedObjects.find((item) => item.id === PUBLIC_WORKSPACE_ID);


The name hasPublicWorkspace indicates a boolean values, can we change find -> some?

Sure, changed that.

ruanyl · 2023-09-19T05:03:03Z

src/plugins/workspace/server/workspace_client.ts

+       */
+      if (principals.users) {
+        const hasPersonalWorkspace = savedObjects.find((item) =>
+          principals.users?.includes(item.id)


Can you leave a comment and say something like: user id will be personal workspace id

Sure, done.

yuye-aws · 2023-09-19T05:23:52Z

src/plugins/workspace/server/workspace_client.ts

@@ -125,6 +227,52 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
          type: WORKSPACE_TYPE,
        }
      );
+      const scopedClientWithoutPermissionCheck = this.getScopedClientWithoutPermission(requestDetail);


Please fix lint

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

raintygao · 2023-09-19T06:16:36Z

Will permission related changes in this PR be conflicted with this task? https://github.com/orgs/opensearch-project/projects/127/views/5?sortedBy%5Bdirection%5D=asc&sortedBy%5BcolumnId%5D=Assignees&pane=issue&itemId=38846476

SuZhou-Joe · 2023-09-19T09:00:26Z

Will permission related changes in this PR be conflicted with this task? https://github.com/orgs/opensearch-project/projects/127/views/5?sortedBy%5Bdirection%5D=asc&sortedBy%5BcolumnId%5D=Assignees&pane=issue&itemId=38846476

Let me change the setup permission to comply with the new permission model.

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* feat: setup reserved workspaces when calling list workspaces API Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: fix test flow Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: fix lint Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: fix typos Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: setup personal workspace using correct id Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * refractor: move function getPrincipalsFromRequest to a util function Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: change code according to comment Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: modify reserved workspace permission Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com> (cherry picked from commit 4bccdea) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

… (#183) * feat: setup reserved workspaces when calling list workspaces API * feat: fix test flow * feat: fix lint * feat: fix typos * fix: setup personal workspace using correct id * refractor: move function getPrincipalsFromRequest to a util function * feat: change code according to comment * feat: modify reserved workspace permission --------- (cherry picked from commit 4bccdea) Signed-off-by: SuZhou-Joe <suzhou@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

github-actions bot added the all-star-contributor label Sep 18, 2023

feat: setup reserved workspaces when calling list workspaces API

2d913a2

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

SuZhou-Joe force-pushed the feature/workspace-setup branch from b2e16b5 to 2d913a2 Compare September 18, 2023 23:34

SuZhou-Joe added 2 commits September 19, 2023 07:43

feat: fix test flow

8f99b8c

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

feat: fix lint

eed6e6d

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

yuye-aws reviewed Sep 19, 2023

View reviewed changes

feat: fix typos

feefd27

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

ruanyl reviewed Sep 19, 2023

View reviewed changes

yuye-aws reviewed Sep 19, 2023

View reviewed changes

SuZhou-Joe added 3 commits September 19, 2023 13:36

fix: setup personal workspace using correct id

1f85495

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

refractor: move function getPrincipalsFromRequest to a util function

5cbd863

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

feat: change code according to comment

ec7c21d

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

feat: modify reserved workspace permission

c19d904

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

ruanyl approved these changes Sep 20, 2023

View reviewed changes

yuye-aws approved these changes Sep 20, 2023

View reviewed changes

raintygao approved these changes Sep 20, 2023

View reviewed changes

SuZhou-Joe added the backport workspace-2.9 label Sep 20, 2023

SuZhou-Joe merged commit 4bccdea into ruanyl:workspace Sep 20, 2023
21 of 23 checks passed

opensearch-workspace-development bot mentioned this pull request Sep 20, 2023

[Backport workspace-2.9] Feat: setup reserved workspaces when calling list workspaces API #183

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: setup reserved workspaces when calling list workspaces API #178

Feat: setup reserved workspaces when calling list workspaces API #178

SuZhou-Joe commented Sep 18, 2023 •

edited

Loading

codecov-commenter commented Sep 18, 2023 •

edited

Loading

yuye-aws Sep 19, 2023

SuZhou-Joe Sep 19, 2023

yuye-aws Sep 19, 2023

SuZhou-Joe Sep 19, 2023

yuye-aws Sep 19, 2023

SuZhou-Joe Sep 19, 2023

yuye-aws commented Sep 19, 2023

SuZhou-Joe commented Sep 19, 2023

ruanyl Sep 19, 2023

SuZhou-Joe Sep 19, 2023

ruanyl Sep 19, 2023

SuZhou-Joe Sep 19, 2023

ruanyl Sep 19, 2023

SuZhou-Joe Sep 19, 2023

ruanyl Sep 19, 2023 •

edited

Loading

SuZhou-Joe Sep 19, 2023

yuye-aws Sep 19, 2023

raintygao commented Sep 19, 2023

SuZhou-Joe commented Sep 19, 2023

Feat: setup reserved workspaces when calling list workspaces API #178

Feat: setup reserved workspaces when calling list workspaces API #178

Conversation

SuZhou-Joe commented Sep 18, 2023 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Check List

codecov-commenter commented Sep 18, 2023 • edited Loading

Codecov Report

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

yuye-aws commented Sep 19, 2023

SuZhou-Joe commented Sep 19, 2023

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

ruanyl Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raintygao commented Sep 19, 2023

SuZhou-Joe commented Sep 19, 2023

SuZhou-Joe commented Sep 18, 2023 •

edited

Loading

codecov-commenter commented Sep 18, 2023 •

edited

Loading

ruanyl Sep 19, 2023 •

edited

Loading