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

organize a bit and document configuration #13

Closed
volodymyrss opened this issue Mar 4, 2021 · 11 comments
Closed

organize a bit and document configuration #13

volodymyrss opened this issue Mar 4, 2021 · 11 comments
Assignees
Labels
documentation Improvements or additions to documentation refactoring

Comments

@volodymyrss
Copy link
Member

No description provided.

@volodymyrss volodymyrss self-assigned this Mar 4, 2021
@volodymyrss
Copy link
Member Author

most of the time, location of the module is user as root for searching config
else, there is a variable

@volodymyrss volodymyrss added documentation Improvements or additions to documentation refactoring labels Mar 9, 2021
@volodymyrss
Copy link
Member Author

there is a separate point of secret handling. currently, we store some secrets in config. it is not uncommon to do so.
but, it is better to store in config references to the secret location. in the simplest case, it can be reference to env variable: something like

auth_secret: $ODA_DISPATCHER_OAUTH_SECRET

dispatcher would need to interpret this in the config, substituing actual env variable.

similarly, it can be file to read it from. etc.

@burnout87
Copy link
Collaborator

As far as I understood, this will be stored within a record in a table in the DB of a certain deployment.

So the configuration could provide the database access credential and point to the table containing the desired data.

@volodymyrss
Copy link
Member Author

As far as I understood, this will be stored within a record in a table in the DB of a certain deployment.

So the configuration could provide the database access credential and point to the table containing the desired data.

yes, that's an option!

But it's also good that dispatcher does not depend on frontend database. For now - it still does not. This way it is more decoupled and more reliable. Also we currently can run dispatcher without frontend database. At least there should be an option preserve this behavior. Also for testing.

So maybe we could have several possibilities?

  • Set secret directly in the config
  • read it from some source, one of:
    • front-end database
    • env variable

@burnout87
Copy link
Collaborator

So maybe we could have several possibilities?

  • Set secret directly in the config

  • read it from some source, one of:

    • front-end database
    • env variable

A third option could be based on the setup a DB specific for the dispatcher: this would be populated during the deployment, perhaps it could be used ful for other purposes...

@volodymyrss
Copy link
Member Author

A third option could be based on the setup a DB specific for the dispatcher: this would be populated during the deployment, perhaps it could be used ful for other purposes...

to store secret in the DB? as an option - sure, but still, I think it should still be referenced in the config. to have single central source of configuration - with references to other sources (DBs, variables, files) when needed. ok?

the only concern is that it should not be more complex than necessary.

by the way, there is a redis db there, indeed used for other purposes, e.g. for celery there.
but the dispatcher can be also run without it.

@burnout87
Copy link
Collaborator

A third option could be based on the setup a DB specific for the dispatcher: this would be populated during the deployment, perhaps it could be used ful for other purposes...

to store secret in the DB? as an option - sure, but still, I think it should still be referenced in the config. to have single central source of configuration - with references to other sources (DBs, variables, files) when needed. ok?

the only concern is that it should not be more complex than necessary.

Exactly, it only makes sense if it can be used for more purposes than just storing the secret key, if that is the only use we would make of it...no need to struggle too much with another bunch of configuraiton

by the way, there is a redis db there, indeed used for other purposes, e.g. for celery there.
but the dispatcher can be also run without it.

thanks

@burnout87 burnout87 reopened this Mar 12, 2021
@volodymyrss
Copy link
Member Author

to store secret in the DB? as an option - sure, but still, I think it should still be referenced in the config. to have single central source of configuration - with references to other sources (DBs, variables, files) when needed. ok?
the only concern is that it should not be more complex than necessary.

Exactly, it only makes sense if it can be used for more purposes than just storing the secret key, if that is the only use we would make of it...no need to struggle too much with another bunch of configuraiton

Indeed! Which is why I thought the first option - simply keeping it in the config - is pretty functional. It is what is done now for sentry and logstash, and is probably just fine.
It is somewhat common in other parts of the platform, e.g. for frontend, config is created on deployment, by injecting some secret into it.

The disadvantage of this approach is that the entire config has to be secret, or has to be constructed on deployment from secret and other parts. This is a bit annoying and causes rightfully misunderstandings, such as this.

Hence, suggestion to slightly complicate the config, by following another common approach: make reference in the config to some other locations, reserved for secrets. Variables or files would be just fine. They are easy to safely create on deployment too.

I would see is as nice-to-have feature, which would improve our life. Whether it is worth addressing it, depends on:

  • the scope, which we discuss here. We should note it then in a doc.
  • and on the time it would take. Basically, what do you think, how long will it take to add this approach of storing secret ouf of the config, at least just in the variable, and referencing it?

@volodymyrss
Copy link
Member Author

look, I added this bde01f6

Note that this issue is marked as "refactoring" - it is not of the immediate need.
It might be useful if it turns out that current design slows us down.
Some refactoring also would help adding this separate secret feature, as an option, even if it is rather a feature than a refactoring.

We should appreciated that refactoring takes time, not with benefit which is not immediately obvious to the users/stakeholders. So we can not spend too much on it, but we should do it regularly.

Still, we should focus to provide features regularly. E.g. maybe it is better to start by putting the secret plain in the config, and doing that part - and only after proceeding to improve.

This issue here is also marked as documentation - since we made effort to understand and improve, we can explain it, it will save us time.

@burnout87
Copy link
Collaborator

Let's indeed go for a step-by-step approac, putting first the secret clear-text within the config (config_env.yaml).
Next we can think of a more structured approach consisting in placing a reference to the location of the secret within the config (env. variable?).

It is indeed importanto for the future and for providing a more structured plastform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation refactoring
Projects
None yet
Development

No branches or pull requests

2 participants