Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cli option to use settings from env variables #7160

Merged
merged 1 commit into from Sep 23, 2023

Conversation

Minion3665
Copy link
Member

Change-Id: I75762ad620132037523fa82167a3ff17075c7027

  • Target version: master

Summary

Currently in docker it is possible to do configuration through
environment variables
, which
works using the start-collabora-online.sh start-collabora-online.pl
scripts. This PR lets COOLWSD listen to the same environment
variables directly

TODO

  • ...

Checklist

  • Code is properly formatted
  • All commits have Change-Id
  • I have run tests with make check
  • I have issued make run and manually verified that everything looks okay
  • Documentation (manuals or wiki) has been updated or is not required

@Minion3665 Minion3665 marked this pull request as ready for review September 6, 2023 09:57
@timur-g
Copy link
Contributor

timur-g commented Sep 11, 2023

Can you please explain how this will work?

@Minion3665
Copy link
Member Author

@timur-g

Can you please explain how this will work?

Sure thing! The idea is to be able to replace the work done by the docker scripts (see https://github.com/CollaboraOnline/online/blob/master/docker/from-packages/scripts/start-collabora-online.sh) directly in wsd...

if test -n "${username}"; then
    perl -pi -e "s/<username (.*)>.*<\/username>/<username \1>${username}<\/username>/" /etc/coolwsd/coolwsd.xml
fi
if test -n "${password}"; then
    perl -pi -e "s/<password (.*)>.*<\/password>/<password \1>${password}<\/password>/" /etc/coolwsd/coolwsd.xml
fi
if test -n "${server_name}"; then
    perl -pi -e "s/<server_name (.*)>.*<\/server_name>/<server_name \1>${server_name}<\/server_name>/" /etc/coolwsd/coolwsd.xml
fi
if test -n "${dictionaries}"; then
    perl -pi -e "s/<allowed_languages (.*)>.*<\/allowed_languages>/<allowed_languages \1>${dictionaries:-de_DE en_GB en_US es_ES fr_FR it nl pt_BR pt_PT ru}<\/allowed_languages>/" /etc/coolwsd/coolwsd.xml
fi

This will let us remove those scripts entirely from docker, but we still want to keep the ability to add settings from environment variables because it makes configuring docker easier.

The idea here is to add an extra option (--use-env-vars) which will get COOLWSD to look at the same environment variables that the scripts did directly. Does that answer the question?

@timur-g
Copy link
Contributor

timur-g commented Sep 11, 2023

Thanks, just one more. Will you update documentation?

@Minion3665 Minion3665 force-pushed the private/skyler/env-var-config branch 2 times, most recently from 61202ca to 23551fa Compare September 11, 2023 12:01
@Minion3665
Copy link
Member Author

@timur-g

Thanks, just one more. Will you update documentation?

thanks for the reminder! I've updated the manpage to add the new option now. I'd like to edit https://sdk.collaboraonline.com too but I don't know where the source to that is... do you?

@timur-g
Copy link
Contributor

timur-g commented Sep 11, 2023

No, but I guess @thebearon knows.

@timar timar force-pushed the private/skyler/env-var-config branch from 23551fa to 7459122 Compare September 22, 2023 21:54
@timar timar removed the request for review from Ashod September 22, 2023 21:55
Copy link
Member

@timar timar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • remoteconfigurl is missing
  • domain is missing (although it was deprecated by alias_groupX so probably we can skip it)

Currently [in docker it is possible to do configuration through
environment variables](https://col.la/dockercodeconfigviaenv), which
works using the start-collabora-online.sh start-collabora-online.pl
scripts. This commit lets COOLWSD listen to the same environment
variables directly

Change-Id: I75762ad620132037523fa82167a3ff17075c7027
Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
@timar timar force-pushed the private/skyler/env-var-config branch from 7459122 to f011240 Compare September 22, 2023 22:13
@timar
Copy link
Member

timar commented Sep 22, 2023

  • remoteconfigurl is missing
  • domain is missing (although it was deprecated by alias_groupX so probably we can skip it)

I added remoteconfigurl.

@timar timar merged commit 1b3218d into master Sep 23, 2023
12 checks passed
@timar timar deleted the private/skyler/env-var-config branch September 23, 2023 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants