V2 - Githubissues

Josue-T commented 8 months ago

Problem

401
412
424 (not the best way to fix it...)
376 (still be buggy)
422
416
406
Implement MSC1929 #434

Solution

Rework package with packaging v2 practice
Rework config panel with settings instead of sed directly in config file

TODO

[x] Test email with system user (need https://github.com/YunoHost/yunohost/pull/1780 to work correctly)
[x] Manage issues with LDAP user vs system user -> should write a patch to fix https://github.com/matrix-org/matrix-synapse-ldap3/issues/169 to be able to remove the synapse yunohost user. -> https://github.com/matrix-org/matrix-synapse-ldap3/pull/186
[x] Standardize systemd unit name
[x] Write test.toml
[x] Test if we still need the SSO and password authentication in config panel
[x] Test config panel (need https://github.com/YunoHost/yunohost/pull/1772 and https://github.com/YunoHost/yunohost/pull/1789 to work correctly and still have this issue https://github.com/YunoHost/issues/issues/2331)
[x] Manage upgrade from old version
[x] Update homeserver config template from upstream
[x] Test and fix issues
[ ] Check for bookworm support

PR Status

[x] Code finished and ready to be reviewed/tested
[x] The fix/enhancement were manually tested (if applicable)

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

Thatoo commented 8 months ago

I tried to install it but it has failed : https://paste.yunohost.org/raw/uwetisotec

Josue-T commented 8 months ago

I tried to install it but it has failed : https://paste.yunohost.org/raw/uwetisotec

Well, as it's written, it's a draft, so it's not finished. :wink:

Thatoo commented 7 months ago

[x] Install works now : happy v2!!!
[x] I could login : CAS works again
[ ] but when I tried to change some settings, I got this error :

2023-11-15 21:28:33,296: DEBUG - + ynh_die '--message=Variable $data_dir wasn'\''t initialized when trying to replace __DATA_DIR__ in /etc/matrix-synapse/homeserver.yaml'
2023-11-15 21:28:33,390: WARNING - Variable $data_dir wasn't initialized when trying to replace __DATA_DIR__ in /etc/matrix-synapse/homeserver.yaml

https://paste.yunohost.org/raw/asuwowopuj

csolisr commented 7 months ago

* [ ]  but when I tried to change some settings, I got this error :

2023-11-15 21:28:33,296: DEBUG - + ynh_die '--message=Variable $data_dir wasn'\''t initialized when trying to replace __DATA_DIR__ in /etc/matrix-synapse/homeserver.yaml'
2023-11-15 21:28:33,390: WARNING - Variable $data_dir wasn't initialized when trying to replace __DATA_DIR__ in /etc/matrix-synapse/homeserver.yaml

Similar issue here - and it's even more complicated now, because I can't even force upgrading to the latest version:

sudo yunohost app upgrade synapse -u https://github.com/YunoHost-Apps/synapse_ynh/tree/v2 -F
Info: Now upgrading synapse...
Info: Creating a safety backup prior to the upgrade
Info: Collecting files to be backed up for synapse...
Warning: It's hightly recommended to make your backup when the service is stopped. Please stop synapse service with this command before to run the backup 'systemctl stop matrix-synapse.service'
Info: Declaring files to be backed up...
Warning: /var/cache/yunohost/app_tmp_work_dirs/app_1sckfx0q/scripts/backup: line 52: synapse_db_name: variable unassigned
Error: Could not back up synapse
Info: The operation 'Create a backup archive' could not be completed. Please share the full log of this operation using the command 'yunohost log share 20231121-174305-backup_create' to get help
Error: Nothing to save

Is there some way of manually setting the synapse_db_name variable in YNH so I can upgrade?

Josue-T commented 7 months ago

Hello,

PLEASE, don't upgrade to this version. This PR is just for information but is NOT ready. There still be a lot of work before to release it !! It could eventually be tested but NOT on production.

csolisr commented 7 months ago

Hello,

PLEASE, don't upgrade to this version. This PR is just for information but is NOT ready. There still be a lot of work before to release it !! It could eventually be tested but NOT on production.

Yup, just noticed it the hard way. Guess I'll have to wipe and reinstall the stable version later.

csolisr commented 7 months ago

Oh and if you stubbornly tried this PR before it was baked, and now you're stuck on the update part like I did, you can try setting the uninitialized values by hand:

sudo yunohost app setting synapse synapse_db_name -v $(sudo yunohost app setting synapse db_name);
sudo yunohost app setting synapse data_path -v $(sudo yunohost app setting synapse data_dir);

orhtej2 commented 7 months ago

Thing to consider for bookworm: Debian 12 ships OpenSSL 3.x which renders this line invalid:

ynh_exec_warn_less openssl dhparam -out /etc/ssl/private/dh2048.pem -outform PEM -2 2048 -dsaparam

When moving bits to end of command it yields Error, generator may not be chosen for DSA parameters.

I suggest either getting rid of -dsaparam for slower but safer keys OR removing -2 to get rid of this incompatibility. WDYT?

Gredin67 commented 5 months ago

@YunoHost-Apps/matrix-bridges this is a call for contributions to this most important PR in the yunohost matrix stack ! I'm sure some of you could contribute to one or the other ToDos listed by @Josue-T in the description.

Salamandar commented 5 months ago

@YunoHost-Apps/matrix-bridges this is a call for contributions to this most important PR in the yunohost matrix stack ! I'm sure some of you could contribute to one or the other ToDos listed by @Josue-T in the description.

TBH this merge request is huge and should have been splitted in multiple merge requests. Regrouping multiple goals (packaging v2 + issue fixes) exponentially increases the difficulty to review and fix this merge request.

I'll try it though.

Salamandar commented 5 months ago

!testme

yunohost-bot commented 5 months ago

:v:

Thatoo commented 5 months ago

I'll wait for @Josue-T to answer to @Salamandar before testing install, upgrade and config panel.

Gredin67 commented 5 months ago

@YunoHost-Apps/matrix-bridges I've open a PR against @Josue-T v2 PR and given you access rights. In case any of you want to contribute https://github.com/Gredin67/synapse_ynh/tree/v2

I've started to integrate @Salamandar review. Maybe @thardev who's an experienced v2 packager could help here ?

Gredin67 commented 5 months ago

replacing the __FOOBAR__ keys in homeserver.yaml seem to fail after applying the config panel. (and maybe at install).

thardev commented 5 months ago

Hi @Gredin67 and everyone 🙂

I'll try to help here when I have some time.

Gredin67 commented 5 months ago

Did anyone realize that a Debian package is available for synapse from bookworm and bullseye-backports ? Does it make sense to use it here for single installations which is probably the main use-case ? https://matrix-org.github.io/synapse/latest/setup/installation.html#debianubuntu

lapineige commented 5 months ago

Well I don't know about that, but from until Yunohost is ready for bookworm it's not usable in this package.

Josue-T commented 4 months ago

Let's see what the CI say !testme !bookwormtestme

yunohost-bot commented 4 months ago

Living in the future, are we?

yunohost-bot commented 4 months ago

Alrighty!

Josue-T commented 4 months ago

!testme !bookwormtestme

yunohost-bot commented 4 months ago

:sunflower:

yunohost-bot commented 4 months ago

:book:

Josue-T commented 4 months ago

Following the CI on bookworm @alexAubin do you know why config panel are not validated ?

Look like linked to this part, but I don't really understand why on bullseye it works. Is it linked to some change planified on bookworm ? Will be the markdown text help support removed ?

...

[advanced]
name = "Advanced Settings"
services = ["matrix-__APP__"]

    [advanced.help]
    name = "SETTINGS FOR EXPERTS IN SERVER ADMINISTRATION"

        [advanced.help.text] # -> look like that this make crashing the CI on bookworm
        ask = '''
        !!There are security and privacy risks if you change these settings without knowing what you do!!
        '''
        type = "markdown"

    [advanced.others]
    name = "Others"

            [advanced.others.report_stats]
            ask = "Server statistics"
            type = "boolean"
            yes = "true"
            no = "false"
            help = "Enable to send anonymous statistics to Synapse Developers to improve performance."

    [advanced.guests]
    name = "Experience for Guests / Anonymous"

        [advanced.guests.invite_client_location]
        ask = "Web client location to direct users to during an invite"
        type = "url"
        help = "This is passed to the identity server as the org.matrix.web_client_location key. Defaults to unset, giving no guidance to the identity server."

...

Josue-T commented 4 months ago

!testme

yunohost-bot commented 4 months ago

:v: