feat: Removing dependency on usr, accout, profile db entries #398

ibuziuk · 2022-12-06T10:29:24Z

NOTE: PR should be merged only once operands like dashboard do not fall back on retrieving any information from db e.g. workspace API dependency eclipse-che/che#21846

What does this PR do?

Removing dependency on usr, accout, profile db entries. Data is not persisted once user first time access Eclipse Che meaning that after the update db is no longer used as the source of valid data
Update the way 'profile-secret' is generated (no data for the secret is obtained from the db anymore) NOTE that the secret still contains a dummy email postfixed with @che which is done for backward compatibility with UD that still reads 3 properties from the secret (id / name / email)
Removes 'user-preferences' secret that is not relevant in the context of DevWorskpaces anymore

kind: Secret
apiVersion: v1
metadata:
  name: user-preferences
  namespace: ibuziuk-dev
  uid: ca6af2f3-ed72-45b9-ade0-c8162f506d2d
  resourceVersion: '1987202263'
  creationTimestamp: '2022-12-06T09:43:59Z'
  labels:
    controller.devfile.io/mount-to-devworkspace: 'true'
  annotations:
    controller.devfile.io/mount-as: file
    controller.devfile.io/mount-path: /config/user/preferences
  managedFields:
    - manager: okhttp
      operation: Update
      apiVersion: v1
      time: '2022-12-06T09:43:59Z'
      fieldsType: FieldsV1
      fieldsV1:
        'f:data':
          .: {}
          'f:codenvy-created': {}
          'f:infrastructureNamespace': {}
          'f:infrastructureNamespaceTemplate': {}
          'f:temporary': {}
        'f:metadata':
          'f:annotations':
            .: {}
            'f:controller.devfile.io/mount-as': {}
            'f:controller.devfile.io/mount-path': {}
          'f:labels':
            .: {}
            'f:controller.devfile.io/mount-to-devworkspace': {}
        'f:type': {}
data:
  codenvy-created: MTY3MDMxOTgzOTQ0Ng==
  infrastructureNamespace: aWJ1eml1ay1kZXY=
  infrastructureNamespaceTemplate: PHVzZXJuYW1lPi1kZXY=
  temporary: ZmFsc2U=
type: Opaque

Issue Refernce

Part of eclipse-che/che#21374

How to test this PR?

Image - quay.io/ibuziuk/che-server:pr-398

Deploy vanilla Eclipse Che or Dev Spaces on a cluster (RHPDS small 4.10 cluster with 5 pre-created users is recommended)
Log in as regular user (user1 on RHPDS) and start a few workspaces
Update CR to reference the che-server image from PR

spec:
  components:
    cheServer:
      debug: true
      deployment:
        containers:
          - image: 'quay.io/ibuziuk/che-server:pr-398'
            name: che

Log in as a new user (user2 on RHPDS) and start a few workspaces
Logout and log in again as user1 - check that old and new workspaces are started correctly
Check the database, only entries for the first user (user1 on RHPDS) persisted, and no entries for the second user (user2 on RHPDS)

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

skabashnyuk

In general looks good. Some small nitpicks in the comments.

skabashnyuk · 2022-12-09T07:16:34Z

...rg/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java

   * @return true if the namespace can be created, false if the namespace is expected to already
   *     exist
   * @throws InfrastructureException on failure
   */
-  protected boolean canCreateNamespace(RuntimeIdentity identity) throws InfrastructureException {
+  protected boolean canCreateNamespace(RuntimeIdentity identity, String userName)


When I look at canCreateNamespace and getOrCreate I would consider

Add username to both methods

Or add username/ownerName to RuntimeIdentity
Since RuntimeIdentity has already ownerId I think second option is better

I also thought about that, but this would require quite a few changes in the obsolete classes like PluginBrokerManager which are planned to be removed completely in Phase 2 of the db removal procedure. The goal of this PR is to make all the stored data in Postgres irrelevant. In the future PR, major cleanup / refactoring is going to happen when the whole DAO layer will be removed completely (currently targeted for 7.61)

skabashnyuk · 2022-12-09T07:17:40Z

...rg/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java

@@ -320,14 +309,14 @@ public KubernetesNamespace getOrCreate(RuntimeIdentity identity) throws Infrastr
    KubernetesNamespace namespace = get(identity);

    var subject = EnvironmentContext.getCurrent().getSubject();
+    var userName = subject.getUserName();


See my comment to canCreateNamespace method

skabashnyuk · 2022-12-09T07:19:02Z

.../workspace/infrastructure/kubernetes/namespace/configurator/UserPreferencesConfigurator.java

-        .endMetadata()
-        .build();
-  }
+      throws InfrastructureException {}


consider adding some descriptive message to the exception.

skabashnyuk · 2022-12-09T07:20:53Z

...java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java

@@ -106,14 +106,14 @@ public OpenShiftProject getOrCreate(RuntimeIdentity identity) throws Infrastruct
    OpenShiftProject osProject = get(identity);

    var subject = EnvironmentContext.getCurrent().getSubject();
+    var userName = subject.getUserName();


see my comment to canCreateNamespace method

skabashnyuk · 2022-12-09T07:22:43Z

wsmaster/che-core-api-account/src/main/java/org/eclipse/che/account/api/AccountManager.java

