ls1intum / Artemis

Artemis - Interactive Learning with Automated Feedback
https://docs.artemis.cit.tum.de
MIT License
481 stars 288 forks source link

Tracking Discussion: Required steps for GitLab to LocalVC migration #8754

Open b-fein opened 3 months ago

b-fein commented 3 months ago

Is your feature request related to a problem?

Like the migration from BitBucket to LocalVC, a similar automatic migration should be possible for the migration from GitLab to LocalVC.

This issue serves as a collection/tracker to discuss required steps for this to be possible.

Relevant items that come to mind right now:

krusche commented 3 months ago

The migration from Bitbucket to LocalVC can be found in chore/bitbucket-localvc-migration-release. It might be helpful.

BaumiCoder commented 2 months ago

Why does the BitBucket to LocalVC Migration uses two extra keys for @Value? Both does not seem to be necessary for me:

  1. migration.local-vcs-repo-path could be replaced with artemis.version-control.local-vcs-repo-path, which already exists at the time of the BitBucket migration (e.g. used here).
  2. migration.base-url should be always equal to server.url. Or how can this be different?

@reschandreas can you maybe answer that questions, as the author of the BitBucket to LocalVC migration?

reschandreas commented 2 months ago

Hi! I intentionally used different values to ensure that migration requires a deliberate action, preventing it from happening accidentally during updates. This way, the Artemis admin must consciously decide and update their configuration to migrate.

b-fein commented 2 months ago

@BaumiCoder I guess with our plan to trigger the migration by a class with a special profile this then serves the same purpose and the different configuration values are not required.

BaumiCoder commented 2 months ago

Thanks @reschandreas, with this background information I agree with @b-fein. I moved to the existing values for the migration here (see 55836b787974b38a1b7cc354f96cbf0963bfb91d).

BaumiCoder commented 2 months ago

The log message here claims that the URL will not be changed in the database, because the migration failed. In contrast to this the lines 344 and 345 are also executed in this case, so the URL is saved, isn't? @reschandreas Is that intended or a bug?

reschandreas commented 2 months ago

looks like a bug to me. We changed the behaviour of the method several times (we weren't sure if we should keep the url or not), so this could have slipped through. I'm not aware of any issues during the migration though.

krusche commented 2 months ago

I think we had some entries in the database with the old URL. I always said this should not happen ;-) so it's probably a bug in the migration

BaumiCoder commented 2 months ago

Okay, I fixed that bug (13e0764). However, this bug would have led to an null URL in the database (if that is possible). An old URL is still possible if some error blocks the migration of a repository (e.g. the source repository is not available anymore).

BaumiCoder commented 2 months ago

Another GitLab specific thing is in the Artemis Database. If tokens are used for the authentication of the users against GitLab, Artemis stores these tokens in the column vcs_access_token of jhi_user. LocalVC also will have a token authentication (#8664). @SimonEntholzer Will your implementation be able to work with these tokens from GitLab, or should we delete the existing tokens (so that new ones are created if necessary)? I see two differences between the GitLab and the localVC tokens:

  1. Prefix: vcpat- at localVC / glpat- at GitLab
  2. Number of the characters after the prefix (digits and letters): actual 40 at localVC / 21 at GitLab (maybe depending on some GitLab configuration)
BaumiCoder commented 2 months ago

To make sure that the old tokens do not cause any trouble or became unnecessary data, the migration now removes the token data (da6b3b3).

FYI: @SimonEntholzer

dfuchss commented 2 months ago

I've also updated our Test Container (LocalVC + Jenkins): https://github.com/kit-sdq/Artemis-SAML2-Test-Docker/releases/tag/v7.4.2-lvc

So this environment is ready to be used for testing as well :)