mwdb-core icon indicating copy to clipboard operation
mwdb-core copied to clipboard

Implement support for secrets mounted as a file

Open psrok1 opened this issue 2 years ago • 1 comments

Feature Category

  • [ ] Correctness
  • [ ] User Interface / User Experience
  • [ ] Performance
  • [x] Other (please explain)

Describe the problem

In most environments, secrets can be mounted as an environment variable or a file placed in special directory.

Kubernetes support both ways, but some environments don't (e.g. Docker Swarm). In addition, storing the actual secret values in files has additional security advantages because it's easier to keep appropriate access control rules on files.

Describe the solution you'd like

Support for special env vars (KARTON_S3_ACCESS_KEY_FILE) with path to the file containing secret value.

It would be similar approach to MinIO Custom Access and Secret Key files described in https://docs.min.io/docs/minio-docker-quickstart-guide

Implementation can be done in few ways:

  • Implement ConfigSource (like EnvironmentConfigSource) that provides special handling for keys ending with _FILE.
  • Somehow modify the typedconfig to support this option only for chosen keys (optional)

Describe alternatives you've considered

None

psrok1 avatar Sep 15 '22 15:09 psrok1

What happens when there's both a "regular" config variable (MWDB_XXX) and "file" one (MWDB_XXX_FILE)? Options are:

  • "normal" option has the precedence
  • "file" option has the precedence
  • runtime error (I prefer that option, but it's not like typedconfig work in most cases)

What happens when a regular variable name ends with _file? That's not entirely a theoretical question, as there already is flask_config_file for mwdb. Again, I see a few options:

  • Careful coding, soFLASK_CONFIG_FILE_FILE for file variable and FLASK_CONFIG_FILE for regular variable. A bit ugly, because it makes it impossible to use "flask_config" variable (name conflict). Not a issue in the real world though, probably.
  • Disallow variables that end with _file. Backward-compatibility breach so I'm not sure. Though in this case you can rename the variable to flask_config, handle it as a content of the flask config file, and it'll more or less work magically, I think?
  • Ignore the issue ;). This is a pretty niche corner case.

Finally, you seem to consider only adding this as a general config provider. But I Imagine most people won't use things like, for example, MWDB_REQUEST_TIMEOUT_FILE. Maybe it's enough to add explicit support for a few magic keys, like s3_storage_secret_key_file. Don't like this solution too much, but leaving it for consideration.

msm-code avatar Sep 15 '22 16:09 msm-code

I see there is no widely supported standard for that and every platform does this in a bit different way e.g. contents are key=value, just value or need to be fetched somehow.

So we should not complicate configuration further and stay on supporting mwdb.ini and env vars.

psrok1 avatar Jan 23 '23 13:01 psrok1