Add "move to a new domain" + fixes #28

Gofannon · 2018-05-02T10:52:42Z

Problem

Baikal cannot be moved to a new domain + small fixes

Solution

add change_url script + adapt check_process
add check on the new domain to see if path /.well-known/carddav is already served by other application (like Nextcloud in my case) See Check if the caldav/cardav uri are available on the server before installing #17 and Handle .well-known in case of conflict #25

Solution not perfect !
This is not perfect as for my use case, baikal does not work anymore after moving the app.

on the new domain, nextcloud is already installed and provided the /.well-known/ path.
while moving baikal, it detects that the /.well-known is not free and do not serve it
baikal does no do anything anymore and nextcloud is expected to do all the work
user is not warned about this and cannot know it

Some possible enhancements

instead of doing the migration, it might be usuful to deny it in the first place. (See Check if the caldav/cardav uri are available on the server before installing #17)
move baikal or nextcloud to a sub domain in order to avoid this conflict (I will do this to get back baikal running)

Edit 1

PR modified to detect if Nextcloud is already installed and abort the installation with an "user friendly" message (does not work at the moment and help needed - See Add "move to a new domain" + fixes #28 (comment) )

PR Status

Obviously, you should really check these affirmations
Work finished. Package_linter & Package_check, basic tests and upgrade from last version OK.
Upgrade tested on a VM and allowed to realise the conflict with nextcloud (see #25)
Could be reviewed and tested.

Validation

Minor decision

Upgrade previous version :
Code review :
Approval (LGTM) :
Approval (LGTM) :
CI succeeded :
When the PR is mark as ready to merge, you have to wait for 3 days before really merge it.

Testing

Fix fake patch...

…as Nextcloud)

Gofannon · 2018-09-03T17:15:01Z

Possible to review this forgotten PR @YunoHost-Apps/apps-group ?

It might not me merge right now because of conflit for handling $DOMAIN/.well-known/carddav (See PR for details)

maniackcrudelis · 2018-09-05T18:09:44Z

sources/extra_files/app/Core/Frameworks/Baikal/Core/AbstractExternalAuth.php

@@ -0,0 +1,130 @@
+<?php


All the patch modifications are from an older PR, #24, this part can be ignored for this PR.

I am not sure I get it so to be sure, shall I delete these files then ?

No, it's only that you took master instead of testing to work on this PR. So, there's so some commit from a previous PR.
I was just saying that we can ignore that part for this PR.

maniackcrudelis · 2018-09-05T18:13:29Z

scripts/upgrade

@@ -64,6 +64,20 @@ ynh_setup_source "$final_path"
 # NGINX CONFIGURATION
 #=================================================

+# Delete current nginx configuration to be able to check if .well-known is already served.
+ynh_remove_nginx_config
+ynh_app_setting_delete $app "checksum__etc_nginx_conf.d_$domain.d_$app.conf" || true


Here, could you please check if the config has been modified by the user before deleting it.
https://github.com/YunoHost/yunohost/blob/unstable/data/helpers.d/filesystem#L285

I took the code from here https://github.com/YunoHost-Apps/nextcloud_ynh/blob/master/scripts/upgrade#L109-L116 and do understand what and why it is needed

I don't know myself, but it would be good to use the helper ynh_backup_if_checksum_is_different file, in case the admin changed its config file.

maniackcrudelis · 2018-09-05T18:16:37Z

scripts/upgrade

+# Handle root path, avoid double slash.
+# Temporary fix, in waiting for an upgrade of the helper. (#361)
+#path_url_slash_less=${path_url%/}
+#ynh_replace_string "__PATH__/" "$path_url_slash_less/" "../conf/nginx.conf"


Why are you adding that ?
This PR, #361, has been merged on September 2017

My bad /o, will correct that later

maniackcrudelis · 2018-09-05T18:18:28Z

scripts/install

@@ -70,6 +70,20 @@ ynh_setup_source "$final_path"
 # NGINX CONFIGURATION
 #=================================================

+# Do not serve .well-known if it's already served on the domain
+if https://github.com/YunoHost-Apps/nextcloud_ynh/blob/962cc61ec8ffb045d89dd460cf8b55778d862ffc/scripts/install "https://${domain}/.well-known/caldav" ; then


I guess you would write is_url_handled ?

Well yes... copy pasta

maniackcrudelis · 2018-09-05T18:19:59Z

scripts/change_url

+  # Replace path in "return" directives
+  ynh_replace_string "return \([[:digit:]]\{3\}\) $old_path" "return \1 $new_path"  "$nginx_conf_path"
+  # Calculate and store the nginx config file checksum
+  ynh_store_file_checksum "$nginx_conf_path"


Please use the new way to handle that part.
https://github.com/YunoHost/example_ynh/blob/master/scripts/change_url#L69-L78

Done (did not tried yet)

maniackcrudelis · 2018-09-05T18:23:05Z

instead of doing the migration, it might be usuful to deny it in the first place. (See #17)

Sounds like the best thing to do.
Trig an error and explain why it can't be moved to another domain or why it can't be installed.

Gofannon · 2018-09-13T10:51:56Z

It was my first code PR, I will rework on it with new helpers and my experience when I will have time.

Thanks for the review and advices

Gofannon · 2018-11-02T20:22:16Z

Sounds like the best thing to do.
Trig an error and explain why it can't be moved to another domain or why it can't be installed.

In the last commit, I tried to use is_url_handled to verify if Nextcloud is already installed and then exits with an error message. I cannot make it run :/ package_check fails and Baikal cannot be installed. Am i using the helper in a wrong way ?

I do not understand how the helper runs so I created a is_url_handled_custom to dig but it is still unclear. So I committed anyway to be able to share the code and discuss. I might have missed something
helper code

cat package_check/Complete.log
[...]
2018-11-02 20:46:35,690 INFO     yunohost.hook <lambda> - [991.1] + echo 'je suis devant le if'
2018-11-02 20:46:35,690 INFO     yunohost.hook <lambda> - [991.1] je suis devant le if
2018-11-02 20:46:35,690 INFO     yunohost.hook <lambda> - [991.1] + is_url_handled_custom https://sous.domain.tld/.well-known/caldav
2018-11-02 20:46:35,690 INFO     yunohost.hook <lambda> - [991.1] + output=($(curl -k -s -o /dev/null       -w 'x%{redirect_url} %{http_code}' "$1"))
2018-11-02 20:46:35,690 INFO     yunohost.hook <lambda> - [991.1] ++ curl -k -s -o /dev/null -w 'x%{redirect_url} %{http_code}' https://sous.domain.tld/.well-known/caldav
2018-11-02 20:48:46,298 INFO     yunohost.hook <lambda> - [991.1] + local output
2018-11-02 20:48:46,299 INFO     yunohost.hook <lambda> - [991.1] + echo ¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤
2018-11-02 20:48:46,299 INFO     yunohost.hook <lambda> - [991.1] ¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤
2018-11-02 20:48:46,299 INFO     yunohost.hook <lambda> - [991.1] + echo 'contenu variable '\''output'\'' : x'
2018-11-02 20:48:46,300 INFO     yunohost.hook <lambda> - [991.1] contenu variable 'output' : x
2018-11-02 20:48:46,300 INFO     yunohost.hook <lambda> - [991.1] + echo ¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤
2018-11-02 20:48:46,300 INFO     yunohost.hook <lambda> - [991.1] ¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤¤
2018-11-02 20:48:46,300 WARNING  yunohost.hook <lambda> - [991.1] Baikal cannot be installed because of a conflit with another application, Nextcloud usually. This app handles the location where you want to install Baikal (https://sous.domain.tld/.well-known/caldav). You can either install Baikal on another domain, subdomain or move Nextcloud to another location
2018-11-02 20:48:46,300 INFO     yunohost.hook <lambda> - [991.1] + [[ ! x =~ /yunohost/sso/ ]]
2018-11-02 20:48:46,300 WARNING  yunohost.hook <lambda> - [991.1] !!
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] + [[ 000 != 404 ]]
2018-11-02 20:48:46,301 WARNING  yunohost.hook <lambda> - [991.1]   baikal's script has encountered an error. Its execution was cancelled.
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] + echo 'je passe dans le if'
2018-11-02 20:48:46,301 WARNING  yunohost.hook <lambda> - [991.1] !!
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] je passe dans le if
2018-11-02 20:48:46,301 WARNING  yunohost.hook <lambda> - [991.1]
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] + ynh_die 'Baikal cannot be installed because of a conflit with another application, Nextcloud usually. This app handles the location where you want to install Baikal (https://sous.domain.tld/.well-known/caldav). You can either install Baikal on another domain, subdomain or move Nextcloud to another location'
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] + echo 'Baikal cannot be installed because of a conflit with another application, Nextcloud usually. This app handles the location where you want to install Baikal (https://sous.domain.tld/.well-known/caldav). You can either install Baikal on another domain, subdomain or move Nextcloud to another location'
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] + exit 1
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] + ynh_exit_properly
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] + local exit_code=1
2018-11-02 20:48:46,301 INFO     yunohost.hook <lambda> - [991.1] + '[' 1 -eq 0 ']'
2018-11-02 20:48:46,302 INFO     yunohost.hook <lambda> - [991.1] + trap '' EXIT
2018-11-02 20:48:46,302 INFO     yunohost.hook <lambda> - [991.1] + set +eu
2018-11-02 20:48:46,302 INFO     yunohost.hook <lambda> - [991.1] + echo -e '!!\n  baikal'\''s script has encountered an error. Its execution was cancelled.\n!!'
2018-11-02 20:48:46,302 INFO     yunohost.hook <lambda> - [991.1] + type -t ynh_clean_setup
2018-11-02 20:48:46,302 INFO     yunohost.hook <lambda> - [991.1] + ynh_die
[...]

maniackcrudelis · 2018-12-03T23:03:40Z

I'm not sure to understand the problem you have with this helper. I just tried it, it seems to work as expected.
Anyway, I'm going to rewrite it and commit as a third helper in this PR, in order to have a simpler version of the helper. More comprehensible...

A simpler version of the helper, without the array and with more comments.

maniackcrudelis · 2018-12-03T23:20:55Z

Please try with this new helper.
If it's easier for you to work with this one, please upgrade the helper in the experimental repository, or tell me to do so.

alexAubin · 2019-02-04T17:06:44Z

scripts/install

@@ -70,6 +70,22 @@ ynh_setup_source "$final_path"
 # NGINX CONFIGURATION
 #=================================================

+# Exits if another app serves "https://${domain}/.well-known/caldav" and display an error message for user
+# Waiting for Yunohost to handles this kind of conflict https://github.com/YunoHost-Apps/baikal_ynh/issues/17
+echo "je suis devant le if"


Might want to remove those debug messages :D

alexAubin · 2019-02-04T17:08:44Z

Bump guys, just passing by to say that I'm interested in seeing this PR merged because an user had this issue today about trying to install Baikal while Nextcloud was installed 😅

Gofannon · 2019-02-06T15:36:46Z

Hello there,

I used the new helper provided by @maniackcrudelis and did some tests but could not totally test the PR. Package_check could not install baikal so I tested directly on a VM.

On the VM, if nextcloud is already installed, baikal cannot be installed so this part is okay. Issue is that baikal does not work on my VM... There is an nginx error about php in the logs but the config files are identical on my production server and on the VM. So I give up on this.

My VM could also be in a poor state as I have removed a lot of applications in order to test baikal / nextcloud.

maniackcrudelis · 2019-02-06T16:18:08Z

Looks good to me
But looks like the CI disagree with me...

maniackcrudelis · 2019-02-06T22:52:44Z

Ok, I investigated a little this strange helper. (I love when the code is clear and without ambiguities...)
So it appears that when you use package_check, trying to reach https://${domain}/.well-known/caldav gets you to https://${domain}/yunohost/admin, and when you're doing the same things from a raw YNH, you get https://${domain}/yunohost/sso.
From the original helper,

It's handled if it does not redirect to the SSO nor return 404

Which means probably that the new helper should return 0 instead of 1.
[...]
I'm not even sure now...
I've installed nextcloud, now I get https://${domain}/nextcloud/remote.php/dav and the install crashes.

Ok, let's recap:

Without nextcloud:
- With package_check, .well-known redirect to https://${domain}/yunohost/admin
  - It fails both with is_url_handled_rewrited and is_url_handled.
    It shouldn't though...
- Without package_check, .well-known redirect to https://${domain}/yunohost/sso
  - It fails with is_url_handled_rewrited
  - But it works with is_url_handled
    It should work here as well.
