Skip to content

migrate database if app.ini found #5290

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jan 5, 2019
Merged

migrate database if app.ini found #5290

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a161f18
migrate database if app.ini found
pciavald Nov 7, 2018
670f44b
replacing hard-coded user id by env variable
pciavald Nov 7, 2018
8caca61
Merge branch 'master' into migrate-db-if-config
techknowlogick Jan 5, 2019
e9469e6
Update per @zeripath's feedback
techknowlogick Jan 5, 2019
81be951
Merge branch 'master' into migrate-db-if-config
techknowlogick Jan 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions docker/usr/bin/entrypoint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ if [ "${USER}" != "git" ]; then
sed -i -e "s/AllowUsers git$/AllowUsers ${USER}/g" /etc/ssh/sshd_config
fi

if [ -z "${USER_GID}" ]; then
USER_GID="`id -g ${USER}`"
fi

if [ -z "${USER_UID}" ]; then
USER_UID="`id -u ${USER}`"
fi

## Change GID for USER?
if [ -n "${USER_GID}" ] && [ "${USER_GID}" != "`id -g ${USER}`" ]; then
sed -i -e "s/^${USER}:\([^:]*\):[0-9]*/${USER}:\1:${USER_GID}/" /etc/group
Expand All @@ -22,6 +30,13 @@ for FOLDER in /data/gitea/conf /data/gitea/log /data/git /data/ssh; do
mkdir -p ${FOLDER}
done

if [ -f /data/gitea/conf/app.ini ]; then
echo "Found app.ini config file, migrating database"
chmod 644 /data/gitea/conf/app.ini
Copy link
Contributor

Choose a reason for hiding this comment

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

This will allow anyone with access to the server to read the app.ini, exposing any credentials (database, metrics, smtp).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... I guess you're right. I guess this should be 600. In some ways this is not so bad because this for docker - you shouldn't have other users on it in any case, that's not the docker way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to change the file permissions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly not. If you've not set the INSTALL_LOCK setting in your app.ini and you've got the wrong mode then you won't be able to use /install. However I guess we shouldn't be being preventing people from shooting themselves in the foot like that. Pop a PR on if you're able otherwise I can when I'm next at a dev box. @techknowlogick do you agree that perhaps we shouldn't changing the mode here?

Copy link
Contributor

Choose a reason for hiding this comment

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

@pciavald What was your reasoning for changing the mode?

chown -R ${USER_UID}:${USER_GID} /data/git /data/gitea
su - ${USER} -c gitea migrate -c /data/gitea/conf/app.ini
fi
Copy link
Member

Choose a reason for hiding this comment

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

Please use ${USER}, ${USER_UID}, and ${USER_GID} variables instead of hardcoding them.

Copy link
Contributor Author

@pciavald pciavald Nov 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

${USER_UID}, ${USER_GID}, $UID and $GID are undefined, i'll use $USER:$USER is that ok ?

Copy link
Contributor Author

@pciavald pciavald Nov 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I see these variables (USER_UID and USER_GID) should be defined by default to 1000 if not specified by the docker-compose environment key, and they're not defined in my docker-compose.

So I don't understand why they are undefined, i've echoed them in the entrypoint and only USER was defined. Using $USER:$USER in the chown may create problems if a developer defines the USER_UID and USER_GID in their env.

Copy link
Contributor

Choose a reason for hiding this comment

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

The original entry point script just checks for the variables and if they're there does something. That's why they're undefined.

You may have to adjust it to define the variables if they're not there or empty.

if [ -z "${USER_GID}" ]; then
  USER_GID="`id -g ${USER}`"
fi

And similarly for UID. Otherwise I'd be suspicious that the chown could choose the wrong UID and GID in cases were they were set explicitly - but I'd happily defer to more expert opinion on this.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes. You are right @pciavald, I missed that they were undefined. I think that @zeripath's approach of setting a default for the values is a good one.


if [ $# -gt 0 ]; then
exec "$@"
else
Expand Down