ZOOKEEPER-5050. Disable AdminServer and enhance documentation to highlight security considerations

[anmolnar]

No description provided.

[phunt]

shouldn't we err on the side of security instead? (see prev line comment)

[anmolnar]

what do you mean by that?

[phunt]

It's not clear to me (and I don't see here nor in the references JIRA) why we wouldn't just flip this and disable by default, include the excellent new docs you've added, and put the burden on the user to ensure the requisite security enforcement prior to enabling/overriding? I think this would be fine and "backward compatible" given the user can make a change via config at runtime - we could include such details in the release notes.

[anmolnar]

@eolivelli PTAL.

[anmolnar]

@phunt @eolivelli I've updated the ticket and the patch to disable Admin Server in the default configuration. Also modified the docs accordingly. PTAL.

[eolivelli]

LGTM

for the next major release we should disable this by default

let's ensure that we document the default behavior on each version

[anmolnar]

Thanks @eolivelli !

I think we can leave the default setting enabled on other branches and backport this patch without the code change.

[phunt]

lgtm - +1. Having this off by default with clear documentation on how to enable safely is great to see. Thanks Andor.

[anmolnar]

Thank you guys for the reviews. If I got it right, we should not make Admin Server disabled by default on branches other than the master. Which means I need to carefully backport this patch with the text "reversed" and referring to the enabled behavior.