CanastaWiki / Canasta

MediaWiki Docker image for Canasta, an all-in-one MediaWiki stack for easy deployment and management of enterprise-ready MediaWiki on production environments.
https://www.canasta.wiki
MIT License
38 stars 28 forks source link

Add /update-docker-gateway.sh script #157

Closed pastakhov closed 1 year ago

pastakhov commented 1 year ago

Actually it adds MW_MAP_DOMAIN_TO_DOCKER_GATEWAY environment variable (disabled by default) and this allows to keep /etc/hosts intact.

This change can break Visual Editor on some wiki where firewall does not allow to connect from the server itself, because it was enabled by default in Canasta to modify /etc/hosts, but it should be enabled when it is really needed only

github-actions[bot] commented 1 year ago

Image is built and pushed to the repository as ghcr.io/canastawiki/canasta:1.35.8-20221123-157

jeffw16 commented 1 year ago

Sorry, would someone please explain to me what this PR intends to do?

jeffw16 commented 1 year ago

Also, be sure to not merge this change into master, as all changes should be merged into its own release branch.

pastakhov commented 1 year ago

Testing, MW_MAP_DOMAIN_TO_DOCKER_GATEWAY is not true (default):

web_1   | + /update-docker-gateway.sh
web_1   | ++ get_docker_gateway
web_1   | ++ getent hosts gateway.docker.internal
web_1   | ++ awk '{ print $1 }'
web_1   | + DOCKER_GATEWAY=172.17.0.1
web_1   | + '[' -z 172.17.0.1 ']'
web_1   | ++ get_mediawiki_variable wgServer
web_1   | ++ php /getMediawikiSettings.php --variable=wgServer --format=string
web_1   | PHP Warning:  Illegal offset type in isset or empty in /var/www/mediawiki/w/canasta-extensions/VisualEditor/includes/VisualEditorHooks.php on line 48
web_1   | PHP Warning:  Illegal offset type in /var/www/mediawiki/w/canasta-extensions/VisualEditor/includes/VisualEditorHooks.php on line 49
web_1   | + WG_SITE_SERVER=http://localhost:8092
web_1   | + cp /etc/hosts /root/hosts.new
web_1   | + sed -i '/# MW_SITE_HOST/d' /root/hosts.new
web_1   | + '[' -n http://localhost:8092 ']'
web_1   | ++ echo http://localhost:8092
web_1   | ++ sed -e 's|^[^/]*//||' -e 's|[:/].*$||'
web_1   | + MW_SITE_HOST=localhost
web_1   | + isTrue 0
web_1   | + case $1 in
web_1   | + return 1
web_1   | + echo 'MW_MAP_DOMAIN_TO_DOCKER_GATEWAY is not true'
web_1   | MW_MAP_DOMAIN_TO_DOCKER_GATEWAY is not true
web_1   | + cp -f /root/hosts.new /etc/hosts
web_1   | + sed -i s/DOCKER_GATEWAY/172.17.0.1/ /etc/msmtprc
web_1   | Checking permissions of /mediawiki...
web_1   | + echo 'Checking permissions of /mediawiki...'

MW_MAP_DOMAIN_TO_DOCKER_GATEWAY is true:

web_1   | + /update-docker-gateway.sh
web_1   | ++ get_docker_gateway
web_1   | ++ getent hosts gateway.docker.internal
web_1   | ++ awk '{ print $1 }'
web_1   | + DOCKER_GATEWAY=172.17.0.1
web_1   | + '[' -z 172.17.0.1 ']'
web_1   | ++ get_mediawiki_variable wgServer
web_1   | ++ php /getMediawikiSettings.php --variable=wgServer --format=string
web_1   | PHP Warning:  Illegal offset type in isset or empty in /var/www/mediawiki/w/canasta-extensions/VisualEditor/includes/VisualEditorHooks.php on line 48
web_1   | PHP Warning:  Illegal offset type in /var/www/mediawiki/w/canasta-extensions/VisualEditor/includes/VisualEditorHooks.php on line 49
web_1   | + WG_SITE_SERVER=http://localhost:8092
web_1   | + cp /etc/hosts /root/hosts.new
web_1   | + sed -i '/# MW_SITE_HOST/d' /root/hosts.new
web_1   | + '[' -n http://localhost:8092 ']'
web_1   | ++ echo http://localhost:8092
web_1   | ++ sed -e 's|^[^/]*//||' -e 's|[:/].*$||'
web_1   | + MW_SITE_HOST=localhost
web_1   | + isTrue 1
web_1   | + case $1 in
web_1   | + return 0
web_1   | + [[ localhost =~ ^[0-9]+.[0-9]+.[0-9]+.[0-9]+$ ]]
web_1   | + echo 'Add MW_SITE_HOST '\''172.17.0.1 localhost'\'' to /etc/hosts'
web_1   | + echo '172.17.0.1 localhost # MW_SITE_HOST'
web_1   | + cp -f /root/hosts.new /etc/hosts
web_1   | Add MW_SITE_HOST '172.17.0.1 localhost' to /etc/hosts
web_1   | + sed -i s/DOCKER_GATEWAY/172.17.0.1/ /etc/msmtprc
web_1   | + echo 'Checking permissions of /mediawiki...'
vedmaka commented 1 year ago

Kindly requesting clarification on what this PR intends to do.

@jeffw16 the goal of this change is to make this behaviour (which modifies /etc/hosts inside container) configurable and thus make it possible to opt-out from this on some setups (we do experience issues with modified hosts file on K8s)

pastakhov commented 1 year ago

I'd propose to avoid duplicating thus function and since it looks like we need it both at run-apache.sh

I agree, and I use it already for Taqasta, see https://github.com/WikiTeq/Taqasta/blob/master/_sources/scripts/functions.sh. But this is a big change, let's do this on next step/patch. We have a lot of such changes/fixes/improvements in Taqasta, I discuss with my boss about backporting them.

pastakhov commented 1 year ago

Since this was enabled by default on Canasta maybe it'd be better to also keep it enabled by default?

good question, I guess WikiWorks should decide this

vedmaka commented 1 year ago

@jeffw16 what's your vote on default value of the MW_MAP_DOMAIN_TO_DOCKER_GATEWAY ? ;)

jeffw16 commented 1 year ago

Thanks for asking @vedmaka! To be honest, I don't think I have enough knowledge to make the decision for this. I think the way things have been working right now have worked well for almost everyone at the moment, so my intuition is to keep it the same as it is right now by default. Are you and @pastakhov saying that the default should be changed? Sorry if I'm having trouble grasping the concept here.

vedmaka commented 1 year ago

I'd also keep it enabled by default to match how it worked before, @pastakhov

github-actions[bot] commented 1 year ago

Image is built and pushed to the repository as ghcr.io/canastawiki/canasta:1.35.8-20221129-157

jeffw16 commented 1 year ago

please don't merge until we can get this into 1.2.2