Config system

[rubenseyer]

Alright, so here's my suggestion for what the config system could look like. Sorry for being a bit spammy with the issue mentions--I didn't expect them to show up while I was still tidying history. I'm holding off on the Let's Encrypt implementation because I want to see how the tls-sni debacle ends before adding another HTTP server just to respond to the challenge.

• I extended the pre-existing system in serverconf based on string maps. It is format independent - if it can encode string key-value pairs (and has a file format extension) it can be a config format.
• The default config file generated is an .ini file. ~~If the config file is named murmur.ini an additional translation step is performed~~, including warnings for some unsupported features (but I won't promise I catch all of them). (The .ini support uses go-ini.)
• I still dislike the JSON-with-comments idea, ~~but I quickly implemented it anyway to showcase the format independence. However, it would probably require a few good test cases for me to be confident it's usable.~~
• The config files are only read, never written. ~~The freeze system still acts as the persistent storage, which means that to revert a config value to defaults, you can't just comment it out, you have to specify it unset. I think this is an acceptable compromise.~~
• ~~The config system introduces one new feature: if the config format supports some form of subsets (sections in .ini, nested objects in .json) it is possible to set specific overrides for single virtual servers (including reverting to defaults). Not the best form of virtual server management, but at least it's something.~~

To make this meaningful, I also implemented some missing config keys that users migrating from Murmur probably would want. Features like databases, RPC and bandwidth limiting are still missing (but the latter should probably wait for web client support). I also added the SuperUser CLI flags from Murmur. I think the remaining unimplemented Murmur CLI flags would be meaningless for Grumble right now, at least.

[rubenseyer]

I don't mind whether you keep the JSON support or not. If there is no use-case for it we could just drop it.

[mkrautz]

Sorry for leaving this be.

I'll quickly sketch up my thoughts on the personalities in Grumble:

I think the code we're working with now should just be "grumble". If we do implement personalities, they'd probably be a separate command (separate package main). That seems cleaner to me. The way I imagine it would pan out is that we refactor the Server part into a package, with the hooks necessary to implement custom handling.

Anyway, in my mind, since Murmur doesn't have config files at the moment, we should probably just use the Murmur .ini format all the way through.

I guess that does make some sense: Grumble, as it is right now has no way to set config options. (I removed my JSON-RPC thing and/or SSH thing I had for a while). With that in mind, I don't think there's much to lose from changing the config names Grumble uses already. Users of the current builds have no way to set them, right? Also, I don't think anyone really uses it in production, though I could be wrong, of course. So, there's no shame in dropping backwards compatibility with the freeze files.

[rubenseyer]

Sorry for being inactive. I've been busy with other stuff and the ACME spec doesn't seem to be making a lot of progress anyway. I had some patches on my computer waiting for consensus that I am uploading now.

Seems like I am outvoted on the CamelCase point so I've dropped the compatibility layer design entirely and made grumble.ini configuration a superset of murmur.ini. (I still left some convenience features enabled in the .ini parser regarding quote escaping and boolean keys etc.).

I did keep the warnings for unsupported common Murmur configuration around but they are separate and easily removed now if you don't want them.

The branch history is a bit long now but I want to avoid rewriting the commits with issue mentions until the squash and merge.

[quite]

Is @mkrautz waiting for changes by @rubenseyer here, still? I only mean well. Looks like this PR could solve a number issues related to not having a proper configuration file system in place!

[poVoq]

Sorry to ask, but is there any news on this? I am about to setup a mumble server again, and would really like to use Grumble.

[actown]

Given this PR is a few years out of date, if you would like to rebase it on master and give it a test that would be awesome. If I have time this weekend I can take a look myself.

[poVoq]

If I have time this weekend I can take a look myself.

Would be awesome if you could have a look at it. Anything basic will do, if some channels could be preconfigured like in uMurmur that's totally sufficient.

[rubenseyer]

Wow, I had forgotten about this! I'm still around, but probably too busy to do anything too soon. I'll try to rebase and update everything if I get time and no one else beats me.

Looking through what has happened since I'm gone, here's a few things off the top of my head:

• Check that #51 works (also more on Grumble-specific configuration later)
• #52 presents a design decision. No user in normal (that is, running Grumble itself) operation can currently set configuration in that way (with RPC unimplemented). However, unless you are careful, the settings persist in the freezelog and this may cause unexpected behaviour (or at least it worked that way back in 2018). The .ini files take care to not persist, but they will also override settings set in the freezelog on load for this reason (in case old configuration was left in), because I think that's more obvious for the user. We should think about what we want to happen.
• #55 conflicts with this one (that's right, server password support was hidden here all along!), so we'll have to merge those changes -- notably, you can't be Murmur.ini-compatible and stored hashed passwords like for SU, so we need to special-case and hash incoming passwords from the config file (which is pointless because you'll still have them unhashed on disk anyway) or special-case and allow unhashed passwords to accept Murmur behaviour (but you might not like that for security reasons)
• As you can probably read from the history of this pull request, these features have gone through a lot of redesign in terms of scope. I went in with some suggestions for enhancements for the configuration (json support, new configuration keys unsupported by Murmur, virtual server config without rpc) but in the end it was decided to scale back so that it essentially is literally murmur.ini. That meant no Grumble-specific configuration and renaming every key to Murmur names. This breaks every single existing server, so that configuration must be generated again, but when consensus made that decision in 2018 the reasoning was that nobody uses Grumble in production anyway. If someone actually is using this for real nowadays we ought to avoid renaming the keys, and probably just bring back the murmur.ini translation layer (but that would just be a matter of dropping some later commits, because I kept everything!). (Also, if those other features sound interesting this time and we consider that Grumble may diverge from Murmur in configuration feature set I might consider fixing the other suggestions too.)
• Lastly, I added myself to AUTHORS because every contributor back then had done so, but this file has since gone entirely unused. I'd suggest dropping that commit from this pull request and just update it separately with every contributor, or getting rid of it.

Sorry for dropping a book on you, but I thought I'd write down some of these considerations so we can think about the decisions involved. Merging this is not far away in terms of code changes but it is an entirely new part to Grumble so it was always gonna be a matter of design problems first and foremost.

[actown]

It sounds like this PR brings up a bunch of questions that we should figure out the best way to handle.

• I’m unsure if anyone is using Grumble in production and I’m unsure of the best way to determine that. Given the state of this repo I suspect that most people are still using the main server provided by their local package repo or via the static builds on the project website.
• What’s the goal of this project? I have my views, please don’t take them as gospel. Personally I’d like to see this project be the default server backend for Mumble (a long way to go for this), splitting out the server from the client+QT base. Ideally doing so would allow the server and client to move in a direction that works best for each component. This also brings up some points that will need to be considered: Do we keep compatibility with older configs? ICE (please no)? gRPC (yes please)? Most of these can be worked around but will require some work. Would love to hear some opinions of this.
• AUTHORS is fine, the main project uses this so we might as well follow suit. This should be its own task to get the file caught up.

[rubenseyer]

I think my views on this have remained unchanged since I first began this PR --- in essence, I agree.

I think that for Grumble to become a default choice we will need compatibility to allow easy migration (but compatibility need not mean that we enforce Grumble to have exactly the same feature set as Murmur). My solution for that was the translation layer which was a compromise between not breaking existing Grumble servers and allowing compatibility with existing configuration, because I have no problem with the real Grumble config not being exactly the same as murmur.ini (and apparently neither did the original author of serverconf) but I understand it seems like a lot of trouble just because we don't want to rename config keys.

(Also, we clearly would need gRPC for any major production user to consider switching. That was my idea for the next step after this PR back in 2018.)

[rubenseyer]

I rebased everything and re-organized the slew of WIP commits into something more logical. I have changed no design decisions here compared to where I left off in 2018, so I do not expect this to be merged until everyone is satisfied on those. Please do test and review the changes, because I have only built it and nothing more.

Some notes:

• Reading the code again made me realize I was wrong about what happens with #52... Configuration set using this method will indeed persist, but grumble.ini will not override them.
• #55 should work if the password given in the config is in hashed format. Again, we should think about how we want this to work, because this breaks murmur.ini compatibility.

[streaps]

I get this error on first start. The message is also missing a "\n". Unable to open log file (/home/mumble/.grumble/grumble.log): open : no such file or directory

[streaps]

I'm not able to add @all permissions to the root channel, it only adds another SuperUser everytime I try. Is this a new problem with this PR or a known issue? I also cannot start the server anymore. Could this be related?

$ ./grumble
[G] 2020/06/10 16:55:12.632307 Grumble
[G] 2020/06/10 16:55:12.633083 Using data directory: /home/mumble/.grumble
[G] 2020/06/10 16:55:12.633692 Loading server 1
panic: assignment to entry in nil map

goroutine 1 [running]:
main.(*Channel).Unfreeze(0xc0000f4360, 0xc0000f4120)
        /home/mumble/repos/grumble/cmd/grumble/freeze.go:272 +0x4d2
main.NewServerFromFrozen(0xd015f1, 0x1, 0xc0000bec80, 0x0, 0x0)
        /home/mumble/repos/grumble/cmd/grumble/freeze.go:451 +0x808
main.main()
        /home/mumble/repos/grumble/cmd/grumble/grumble.go:245 +0x1494

[rubenseyer]

I can reproduce the same crash on both master and this PR:

1. Log in as SuperUser (on master you need to "cheat" to do this)
2. Make any change to the @all acl
3. Stop the server
4. Fail to start the server

I have filed PR #71 to fix it, because it is not really pertinent to this one (which is bogged down in years of reviews anyway). You should be able to easily rebase those changes onto this one if you want to combine them immediately -- this shouldn't touch those areas too much. I'll rebase here later if the other one is merged.