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

Explore if RabbitMQ can load and validate its config file instead of application_controller #551

Closed
michaelklishin opened this issue Jan 14, 2016 · 17 comments
Assignees
Milestone

Comments

@michaelklishin
Copy link
Member

This would give us some specific improvements:

  • We can do much more extensive validation
  • We can produce more sensible error messages

This is also a good step towards #550 and should be investigated before it so that we have a more informed opinion.

We should also explore Riak's approach and sub-projects in this area.

@michaelklishin
Copy link
Member Author

It would be interesting to know what @essen, @dcorbacho, @Ayanda-D, and @Dzol think of this.

While #550 is going to happen, supporting Erlang term file configs would be very nice for backwards compatibility. We don't use any particularly sophisticated data structures so there should be a way to do that by coercing YAML to the Erlang terms we use today.

@hairyhum
Copy link
Contributor

As I remember, Riak and Ejabberd use additional .config file for variables with too sophisticated structure.

@michaelklishin
Copy link
Member Author

So they effectively load two configs and merge them? That sounds reasonable for us.

@hairyhum
Copy link
Contributor

Riak is not using YAML, they use this https://github.com/basho/cuttlefish
This library create .config every time riak is started from .conf file (in sysctl format) and advanced.config (erlang config format)

@michaelklishin
Copy link
Member Author

OK. Whether or not we want to use the same format and Cuttlefish belongs to #550. Is the generated .config file still loaded by application controller or one of Riak apps?

@hairyhum
Copy link
Contributor

It is loaded by application controller just like regular config file. All validations are being executed during config generation with cuttlefish.

@michaelklishin
Copy link
Member Author

So in other words, if we are to follow this route (which is quite clever IMO), validation would be handled in #550 and this issue should be simply closed because there's no real need to do this.

Any objections to this approach?

Also, @hairyhum please take a look at how they handle file generation (e.g. where the target file is located — is it next to the source one?) and how we can make it work with our current way of setting config file path (which is via environment variables). This also can complicate upgrades, so we need to consider that.

@hairyhum
Copy link
Contributor

Riak use cuttlefish_rebar_plugin and shell scripts to generate app.config in .../generated.configs/app.<date>.config and run erl with -config pointing to this location.
If we will use cuttlefish env variable could point to .conf (e.g. RABBITMQ_CONF_FILE=rabbitmq.conf) file and do same thing - create app.config in some location and point erlang to it.

I also looked at ejabberd config implementation. There are separate module that loads config from different sources and store it in mnesia. And there are ~1100 loc.

@Ayanda-D
Copy link
Contributor

Might be a bit off direction to where discussion is going so far, but how fine-grain is the validation going be? And at the moment application_controller will only complain about the .config file format, but playing around with the config values still allows the application startup. Like specifying an unsupported cluster partition handling mechanism. I would fail the startup on such a case. To validate config file outside application_controller, I would probably have two boot steps, one to start a rules process which would load some predefined rules from file (could be in any format, XML for example) to ETS, then second to validate config, against rules held in ETS. If the .config is native Erlang terms (as current implementation), I'd use something like file:consult/1 read the file, which also validates format, then loop through the [{Tag1, Value1},{Tag2, Value2}, ...], validating each Value using rules in ETS, and throw an exception like {error, {badconfig,Value,LineNum}} to the user, to terminate application startup. If config is in a different format, like YAML as suggested, we could also add some mediation using the Erlang YAML parser/generator on the second boot step I've mentioned. My thoughts on this sofar.

@michaelklishin
Copy link
Member Author

@Ayanda-D yes, your thinking about the kind validation we could provide is close to what I had in mind.

@hairyhum
Copy link
Contributor

I'm researching cutterfish config generation for rabbitmq config right now and there are some questionable behaviour.
Cutterfish does not support list type in .conf file, so if user want to add list parameter, there should be either string parsing or multiple parameters, that will be merged in .config file.
It looks like this:

Using string parsing:
.conf file:

auth_backends = rabbit_auth_backend_internal, rabbit_auth_backend_http

.schema file:

{mapping, "auth_backends", "rabbit.auth_backends", [
    {datatype, string}
]}.

{translation, "rabbit.auth_backends", 
fun(Conf) ->
    Settings = cuttlefish:conf_get("auth_backends", Conf),
    [ list_to_atom(Backend) || Backend <- string:tokens(Settings, ",") ]
end}.

Using multiple parameters:
.conf file:

auth_backend.internal = rabbit_auth_backend_internal
auth_backend.http = rabbit_auth_backend_http

.schema file:

{mapping, "auth_backend.$name", "rabbit.auth_backends", [
    {datatype, atom}
]}.

{translation, "rabbit.auth_backends", 
fun(Conf) ->
    Settings = cuttlefish_variable:filter_by_prefix("auth_backend", Conf),
    [ V || {_, V} <- Settings ]
end}.

Both ways will generate same result, but I'm not sure which will be better for users to understand.

@hairyhum
Copy link
Contributor

There is also an option to encode values to setting key. So previous examples will look like

auth_backend.internal = true
auth_backend.http = true

Which can be translated by translation function to same result. But in current example it is not so flexible, but maybe could be used in following example:

default_tags.administrator = true
default_tags.other = true

Schema:

{mapping, "default_tags.$tag", "rabbit.default_user_tags", [{datatype, {enum, [true, false]}}]}.

{translation, "rabbit.default_user_tags", 
fun(Conf) ->
    Settings = cuttlefish_variable:filter_by_prefix("default_tags", Conf),
    [ list_to_atom(Tag) || {["default_tags", Tag], V} <- Settings, V == true ]
end}.

@michaelklishin
Copy link
Member Author

Good finding @hairyhum. This alone may be what tips the scale towards YAML: collections are well covered there. Choices, choices.

@hairyhum
Copy link
Contributor

So here is cuttlefish schema and example config for this schema.
https://gist.github.com/hairyhum/a73c30672acc55125643

There are rabbitmq.example.config in comments with examples and docs to both files above each setting.
There should be more validations to check config values and maybe some other mappings.

You can run cuttlefish escript with this files to generate rabbitmq.config.

@michaelklishin @dumbbell @essen @Ayanda-D can you look at .conf file and say what you think about this way to define config?

@michaelklishin
Copy link
Member Author

It looks pretty good. Some names may be worth revisiting and some examples are missing (e.g. with multiple listeners) but it does demonstrate that Cuttlefish is quite adaptable.

@hairyhum please continue expanding the example with more cases (and settings, we likely don't list 100% there). I'm personally increasingly warming up to the idea of using Cuttlefish with its native sysctl format with a custom mapping over YAML.

@hairyhum
Copy link
Contributor

Speaking about custom mapping, there are several ways to do hierarchical configs (like listeners), and some of them will require more patching.
Here are listeners example:
sysctl version:

listener.tcp.local = 127.0.0.1:5678
listener.tcp.host  = 10.0.0.10:5678

Yaml simple mapping (same as sysctl, just a different format

listener.tcp.local : 127.0.0.1:5678
listener.tcp.host  : 10.0.0.10:5678

Yaml extended mapping (may require some cuttlefish patching)

listeners:
    tcp:
        - 127.0.0.1:5678
        - 10.0.0.10:5678

@michaelklishin
Copy link
Member Author

Closing this as this discussion got gradually intertwined with #500. We won't be validating the config ourselves, at least not yet. But we did learn enough about how Cuttlefish approaches this and what it can do (for #500).

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

3 participants