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

Support for docker secrets #544

Open
rbuffat opened this issue Sep 6, 2021 · 11 comments
Open

Support for docker secrets #544

rbuffat opened this issue Sep 6, 2021 · 11 comments

Comments

@rbuffat
Copy link

rbuffat commented Sep 6, 2021

It would be nice if redis_exporter would support docker secrets for configuration variables.

E.g. the postgres container allows to specify docker secrets with "__FILE" appended to the environment variable.

E.g. POSTGRES_USER=dbuser could be specified as "POSTGRES_USER_FILE=/run/secrets/postgres_user". In the latter case, the POSTGRES_USER variable is extracted from the file /run/secrets/postgres_user.

See also the Docker Secrets section in https://hub.docker.com/_/postgres

@oliver006
Copy link
Owner

Thanks @rbuffat - I think this could be a useful improvement. I currently don't have the bandwidth but I'd be happy to review and merge a PR if one is submitted.

@oliver006
Copy link
Owner

fwiw, the postgres docker image does this in the startup shell script, relevant code is here:
https://github.com/docker-library/postgres/blob/ff5f92b39d9b0081067741459af6202daaf7b3d5/13/alpine/docker-entrypoint.sh#L9

Repository owner deleted a comment from vsilveirarj Feb 26, 2023
@stevleibelt
Copy link

@rbuffat is this still a valid issue for you?

How high is the chance that you can provide a pull request?

Cheers,
Stev

@aballman
Copy link
Contributor

@oliver006 I'm interested in picking this one up. Would it make sense to have another file for users similar to the password file you can specify below?

redisPwdFile = flag.String("redis.password-file", getEnv("REDIS_PASSWORD_FILE", ""), "Password file of the Redis instance to scrape")

I don't think you can have a user without a password so I'm inclined to modify the passwords file to support usernames.

@oliver006
Copy link
Owner

@aballman - appreciate your offer for help here!

would support docker secrets for configuration variables - what configs do you intend to support? Just user and password?

I don't think this needs changes to the exporter itself but can be solved with a bit of shell magic. See my comment from Feb 26 - the postgres docker image pulls the contents from the secret files in the shell and exports it so the postgres process can pick it up with the usual mechanics.

Why not do something similar for the redis_exporter? What do you think?

@aballman
Copy link
Contributor

@oliver006 Taking the postgres example as inspiration, they only provide the _FILE option for a few things.

From the linked dockerhub page:

Currently, this is only supported for POSTGRES_INITDB_ARGS, POSTGRES_PASSWORD, POSTGRES_USER, and POSTGRES_DB.

In this case since the request used user as an example that would be an easy thing to adapt into the existing password file.

{
  "redis://redis6:6379": "",
  "redis://pwd-redis5:6380": "redis-password"
}

Could pretty easily become

{
  "redis://redis6:6379": "",
  "redis://pwd-redis5:6380": "redis-user:redis-password"
}

@aballman
Copy link
Contributor

Why not do something similar for the redis_exporter? What do you think?

I like that the password file approach allows you to use the multi-target service monitor pattern with this exporter. If you do some shell magic to pass args or env for REDIS_PASSWORD and REDIS_USER then you're stuck having to have a 1:1 pairing of exporters to redis instances, or you have to make sure the ACL matches between all your scrape targets.

Seems like maybe the original requestor has moved on. @stevleibelt what's your use case on this one?

@stevleibelt
Copy link

@aballman I have no use case for this issue. My intention was to figure out if this issue is still relevant since is was created more than 18 months ago.

From my point of view, this issue could be closed with a remark that there is currently no use case for it/that the requester does not need it anymore.

Cheers,
Stev

@aballman
Copy link
Contributor

Ah apologies @stevleibelt - I misunderstood and thought you were also interested in seeing this implemented

@rbuffat
Copy link
Author

rbuffat commented Mar 29, 2023

@stevleibelt I'm not familiar with Go, thus my ability to implement this short-term is rather limited.

It still would be nice if this feature would be supported, although I completely understand if this project is not willing to spend the effort of implementing it.

For us, the main advantage of using files for sensitive config options is that we can check in our docker-compose configurations into our version control system and do not have to worry about inadvertently leaking passwords or other sensitive information. There are workarounds around this issue, but they are not nice.

I implemented a small Python library to handle this problem of either reading data from environment variables or files, maybe there exists a similar project for Go: https://github.com/c-technology/envee

@aballman
Copy link
Contributor

@rbuffat What config options would be useful to you in this format?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants