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

Add clconf backend #663

Closed
wants to merge 6 commits into from
Closed

Conversation

lucastheisen
Copy link
Contributor

A brief explanation... The current file backend is insufficient for my needs. Specifically, it can only handle one file at a time. This means we need a preprocessor to merge yaml files before using them in confd. This approach has many downsides:

  1. preprocessor is needed
  2. temp files are needed which can be less secure given that secrets are provided on a ramfs to prevent them from ever being written to disk
  3. watching doesn't work

Long story short, we wrote our preprocessor in go, and called it clconf (containerland config). It is capable of many things, but primarily the ability to merge yaml sources into a single map. The next logical step was to create a backend directly for confd thereby avoiding all the problems listed above.

It could be used as a simple drop in replacement for the file backend (which i think makes the most sense) but was created as its own backend to see if you are interested. It uses the same config option (--file), but allows the value to be a comma delimited list (latter entries overwriting prior entries if overlaps occur). The clconf code was also written to leverage the same libraries as confd because the intention was always to try to merge upstream.

What do you think?

@okushchenko
Copy link
Collaborator

Thanks for opening a PR! The idea of this backend is great, here are a few of my thought:

  1. There is a strong opposition against adding any more backends in core of Confd. Your backend looks too use-case specific. I have a PR with the plugin system, which might solve the issue. Please, take a look at it, maybe even write a small code review if you can, it's open for suggestions. Once it's merged you can rewrite your backend as a plugin, drop alongside the main Confd binary and use it.
  2. I've tried to review the https://gitlab.com/pastdev/s2i/tree/master/clconf dependency, but I just can't figure it out. Is it a fork of confd with your changes or what? Also, I don't like having a dependency on your personal repo in GitLab. This might be a bit biased though, thoughts?

@lucastheisen
Copy link
Contributor Author

@okushchenko ,

  1. I have seen the strong opposition to adding any more backends to core confd. My purpose here is to have a more capable file backend. The current file backend accepts only a single file, but this backend allows for multiple. It could actually replace the current file backend and maintain 100% compatibility. I created this pull request mostly to determine whether you would be willing to replace the current file backend. I will take a look at your plugin solution, but there seems to be no momentum behind that either...
  2. No, clconf is not a fork of confd. Its purpose is to merge multiple yaml files into a single map that can be used either as a library (for other go projects), or to write to stdout (so it can be piped into other external programs). It attempts to be highly compatible with confd (identical dependencies whenever possible) with the hope that i could get my clconf backend to confd accepted as a more capable version of the file backend.

This project is very promising but the resistance to the communities pull requests makes it seem like the project is dead. I really like the idea of a plugin system, as that appears to be the only way confd will add new features, but like i said, there appears to be just as much resistance to that as to the rest of the pull requests. Anyway, I will try to review your plugin pull request and add any input I have, and hopefully that will help make the case.

Thanks for looking into this, and if you are willing to consider this as a replacement for the file backend (or a new backend), I would be happy to make the necessary modifications. If not, please respond and close this ticket so I can decide if i should proceed with my own fork, or just start a new configuration project.

@okushchenko
Copy link
Collaborator

okushchenko commented Feb 23, 2018

@lucastheisen I don't see any resistance in the merging of PRs, besides the need to review, resolve conflicts, apply changes to the existing PRs and it takes time. You can help with that if you want. I've merged a lot of PRs during the last half a year adding 3 new backends and many new features and bug fixes. I only have a limited time to work on Confd, and the backlog is quite big. I can definitely see a lack of communication from me, though. Do you have any suggestions on how can we improve that? Should we create a gitter channel?

It would be great if you can rewrite clconf backend as a replacement for the current file backend, but keep in mind that it will have to be compatible with both json and yaml files. I think that support for multiple files in the file backend is a great idea.

I've put the plugin system on hold a while ago to focus on the backlog of PRs and issues because it affects a lot of critical paths in the codebase and might mess up other PRs. Now is a good time to finally merge it. Also, I forgot to link the PR in the previous comment #568

@lucastheisen
Copy link
Contributor Author

Closing in favor of file backend replacement: Update file backend to support multiple files #676

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

Successfully merging this pull request may close these issues.

2 participants