@@ -51,7 +52,6 @@ public AccountManager(AccountDao accountDao, EventService eventService) {
   */
  public void create(Account account) throws ConflictException, ServerException {
    requireNonNull(account, "Required non-null account");
-    accountDao.create(new AccountImpl(account));


Since you don't expect that someone will call this method. What about throwing an exception?

throw new ServerException("account creation are not expected to be invoked")

Something like that.

ibuziuk · 2023-01-11T14:06:43Z

eclipse-che/che#21846 has been resolved
plan to merge the PR tomorrow

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

devstudio-release · 2023-01-15T08:41:35Z

Build 3.5 :: server_3.x/105: Console, Changes, Git Data

devstudio-release · 2023-01-15T08:51:24Z

Build 3.5 :: sync-to-downstream_3.x/1998: Console, Changes, Git Data

devstudio-release · 2023-01-15T08:54:54Z

Build 3.5 :: get-sources-rhpkg-container-build_3.x/1919: Console, Changes, Git Data

devstudio-release · 2023-01-15T09:00:06Z

Build 3.5 :: get-sources-rhpkg-container-build_3.x/1919:

server : 3.x :: Failed in 50072228 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

ibuziuk changed the title ~~feat: Removing dependency on usr, accout, profile db entries~~ [DO NOT MERGE] feat: Removing dependency on usr, accout, profile db entries Dec 6, 2022

ibuziuk force-pushed the usr-account-profile-deprecation branch 2 times, most recently from 582fee5 to e6ef88f Compare December 7, 2022 12:35

ibuziuk requested a review from skabashnyuk December 8, 2022 09:54

ibuziuk force-pushed the usr-account-profile-deprecation branch 2 times, most recently from 17e050a to 7b0e802 Compare December 8, 2022 16:38

ibuziuk marked this pull request as ready for review December 8, 2022 16:38

ibuziuk requested review from nickboldt, vinokurig, mshaposhnik and amisevsk as code owners December 8, 2022 16:38

skabashnyuk approved these changes Dec 9, 2022

View reviewed changes

ibuziuk force-pushed the usr-account-profile-deprecation branch from 7b0e802 to 4a03f9a Compare December 9, 2022 10:29

This was referenced Dec 12, 2022

[Dashboard] Remove dependency on Workspace API provided by che-server eclipse-che/che#21846

Closed

Decommissioning PostgreSQL database eclipse-che/che#21374

Closed

Update default Postgresql version eclipse-che/che#21881

Closed

olexii4 approved these changes Dec 21, 2022

View reviewed changes

amisevsk approved these changes Dec 21, 2022

View reviewed changes

ibuziuk force-pushed the usr-account-profile-deprecation branch from 4a03f9a to 696b097 Compare January 9, 2023 14:14

ibuziuk changed the title ~~[DO NOT MERGE] feat: Removing dependency on usr, accout, profile db entries~~ feat: Removing dependency on usr, accout, profile db entries Jan 11, 2023

ibuziuk added 2 commits January 12, 2023 13:58

feat: Removing dependency on the usr, account, profile db tables

16d7377

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

fixup! adressing review comments

05fe6f5

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

ibuziuk force-pushed the usr-account-profile-deprecation branch from 696b097 to 05fe6f5 Compare January 12, 2023 12:59

ibuziuk merged commit db249bf into eclipse-che:main Jan 12, 2023

ibuziuk mentioned this pull request Jan 12, 2023

Sprint 230 eclipse-che/che#21925

Closed

60 tasks

che-bot added this to the 7.60 milestone Jan 12, 2023

This was referenced Jan 30, 2023

Error "backend not available" when user logs-in for the first time (namespace provisioning returns error 500) eclipse-che/che#21958

Closed

fix: Simplification of the 'canCreateNamespace' logic #431

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Removing dependency on usr, accout, profile db entries #398

feat: Removing dependency on usr, accout, profile db entries #398

ibuziuk commented Dec 6, 2022 •

edited

skabashnyuk left a comment

skabashnyuk Dec 9, 2022

ibuziuk Dec 9, 2022

skabashnyuk Dec 9, 2022

skabashnyuk Dec 9, 2022

skabashnyuk Dec 9, 2022

skabashnyuk Dec 9, 2022

ibuziuk Dec 9, 2022

ibuziuk commented Jan 11, 2023

devstudio-release commented Jan 15, 2023

devstudio-release commented Jan 15, 2023

devstudio-release commented Jan 15, 2023

devstudio-release commented Jan 15, 2023

feat: Removing dependency on usr, accout, profile db entries #398

feat: Removing dependency on usr, accout, profile db entries #398

Conversation

ibuziuk commented Dec 6, 2022 • edited

What does this PR do?

Issue Refernce

How to test this PR?

PR Checklist

Reviewers

skabashnyuk left a comment

Choose a reason for hiding this comment

skabashnyuk Dec 9, 2022

Choose a reason for hiding this comment

ibuziuk Dec 9, 2022

Choose a reason for hiding this comment

skabashnyuk Dec 9, 2022

Choose a reason for hiding this comment

skabashnyuk Dec 9, 2022

Choose a reason for hiding this comment

skabashnyuk Dec 9, 2022

Choose a reason for hiding this comment

skabashnyuk Dec 9, 2022

Choose a reason for hiding this comment

ibuziuk Dec 9, 2022

Choose a reason for hiding this comment

ibuziuk commented Jan 11, 2023

devstudio-release commented Jan 15, 2023

devstudio-release commented Jan 15, 2023

devstudio-release commented Jan 15, 2023

devstudio-release commented Jan 15, 2023

ibuziuk commented Dec 6, 2022 •

edited