Setting WOODPECKER_DATABASE_DATASOURCE_FILE does not work as expected when using docker image

[smainz]

Component

server, other

Describe the bug

Using this docker-compose.yml snippet

  server:
    image: *woodpecker-server-image
    restart: unless-stopped
    expose:
      - 9000
    volumes:
      - ./woodpecker-server-data:/var/lib/woodpecker/
      - ./secrets/woodpecker/agent-secret.conf:/run/secrets/agent-secret.conf:ro
      - ./secrets/woodpecker/gitea-secret.conf:/run/secrets/gitea-secret.conf:ro
      - ./secrets/woodpecker/gitea-client.conf:/run/secrets/gitea-client.conf:ro
      - ./secrets/woodpecker/docker-config.json:/run/secrets/docker-config.json:ro
      - ./secrets/woodpecker/woodpecker-datasource.conf:/run/secrets/woodpecker-datasource.conf
    environment:
      - WOODPECKER_DATABASE_DRIVER=postgres
      - WOODPECKER_DATABASE_DATASOURCE_FILE=/run/secrets/woodpecker-datasource.conf

does not pick up the datasource secrets from the file, but will try to use /var/lib/woodpecker/woodpecker.sqlite

Log:

server-1  | 9:12AM TRC woodpecker/src/github.com/woodpecker-ci/woodpecker/cmd/server/setup.go:88 > setup datastore: store.Opts{Driver:"postgres", Config:"/var/lib/woodpecker/woodpecker.sqlite", XORM:store.XORM{Log:true, ShowSQL:true}}

The reason is, that the env var WOODPECKER_DATABASE_DATASOURCE is set in the Dockerfile and takes precedence.

To make it work one has to cancel this environment variable by using this:

    environment:
      - WOODPECKER_DATABASE_DRIVER=postgres
      - WOODPECKER_DATABASE_DATASOURCE
      - WOODPECKER_DATABASE_DATASOURCE_FILE=/run/secrets/woodpecker-datasource.conf

Note: No equal sign behind WOODPECKER_DATABASE_DATASOURCE

Had to find out the hard way.

Should this be documented in https://github.com/woodpecker-ci/woodpecker/blob/5e0ec973baea3be1c4f6514b2ce778bfc0c961d6/docs/docs/30-administration/10-server-config.md?plain=1#L66-L102

If you agree, that is is only a documentation issue, I will provide a PR.

System Info

Docker Image `woodpeckerci/woodpecker-server:v2.3.0`

Additional context

No response

Validations

• [X] Read the docs.
• [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• [X] Checked that the bug isn't fixed in the next version already [https://woodpecker-ci.org/faq#which-version-of-woodpecker-should-i-use]

[zc-devs]

Only database settings don't work, right? Do other (WOODPECKER_GRPC_SECRET_FILE, WOODPECKER_AGENT_SECRET_FILE, WOODPECKER_GITEA_CLIENT_FILE, WOODPECKER_GITEA_SECRET_FILE) settings work as intended?

the env var WOODPECKER_DATABASE_DATASOURCE is set in the Dockerfile and takes precedence

There

[smainz]

Yes, it is only the datasource, others work as expected.

I do not have an idea of what could be changed. The code uses a sqlite file relative to the executable, but this can not be used with docker, as yoiu can not mount a volume there.

[zc-devs]

I think we should process WOODPECKER_DATABASE_DATASOURCE_FILE with higher priority than WOODPECKER_DATABASE_DATASOURCE as it is supposed to be more secure. So, that is is not a documentation issue.

[qwerty287]

It looks like this is an upstream issue of https://github.com/urfave/cli because we're directly using the feature: https://github.com/woodpecker-ci/woodpecker/blob/4e44dd0e76b516685bc79af4885bcfb739720986/cmd/server/flags.go#L223

[smainz]

Yes, it is causes upstream, but I do not know if it can be called an issue.

If we change the Dockerfileto something like this:

RUN echo "/var/lib/woodpecker/woodpecker.sqlite" > /etc/woodpecker-datasource.conf
ENV WOODPECKER_DATABASE_DATASOURCE_FILE=/etc/woodpecker-datasource.conf

the problem should vanish.

If the user does set -e WOODPECKER_DATABASE_DATASOURCE... this will take precedence and if he sets -e WOODPECKER_DATABASE_DATASOURCE_FILE...it will overwrite the setting in the Dockerfile.

[zc-devs]

It looks like this is an upstream issue of https://github.com/urfave/cli because we're directly using the feature

But we get this flag from Env via os package:

- FilePath: os.Getenv("WOODPECKER_DATABASE_DATASOURCE_FILE"),
+ FilePath: "/etc/mysql/password",

Note that default values set from file (e.g. FilePath) take precedence over default values set from the environment (e.g. EnvVar).

Also If EnvVars contains more than one string, the first environment variable that resolves is used.

Precedence.

[qwerty287]

But we get this flag from Env via os package:

Yes, I know, I mean we're using the FilePath feature of urfave/cli.

Note that default values set from file (e.g. FilePath) take precedence over default values set from the environment (e.g. EnvVar).

Weird, then this should work :thinking: Or do I misunderstand something completely here?

[zc-devs]

Yeah, we need more ~gold~ tests :) There are no open issues about precedence. So, should work...

[smainz]

Precedence rules are different https://cli.urfave.org/v2/examples/flags/#precedence

Envelope var before file.

[smainz]

In my oppinion, the env var should take precedence to the content of some file.

The shorter the lifetime of a flag the higher the precedence. Rational: The user wants to override some argument for the next execution of a program without much hazzle.

So

1. command line arg
2. Env var
3. Some file

Shall I post a PR for changing the Dockerfile?

[qwerty287]

Shall I post a PR for changing the Dockerfile?

Would be fine for me.

[6543]

ah now i know where it did come from :D

please dont fix bugs in the wrong place ... this bugfix would e.g. not fix it for anything not using docker

(...yes it might not be a problem as that environments dont have preset things... but it still will exist)