Fixes #24146: Backport user table to Rudder 7.3 #5378
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fanf
force-pushed
the
arch_24146/backport_user_table_to_rudder_7_3
branch
from
9ddf8bc
to
3977f3b
Compare
fanf
changed the base branch from
branches/rudder/7.3
to
backports/7.3.12/24146
clarktsiory
reviewed
Feb 9, 2024
...pp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/users/UserRepository.scala
Outdated
Show resolved
Hide resolved
| import com.normation.rudder.users.UserSerialization._ | ||
| import doobie._ | ||
|
|
||
| implicit val logger = Doobie.slf4jDoobieLogger |
There was a problem hiding this comment.
It's not in the 8.1 branch, if it's really really needed here the type should be explicit !
There was a problem hiding this comment.
no, that one is not important: what we want to have is a nice upmerge experience. For that, we want to limit changes between branches, and so want want to have the same changes on all branches (for ex adding types).
But here, since the code was removed in upper branch, adding the type would likely cause a merge conflict, while letting it unchanged is ok (the deletion was already merged).
...ources/rudder/rudder-core/src/test/scala/com/normation/rudder/users/UserRepositoryTest.scala
Outdated
Show resolved
Hide resolved
...ources/rudder/rudder-core/src/test/scala/com/normation/rudder/users/UserRepositoryTest.scala
Outdated
Show resolved
Hide resolved
...ources/rudder/rudder-core/src/test/scala/com/normation/rudder/users/UserRepositoryTest.scala
Outdated
Show resolved
Hide resolved
clarktsiory
reviewed
Feb 9, 2024
...es/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckTableUsers.scala
Outdated
Show resolved
Hide resolved
Comment on lines
47
to
85
| /* | ||
| * During 7.3 cycle, we added the registration of users and their sessions in base. | ||
| * This is to allows better security logs on user sessions + allows to de-correlate rudder | ||
| * users from the `rudder-user.xml` file. | ||
| */ | ||
| class CheckTableUsers( | ||
| doobie: Doobie | ||
| ) extends BootstrapChecks { | ||
|
|
||
| import doobie._ | ||
|
|
||
| override def description: String = "Check if database tables Users and UserSessions exist" | ||
|
|
||
| def createUserTables: IOResult[Unit] = { | ||
| val sql1 = sql"""CREATE TABLE IF NOT EXISTS Users ( | ||
| id text PRIMARY KEY NOT NULL CHECK (id <> '') | ||
| , creationDate timestamp with time zone NOT NULL | ||
| , status text NOT NULL | ||
| , managedBy text NOT NULL CHECK (managedBy <> '') | ||
| , name text | ||
| , email text | ||
| , lastLogin timestamp with time zone | ||
| , statusHistory jsonb | ||
| , otherInfo jsonb -- general additional user info | ||
| );""" | ||
|
|
||
| val sql2 = sql"""CREATE TABLE IF NOT EXISTS UserSessions ( | ||
| userId text NOT NULL CHECK (userId <> '') | ||
| , sessionId text NOT NULL CHECK (sessionId <> '') | ||
| , creationDate timestamp with time zone NOT NULL | ||
| , authMethod text | ||
| , permissions text[] | ||
| , endDate timestamp with time zone | ||
| , endCause text | ||
| );""" | ||
|
|
||
| transactIOResult(s"Error with 'Users' table creation")(xa => sql1.update.run.transact(xa)).unit *> | ||
| transactIOResult(s"Error with 'v' table creation")(xa => sql2.update.run.transact(xa)).unit | ||
| } |
There was a problem hiding this comment.
Some database compatibility issues that may arise :
- The users is version 7.3 will have the tables created, and when upgrading to 8.1 they will need to add a column
tenantsto theUserSessionstable : we should create a migration for that - I imagine there will be no user rolling back from version 8.1 to 7.3, but for developement I have an error reading user sessions from the table when the webapp starts (see the fix I suggested in
UserRepositoryfile)
There was a problem hiding this comment.
yes, we will need an additionnal migration to add the tenants column
...pp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/users/UserRepository.scala
Outdated
Show resolved
Hide resolved
clarktsiory
reviewed
Feb 12, 2024
| @@ -991,6 +979,44 @@ object RudderParsedProperties { | |||
| } | |||
| } | |||
|
|
|||
| // user clean-up | |||
| val RUDDER_USERS_CLEAN_CRON = ( | |||
There was a problem hiding this comment.
Suggested change
| val RUDDER_USERS_CLEAN_CRON = ( | |
| val RUDDER_USERS_CLEAN_CRON: Option[CronExpr] = ( |
fanf
force-pushed
the
arch_24146/backport_user_table_to_rudder_7_3
branch
from
745474c
to
4d4a1a8
Compare
|
The failing tests have nothing to do with the code (they will be investigated appart). Merging by hand because it targets a branch unknown of our bot. |
fanf
merged commit d0fd857
into
Normation:backports/7.3.12/24146
11 of 15 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://issues.rudder.io/issues/24146
Backport of #5121 for branche 7.3.