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

Split config files into multiple files #17653

Open
xorima opened this issue Jun 18, 2019 · 11 comments
Open

Split config files into multiple files #17653

xorima opened this issue Jun 18, 2019 · 11 comments

Comments

@xorima
Copy link
Contributor

xorima commented Jun 18, 2019

What would you like to be added: for the configuration to be able to be split into a conf.d system

Why is this needed: As a maintainer of the Grafana Chef cookbook I find it troublesome to have to handle one monolithic configuration and would like the option to have the configuration split into sensible parts

@torkelo torkelo changed the title Enable use of a conf.d system Split config files into multiple files Jun 19, 2019
@ThomasSchoenbeck
Copy link

Would also be good to separate basic config from security config containing oauth client id's or security keys.

a split of config files would allow to provide basic config for example via kubernetes config map, and the security config via kubernetes secrets.

@didib
Copy link

didib commented Apr 20, 2020

+1 from me as well. I am currently working on integration of grafana and oVirt DWH, and it's hard to decide what to do - write my own complete conf file (and risk overwriting existing changes done to it) or try to edit it in-place (and risk more bugs in my code and in the generated file). For reference, see also:

https://unix.stackexchange.com/questions/4029/what-does-the-d-stand-for-in-directory-names/4040#4040

For a long time I didn't know about "name" for this convention, so it was hard to search for it. systemd calls them "drop-in directories", but I do not see that this name caught (searching google for "drop-in directory" finds mainly systemd references).

@didib
Copy link

didib commented Apr 20, 2020

I'd also write concrete details about how I think/hope it should look. In particular, this should not be left for packagers (e.g. Linux distributions) to do , but done consistently - so that it's easier for 3rd-parties (e.g. plugin developers) to supply their own custom conf without having to deal with the differences between distributions. It's based on my experience dealing over the years with various packages that did similar things, and even more so with those that didn't, at first, and with the pain people had adding this later.

  1. The package should include a directory /etc/grafana/grafana.ini.d
  2. The existing grafana.ini should be inside it, e.g. /etc/grafana.ini.d/main-grafana.ini or whatever
  3. The configuration file language should include a new directive, e.g. "include". Its semantics should be something like: If given a file, include it. If given a directory, include all files inside it ending with '.ini'. Last point is significant so that people or tools can keep backups of files they change there and having grafana ignore them, simply by having a different suffix.
  4. /etc/grafana/grafana.ini should include just a single 'include /etc/grafana/grafana.ini.d', plus some comments.

@andreasgerstmayr
Copy link
Contributor

@torkelo : is there interest in this feature? would this feature be merged if I prepare a PR for it?
I'd implement it like @didib suggested, except that the include directive should accept a glob, for example
include /etc/grafana.ini.d/*.conf
to be conform with common packages like sshd [1] and nginx [2].

The include directive can only be specified at the top-level (not inside any section), and if the same section exists in multiple configuration files, the latter one overrides the first occurrence of any setting.

[1] https://manpages.debian.org/unstable/openssh-server/sshd_config.5.en.html#Include_/etc/ssh/sshd_config.d/*.conf
[2] http://nginx.org/en/docs/ngx_core_module.html#include

@didib
Copy link

didib commented Jun 28, 2020

@torkelo : is there interest in this feature? would this feature be merged if I prepare a PR for it?
I'd implement it like @didib suggested, except that the include directive should accept a glob, for example
include /etc/grafana.ini.d/*.conf
to be conform with common packages like sshd [1] and nginx [2].

I agree

The include directive can only be specified at the top-level (not inside any section), and if the same section exists in multiple configuration files, the latter one overrides the first occurrence of any setting.

  1. You should be more explicit: What if the former sets a setting that the latter does not? E.g:
    file1.conf:
    [sec1]
    key1=val1
    key2=val2

file2.conf:
[sec1]
key1=val1

Do you suggest to revert sec1.key2 to its default setting, not keep val2?

  1. I personally would rather keep at least security.secret_key in its own file. There is also external_image_storage.s3.secret_key. There are other keys that are used in more than one section, sometimes with similar/same semantics, e.g. allow_sign_up. Do you think it's such a bad idea to let a user have a file with just "allow_sign_up = true" and include it in each section they want? Thus be able to change a single place and affect all of them? Not that important, though. If you think this complicates the semantics or the implementation too much, fine.

@andreasgerstmayr
Copy link
Contributor

What if the former sets a setting that the latter does not? E.g:
file1.conf:

[sec1]
key1=val1
key2=val2

file2.conf:

[sec1]
key1=val1

Do you suggest to revert sec1.key2 to its default setting, not keep val2?

No, sec1.key2 should not be reverted. I meant sec1.key1 of file2.conf will override sec1.key1 of file1.conf. sec1.key2 of file1.conf will persist.

I personally would rather keep at least security.secret_key in its own file. There is also external_image_storage.s3.secret_key. There are other keys that are used in more than one section, sometimes with similar/same semantics, e.g. allow_sign_up. Do you think it's such a bad idea to let a user have a file with just "allow_sign_up = true" and include it in each section they want? Thus be able to change a single place and affect all of them? Not that important, though. If you think this complicates the semantics or the implementation too much, fine.

I would keep it simple and restrict includes only to the global level. If we allow includes on the section level as well, we could end up with nested sections, which should not be possible. With the current planned approach you can create a new secret_key.conf with

[security]
secret_key = ...

@avlitman
Copy link

@torkelo : is there interest in this feature? would this feature be merged if I prepare a PR for it?
I'd implement it like @didib suggested, except that the include directive should accept a glob, for example
include /etc/grafana.ini.d/*.conf
to be conform with common packages like sshd [1] and nginx [2].

The include directive can only be specified at the top-level (not inside any section), and if the same section exists in multiple configuration files, the latter one overrides the first occurrence of any setting.

[1] https://manpages.debian.org/unstable/openssh-server/sshd_config.5.en.html#Include_/etc/ssh/sshd_config.d/*.conf
[2] http://nginx.org/en/docs/ngx_core_module.html#include

Hi Andreas,
Did you start a PR for this feature ?
I'm also interesting to have this feature and you mention you can start working on it, so I wonder if you did.

Thank you,
Aviv

@andreasgerstmayr
Copy link
Contributor

@avlitman: No, I didn't start implementing it. It is a big change, I'll only start working on it if a PR has chances to get merged upstream, but there was no reaction from Grafana's maintainers so far.

@avlitman
Copy link

avlitman commented May 31, 2021

@torkelo : is there interest in this feature?
This feature will help me develop Grafana in a much easier and orderly way.
We are using Grafana as a monitoring tool of the ovirt project, that has a lot of users.

Thanks,
Aviv

@zoltanbedi zoltanbedi added this to Inbox in Backend Platform Backlog via automation May 31, 2021
@marefr marefr removed this from Inbox in Backend Platform Backlog Jun 1, 2021
Copy link
Contributor

This issue has been automatically marked as stale because it has not had activity in the last year. It will be closed in 30 days if no further activity occurs. Please feel free to leave a comment if you believe the issue is still relevant. Thank you for your contributions!

@github-actions github-actions bot added the stale Issue with no recent activity label Jan 20, 2024
@xorima
Copy link
Contributor Author

xorima commented Jan 20, 2024

Please don't close this. I think a wider discussion is still needed

@github-actions github-actions bot removed the stale Issue with no recent activity label Jan 23, 2024
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

6 participants