With nextcloud now:
- Didn't try with package_check, but not relevant I think anyway.
- Without package_check, .well-known redirect to https://${domain}/nextcloud/remote.php/dav and fails with both helpers.
  That's the expected behavior.

Currently, is_url_handled_rewrited return 1 only for a 404 or the sso. Is it logical ?
Shit... why do we have to handle that kind of shitty code without explanation...

So I think that, if we have the sso, the admin or 404 (do we really can expect a 404 !?) we should consider that's fine. If we have something else than that, that would means an app has answered the request.

maniackcrudelis · 2019-05-05T18:29:05Z

This PR is quite old.
I fixed the "well-known" issue with this PR #34
And added a change_url script by this other one, #35

This PR can be closed.

Gofannon · 2019-05-06T12:16:43Z

Alright !

maniackcrudelis and others added 7 commits December 17, 2017 18:04

Merge pull request YunoHost-Apps#22 from YunoHost-Apps/testing

4c02e4f

Testing

Fix fake patch...

d1e1592

Merge pull request YunoHost-Apps#24 from YunoHost-Apps/fix_patch

c7eefc2

Fix fake patch...

add change_url functionnality

d3ed551

use handler ynh_abort_if_errors

0228123

fix missing helper with _common.sh

a53b172

handle ".well-known" if it is already served on the domain (same way …

244e673

…as Nextcloud)

maniackcrudelis reviewed Sep 5, 2018

View reviewed changes

Gofannon added 2 commits November 2, 2018 20:35

[enh] change_url - use new helper

bb004f5

[WIP] attempt to fix conflit with nextcloud

7d2d0cd

Rewrite is_url_handled

1057d71

A simpler version of the helper, without the array and with more comments.

alexAubin reviewed Feb 4, 2019

View reviewed changes

Gofannon added 2 commits February 6, 2019 16:27

add new helper 'is_url_handled_rewrited' + use it

70646d5

[enh] correct restore script

d3290ee

maniackcrudelis approved these changes Feb 6, 2019

View reviewed changes

Gofannon closed this May 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "move to a new domain" + fixes #28

Add "move to a new domain" + fixes #28

Gofannon commented May 2, 2018 •

edited

Gofannon commented Sep 3, 2018

maniackcrudelis Sep 5, 2018

Gofannon Nov 2, 2018

maniackcrudelis Dec 3, 2018

maniackcrudelis Sep 5, 2018

Gofannon Nov 2, 2018

maniackcrudelis Dec 3, 2018

maniackcrudelis Sep 5, 2018

Gofannon Nov 2, 2018

maniackcrudelis Sep 5, 2018

Gofannon Nov 2, 2018

maniackcrudelis Sep 5, 2018

Gofannon Nov 2, 2018

maniackcrudelis commented Sep 5, 2018

Gofannon commented Sep 13, 2018

Gofannon commented Nov 2, 2018

maniackcrudelis commented Dec 3, 2018

maniackcrudelis commented Dec 3, 2018 •

edited

alexAubin Feb 4, 2019

alexAubin commented Feb 4, 2019

Gofannon commented Feb 6, 2019

maniackcrudelis commented Feb 6, 2019

maniackcrudelis commented Feb 6, 2019

maniackcrudelis commented May 5, 2019

Gofannon commented May 6, 2019

Add "move to a new domain" + fixes #28

Add "move to a new domain" + fixes #28

Conversation

Gofannon commented May 2, 2018 • edited

Problem

Solution

Edit 1

PR Status

Validation

Gofannon commented Sep 3, 2018

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

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

maniackcrudelis commented Sep 5, 2018

Gofannon commented Sep 13, 2018

Gofannon commented Nov 2, 2018

maniackcrudelis commented Dec 3, 2018

maniackcrudelis commented Dec 3, 2018 • edited

Choose a reason for hiding this comment

alexAubin commented Feb 4, 2019

Gofannon commented Feb 6, 2019

maniackcrudelis commented Feb 6, 2019

maniackcrudelis commented Feb 6, 2019

maniackcrudelis commented May 5, 2019

Gofannon commented May 6, 2019

Gofannon commented May 2, 2018 •

edited

maniackcrudelis commented Dec 3, 2018 •

edited