Fix usage of WOODPECKER_DATABASE_DATASOURCE_FILE

[smainz]

fixes #3389

Set variable to let server detect if it's deployed within a container image.
Set the default database connection based on this.

[zc-devs]

What if get rid of WOODPECKER_DATABASE_DRIVER and WOODPECKER_DATABASE_DATASOURCE in Dockerfiles?
There are default values sqlite3 and woodpecker.sqlite respectively. It covers default "test" deployment. If you want sqlite "prod" deployment you should mount volume anyway (set the path) in Docker and Kubernetes. So, there should not be a problem to set it twice. It increase transparency and you can write config in your preferred way (*_FILE or not).

[xoxys]

What if get rid of WOODPECKER_DATABASE_DRIVER and WOODPECKER_DATABASE_DATASOURCE in Dockerfiles?

I would prefer this way as well.

[smainz]

Lets see, what the options are:

1. Do not change the code and just document, that you have to unset the env var WOODPECKER_DATABASE_DATASOURCE if you want to provide the datasource via _FILE: non breaking, but a bit ugly
2. Change the way the default datasource config woodpecker.sqlite is overriden in the docker image as show in the first commit: non breaking, not totaly obvious why the config is done in that way, leaves garbage file in the image, if not used
3. Remove the env vars WOODPECKER_DATABASE_DRIVER and WOODPECKER_DATABASE_DATASOURCE completely in the Dockerfile: probably breaks existing installs which did rely on this, Potential to loose data in this situation (new woodpecker.sqlite file wil be created in current directory), Possibly some frustration with new users which mount a volume to /var/lib/woodpecker, but do not define WOODPECKER_DATABASE_DATASOURCE or WOODPECKER_DATABASE_DATASOURCE_FILE
4. Remove the env vars WOODPECKER_DATABASE_DRIVER and WOODPECKER_DATABASE_DATASOURCE completely in the Dockerfile and change the hard coded default from woodpecker.sqlite to /var/lib/woodpecker/woodpecker.sqlite: Will break installs not running in the container and relying on the default config (development and testing most probably)

Number 3. would make the docker install compatible with the docs:

woodpecker/docs/docs/30-administration/10-server-config.md

Lines 420 to 422 in 4e44dd0

	### `WOODPECKER_DATABASE_DATASOURCE`
	> Default: `woodpecker.sqlite`

As it looks, the intended place for a volume to be mounted is /var/lib/woodpecker as XDG_CACHE_HOME and
ENV XDG_DATA_HOME also point to it.

Did I miss another option?

[smainz]

Any ideas about the direction?

As it is planned for 2.4 it should not be breaking (for end-users) which makes it 2. or 4.

[zc-devs]

another option?

5. Make FR/PR/fork into/of urfave/cli with adjustable priorities and prefer *_FILE here :)

Edit 1

As it is planned for 2.4 it should not be breaking (for end-users) which makes it 2. or 4.
4. Will break installs not running in the container and relying on the default config

There is non-breaking 1.

Edit 2
However, documentation says
Keep in mind that when the original environment variable gets specified at the same time it will override the value read from the file.
So, works as intended :)

Edit 3
I'm for moving to the 3.x milestone and option 3. For now there is a workaround (option 1).

[smainz]

Just collecting what needs to be done for 3.:

• Update documentation for docker install
• Update dockercompose.example.yml
• Create PR for woodpecker/helm to add ENV vars in values.yml
• Add info to migrations.md docs
• Change Dockerfile.server.xxx (2 files)

All but the last are non breaking and could be done right now

[6543]

I'm against the current change!

this will end in each "important" setting being it's own file :/

we should rather switch to a lib that supports config files written in yaml, toml or json...

[smainz]

this will end in each "important" setting being it's own file :/

we should rather switch to a lib that supports config files written in yaml, toml or json...

It is not about having a config file for settings, the existing means are well suited. The only thing I want to achieve is to be able to keep secrets out of the docker-compose or .env file (kept in a git repo) but to mount them into the container.

Could be compared to

  environment:
    DOCKER_PASSWORD:
       from_secret docker_password

in a pipeline.

[6543]

well that is all fine ... but I would then implement the WOODPECKER_DATABASE_DATASOURCE_FILE as how all other_FILE things are implemented and dont change the dockerfile ...

[6543]

#3419

[6543]

PS: still thanks for contributing you valuable time and try to fix it! :)

[smainz]

PS: still thanks for contributing you valuable time and try to fix it! :)

I should say thank you for woodpecker-ci.

[smainz]

PR updated to use the same config file location as the agent does.

I still prefere this PR over #3419, as it keeps the same semantic as all other "_FILE" env vars which is:

Keep in mind that when the original environment variable gets specified at the same time it will override the value read from the file.

[anbraten]

I would also stick to not adding the _FILE cli-argument again. Creating the folder is a bit "ugly" on the other hand. What do you think about setting sth like ENV IN_CONTAINER=true and in case of that use a different default in go?

[smainz]

I would also stick to not adding the _FILE cli-argument again. Creating the folder is a bit "ugly" on the other hand. What do you think about setting sth like ENV IN_CONTAINER=true and in case of that use a different default in go?

If this is the option everyone agrees to, i can implement it that way:
If IN_CONTAINER is set, the default will be /var/lib/woodpecker/woodpecker.sqlite, otherwise it will be woodpecker.sqlite.

[anbraten]

Yes that's what I had in mind. @6543 What do you think?

[6543]

Ack

[smainz]

WOODPECKER_IN_CONTAINER is not documented in server settings as I consider this an implementation detail.

[6543]

WOODPECKER_IN_CONTAINER is not documented in server settings as I consider this an implementation detail.

you can even "hide" it in the cli help menue too ... (flag option) ... and document it in code why it's hidden

it's done outside of urfave/cli 👍

[woodpecker-bot]

Deployment of preview was torn down