mgr dashboard and rest api configs

mgr dashboard and rest api configs

[Sage Weil]

Hi all,

So we now have 2 services in mgr that present http[s] endpoints: the 
dashboard and the new 'restful' API.  Once 
https://github.com/ceph/ceph/pull/15405 is merged these will be set up by 
vstart.sh for each testing:

 $ MGR=2 ../src/vstart.sh -n -l ...
 ...
 started.  stop.sh to stop.  see out/* (e.g. 'tail -f out/????') for debug 
 output.

 dashboard urls:  http://127.0.0.1:41542/ http://127.0.0.1:41544/
 restful urls:    https://127.0.0.1:41543 https://127.0.0.1:41545
    user/pass:    admin / secret
 ...

The services are configured via the config-key interface:

gnit:build (wip-rest-test) 04:06 PM $ bin/ceph config-key list
[
    "mgr/x/dashboard/server_addr",
    "mgr/x/dashboard/server_port",
    "mgr/x/restful/cert",
    "mgr/x/restful/keys",
    "mgr/x/restful/pkey_file",
    "mgr/x/restful/server_addr",
    "mgr/x/restful/server_port",
    "mgr/y/dashboard/server_addr",
    "mgr/y/dashboard/server_port",
    "mgr/y/restful/cert",
    "mgr/y/restful/keys",
    "mgr/y/restful/pkey_file",
    "mgr/y/restful/server_addr",
    "mgr/y/restful/server_port"
]

Several comments, questions, as I think there's lots of room for 
improvement here:

- The dashboard is always http; the restful endpoint always https.  Should 
we make either/both of them support either?

- The crt and key for restful can either come from the 'cert' or 
'pkey' option, a file named by 'cert_file' or 'pkey_file', or read out of 
/etc/ceph.  The ceph-mgr package currently generates an unsigned cert and 
puts it in /etc/ceph for that purpose.  Should we instead make the restful 
service generate its own key and store it on startup if it is not stored 
on disk?  I'm not a big fan of putting this in /etc/ceph.

- What if you want the crt/key to be shared across mgr daemons?  This 
would make sense if you have a load balancer (or some network indirection 
like that provided by kubernetes) in front of it.  In that case, should we 
allow these options to come from, say, either mgr/$id/$module/$option or 
mgr/*/$module/$option?  Or restructure the namespace so that it's either 
mgr/$module/$option or mgr.$id/$module/$option (more like the config 
sections work where they are [mgr] or [mgr.$id])?  [1]

- s/cert/crt/ and s/pkey/key/?

Comments welcome!
sage


[1] How should config options appear in config-key?  The current proposal 
at http://pad.ceph.com/p/ceph-conf-kv-store suggests config/$type/option 
or config/$type.$id/option.  Where do mgr modules fit into this namespace?

   config/mgr/$module/$option
   config/mgr.x/$module/$option

Or should they maybe not live under the config/ prefix that the "regular" 
ceph options will live under? ?

Re: mgr dashboard and rest api configs

[John Spray]

On Thu, Jun 1, 2017 at 9:23 PM, Sage Weil <sage@redhat.com> wrote:
> Hi all,
>
> So we now have 2 services in mgr that present http[s] endpoints: the
> dashboard and the new 'restful' API.  Once
> https://github.com/ceph/ceph/pull/15405 is merged these will be set up by
> vstart.sh for each testing:
>
>  $ MGR=2 ../src/vstart.sh -n -l ...
>  ...
>  started.  stop.sh to stop.  see out/* (e.g. 'tail -f out/????') for debug
>  output.
>
>  dashboard urls:  http://127.0.0.1:41542/ http://127.0.0.1:41544/
>  restful urls:    https://127.0.0.1:41543 https://127.0.0.1:41545
>     user/pass:    admin / secret
>  ...

I spent a few minutes there wondering why the authentication wasn't
working until I realise that the "/" rest endpoint gives you that "Use
\"ceph tell mgr create_key <key>\" to create a key pair," prompt even
if you are already authenticated!

> The services are configured via the config-key interface:
>
> gnit:build (wip-rest-test) 04:06 PM $ bin/ceph config-key list
> [
>     "mgr/x/dashboard/server_addr",
>     "mgr/x/dashboard/server_port",
>     "mgr/x/restful/cert",
>     "mgr/x/restful/keys",
>     "mgr/x/restful/pkey_file",
>     "mgr/x/restful/server_addr",
>     "mgr/x/restful/server_port",
>     "mgr/y/dashboard/server_addr",
>     "mgr/y/dashboard/server_port",
>     "mgr/y/restful/cert",
>     "mgr/y/restful/keys",
>     "mgr/y/restful/pkey_file",
>     "mgr/y/restful/server_addr",
>     "mgr/y/restful/server_port"
> ]

Aargh!  All the keys are prefixed with the daemon name!  I missed this
change (dc15cd60) when it went in.

We should undo that.  The idea of the interface for modules is that
they can store things and then retrieve them at any time, including
when they have failed over to another daemon.  Modules are very much
*not* meant to have local state on particular daemons (apart from this
particular special case we're discussing about certificates for things
that do HTTP).

>
> Several comments, questions, as I think there's lots of room for
> improvement here:
>
> - The dashboard is always http; the restful endpoint always https.  Should
> we make either/both of them support either?
>
> - The crt and key for restful can either come from the 'cert' or
> 'pkey' option, a file named by 'cert_file' or 'pkey_file', or read out of
> /etc/ceph.  The ceph-mgr package currently generates an unsigned cert and
> puts it in /etc/ceph for that purpose.  Should we instead make the restful
> service generate its own key and store it on startup if it is not stored
> on disk?  I'm not a big fan of putting this in /etc/ceph.

I would really prefer modules to be self contained with this kind of thing:
 - do their own initial setup, rather than having code in the packaging
 - store their stuff in the get_config/set_config stuff and not on the
local filesystem

iirc the main reason that the restful module wanted to put the cert on
the local filesystem at all was just that it was using a library that
couldn't consume a certificate any way other than a file path?

> - What if you want the crt/key to be shared across mgr daemons?  This
> would make sense if you have a load balancer (or some network indirection
> like that provided by kubernetes) in front of it.  In that case, should we
> allow these options to come from, say, either mgr/$id/$module/$option or
> mgr/*/$module/$option?  Or restructure the namespace so that it's either
> mgr/$module/$option or mgr.$id/$module/$option (more like the config
> sections work where they are [mgr] or [mgr.$id])?  [1]

I think we should avoid going down the path of complex configuration
paths (like having a /mgr/foo/ and /mgr/*/ and a rule about which has
precedence) unless there are cases where it seems absolutely
necessary.

Assuming we roll back the change that prefixed every key with the
daemon name, modules are free to do their own prefixing for any
settings that they want to be per-daemon.  To enable it we just need
to make sure the module has a way of finding out the name of the
daemon where it's currently running.

John

>
> - s/cert/crt/ and s/pkey/key/?
>
> Comments welcome!
> sage
>
>
> [1] How should config options appear in config-key?  The current proposal
> at http://pad.ceph.com/p/ceph-conf-kv-store suggests config/$type/option
> or config/$type.$id/option.  Where do mgr modules fit into this namespace?
>
>    config/mgr/$module/$option
>    config/mgr.x/$module/$option
>
> Or should they maybe not live under the config/ prefix that the "regular"
> ceph options will live under? ?
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mgr dashboard and rest api configs

[Sage Weil]

On Thu, 1 Jun 2017, John Spray wrote:
> On Thu, Jun 1, 2017 at 9:23 PM, Sage Weil <sage@redhat.com> wrote:
> > Hi all,
> >
> > So we now have 2 services in mgr that present http[s] endpoints: the
> > dashboard and the new 'restful' API.  Once
> > https://github.com/ceph/ceph/pull/15405 is merged these will be set up by
> > vstart.sh for each testing:
> >
> >  $ MGR=2 ../src/vstart.sh -n -l ...
> >  ...
> >  started.  stop.sh to stop.  see out/* (e.g. 'tail -f out/????') for debug
> >  output.
> >
> >  dashboard urls:  http://127.0.0.1:41542/ http://127.0.0.1:41544/
> >  restful urls:    https://127.0.0.1:41543 https://127.0.0.1:41545
> >     user/pass:    admin / secret
> >  ...
> 
> I spent a few minutes there wondering why the authentication wasn't
> working until I realise that the "/" rest endpoint gives you that "Use
> \"ceph tell mgr create_key <key>\" to create a key pair," prompt even
> if you are already authenticated!
> 
> > The services are configured via the config-key interface:
> >
> > gnit:build (wip-rest-test) 04:06 PM $ bin/ceph config-key list
> > [
> >     "mgr/x/dashboard/server_addr",
> >     "mgr/x/dashboard/server_port",
> >     "mgr/x/restful/cert",
> >     "mgr/x/restful/keys",
> >     "mgr/x/restful/pkey_file",
> >     "mgr/x/restful/server_addr",
> >     "mgr/x/restful/server_port",
> >     "mgr/y/dashboard/server_addr",
> >     "mgr/y/dashboard/server_port",
> >     "mgr/y/restful/cert",
> >     "mgr/y/restful/keys",
> >     "mgr/y/restful/pkey_file",
> >     "mgr/y/restful/server_addr",
> >     "mgr/y/restful/server_port"
> > ]
> 
> Aargh!  All the keys are prefixed with the daemon name!  I missed this
> change (dc15cd60) when it went in.
> 
> We should undo that.  The idea of the interface for modules is that
> they can store things and then retrieve them at any time, including
> when they have failed over to another daemon.  Modules are very much
> *not* meant to have local state on particular daemons (apart from this
> particular special case we're discussing about certificates for things
> that do HTTP).

Okay, but I think all of these options (except 'keys') fall into that 
special case.  We can make get_config() look at mgr/$id/$module/$option 
and, if it doesn't exist, then look at mgr/$module/$option... or make a 
special variant of get_config() for the possibly-mgr-localized options?

> > Several comments, questions, as I think there's lots of room for
> > improvement here:
> >
> > - The dashboard is always http; the restful endpoint always https.  Should
> > we make either/both of them support either?
> >
> > - The crt and key for restful can either come from the 'cert' or
> > 'pkey' option, a file named by 'cert_file' or 'pkey_file', or read out of
> > /etc/ceph.  The ceph-mgr package currently generates an unsigned cert and
> > puts it in /etc/ceph for that purpose.  Should we instead make the restful
> > service generate its own key and store it on startup if it is not stored
> > on disk?  I'm not a big fan of putting this in /etc/ceph.
> 
> I would really prefer modules to be self contained with this kind of thing:
>  - do their own initial setup, rather than having code in the packaging
>  - store their stuff in the get_config/set_config stuff and not on the
> local filesystem
> 
> iirc the main reason that the restful module wanted to put the cert on
> the local filesystem at all was just that it was using a library that
> couldn't consume a certificate any way other than a file path?

That part I fixed with NamedTemporaryFile in
	https://github.com/liewegas/ceph/commit/c169c8fd461bf94459cd98b658d2a2d8bf42bca6

Was there also a python module dependency problem?  If not, it seems much 
more natural for the module to create the crt/key pair the first time it 
stats if it's not present...

> > - What if you want the crt/key to be shared across mgr daemons?  This
> > would make sense if you have a load balancer (or some network indirection
> > like that provided by kubernetes) in front of it.  In that case, should we
> > allow these options to come from, say, either mgr/$id/$module/$option or
> > mgr/*/$module/$option?  Or restructure the namespace so that it's either
> > mgr/$module/$option or mgr.$id/$module/$option (more like the config
> > sections work where they are [mgr] or [mgr.$id])?  [1]
> 
> I think we should avoid going down the path of complex configuration
> paths (like having a /mgr/foo/ and /mgr/*/ and a rule about which has
> precedence) unless there are cases where it seems absolutely
> necessary.

Like this one?
 
> Assuming we roll back the change that prefixed every key with the
> daemon name, modules are free to do their own prefixing for any
> settings that they want to be per-daemon.  To enable it we just need
> to make sure the module has a way of finding out the name of the
> daemon where it's currently running.

The reason why it's there now is that (almost) every config key we've used 
so far need to be able to be mgr-specific.  Do you have an alternative 
proposal (besides trying mgr/$id/$module/$option or 
mgr.$id/$Module/$option and then mgr/$module/$option)?

sage

Re: mgr dashboard and rest api configs

[John Spray]

On Thu, Jun 1, 2017 at 11:16 PM, Sage Weil <sweil@redhat.com> wrote:
> On Thu, 1 Jun 2017, John Spray wrote:
>> On Thu, Jun 1, 2017 at 9:23 PM, Sage Weil <sage@redhat.com> wrote:
>> > Hi all,
>> >
>> > So we now have 2 services in mgr that present http[s] endpoints: the
>> > dashboard and the new 'restful' API.  Once
>> > https://github.com/ceph/ceph/pull/15405 is merged these will be set up by
>> > vstart.sh for each testing:
>> >
>> >  $ MGR=2 ../src/vstart.sh -n -l ...
>> >  ...
>> >  started.  stop.sh to stop.  see out/* (e.g. 'tail -f out/????') for debug
>> >  output.
>> >
>> >  dashboard urls:  http://127.0.0.1:41542/ http://127.0.0.1:41544/
>> >  restful urls:    https://127.0.0.1:41543 https://127.0.0.1:41545
>> >     user/pass:    admin / secret
>> >  ...
>>
>> I spent a few minutes there wondering why the authentication wasn't
>> working until I realise that the "/" rest endpoint gives you that "Use
>> \"ceph tell mgr create_key <key>\" to create a key pair," prompt even
>> if you are already authenticated!
>>
>> > The services are configured via the config-key interface:
>> >
>> > gnit:build (wip-rest-test) 04:06 PM $ bin/ceph config-key list
>> > [
>> >     "mgr/x/dashboard/server_addr",
>> >     "mgr/x/dashboard/server_port",
>> >     "mgr/x/restful/cert",
>> >     "mgr/x/restful/keys",
>> >     "mgr/x/restful/pkey_file",
>> >     "mgr/x/restful/server_addr",
>> >     "mgr/x/restful/server_port",
>> >     "mgr/y/dashboard/server_addr",
>> >     "mgr/y/dashboard/server_port",
>> >     "mgr/y/restful/cert",
>> >     "mgr/y/restful/keys",
>> >     "mgr/y/restful/pkey_file",
>> >     "mgr/y/restful/server_addr",
>> >     "mgr/y/restful/server_port"
>> > ]
>>
>> Aargh!  All the keys are prefixed with the daemon name!  I missed this
>> change (dc15cd60) when it went in.
>>
>> We should undo that.  The idea of the interface for modules is that
>> they can store things and then retrieve them at any time, including
>> when they have failed over to another daemon.  Modules are very much
>> *not* meant to have local state on particular daemons (apart from this
>> particular special case we're discussing about certificates for things
>> that do HTTP).
>
> Okay, but I think all of these options (except 'keys') fall into that
> special case.  We can make get_config() look at mgr/$id/$module/$option
> and, if it doesn't exist, then look at mgr/$module/$option... or make a
> special variant of get_config() for the possibly-mgr-localized options?

Having that get_config variant would be a totally reasonable helper
function on the python side, but I don't think it's essential enough
to make it a first class part of the module interface, as it's just a
string prefix to the key.

>> > Several comments, questions, as I think there's lots of room for
>> > improvement here:
>> >
>> > - The dashboard is always http; the restful endpoint always https.  Should
>> > we make either/both of them support either?
>> >
>> > - The crt and key for restful can either come from the 'cert' or
>> > 'pkey' option, a file named by 'cert_file' or 'pkey_file', or read out of
>> > /etc/ceph.  The ceph-mgr package currently generates an unsigned cert and
>> > puts it in /etc/ceph for that purpose.  Should we instead make the restful
>> > service generate its own key and store it on startup if it is not stored
>> > on disk?  I'm not a big fan of putting this in /etc/ceph.
>>
>> I would really prefer modules to be self contained with this kind of thing:
>>  - do their own initial setup, rather than having code in the packaging
>>  - store their stuff in the get_config/set_config stuff and not on the
>> local filesystem
>>
>> iirc the main reason that the restful module wanted to put the cert on
>> the local filesystem at all was just that it was using a library that
>> couldn't consume a certificate any way other than a file path?
>
> That part I fixed with NamedTemporaryFile in
>         https://github.com/liewegas/ceph/commit/c169c8fd461bf94459cd98b658d2a2d8bf42bca6
>
> Was there also a python module dependency problem?  If not, it seems much
> more natural for the module to create the crt/key pair the first time it
> stats if it's not present...
>
>> > - What if you want the crt/key to be shared across mgr daemons?  This
>> > would make sense if you have a load balancer (or some network indirection
>> > like that provided by kubernetes) in front of it.  In that case, should we
>> > allow these options to come from, say, either mgr/$id/$module/$option or
>> > mgr/*/$module/$option?  Or restructure the namespace so that it's either
>> > mgr/$module/$option or mgr.$id/$module/$option (more like the config
>> > sections work where they are [mgr] or [mgr.$id])?  [1]
>>
>> I think we should avoid going down the path of complex configuration
>> paths (like having a /mgr/foo/ and /mgr/*/ and a rule about which has
>> precedence) unless there are cases where it seems absolutely
>> necessary.
>
> Like this one?

What I meant to say was: I think we should avoid doing this *as part
of the way the module config options work in general* -- without us
having this conversation at all, individual modules can still have
multiple paths for something and apply their own rules for which
option they prefer to respect when they have two ways of setting it.

>> Assuming we roll back the change that prefixed every key with the
>> daemon name, modules are free to do their own prefixing for any
>> settings that they want to be per-daemon.  To enable it we just need
>> to make sure the module has a way of finding out the name of the
>> daemon where it's currently running.
>
> The reason why it's there now is that (almost) every config key we've used
> so far need to be able to be mgr-specific.  Do you have an alternative
> proposal (besides trying mgr/$id/$module/$option or
> mgr.$id/$Module/$option and then mgr/$module/$option)?

My proposal is mgr/$module/<whatever the module wants>
...which in practice for TLS certs would be mgr/$module/$id/$option

Then, enable the modules to retrieve their current daemon's config by
giving them a function that tells them the name of the currently
running daemon.

One day, it would be nice to have modules consume their config from
the new key/val config store on the mon.  Then, they could just have
whatever level of cleverness was available for configuration in
general for those settings, and the current "get_config" stuff would
be exclusively for persistent state as opposed to user-input config.

John

Re: mgr dashboard and rest api configs

[Boris Ranto]

On Thu, 2017-06-01 at 22:16 +0000, Sage Weil wrote:
> On Thu, 1 Jun 2017, John Spray wrote:
> > On Thu, Jun 1, 2017 at 9:23 PM, Sage Weil <sage@redhat.com> wrote:
> > > Hi all,
> > >
> > > So we now have 2 services in mgr that present http[s] endpoints:
> the
> > > dashboard and the new 'restful' API.  Once
> > > https://github.com/ceph/ceph/pull/15405 is merged these will be
> set up by
> > > vstart.sh for each testing:
> > >
> > >  $ MGR=2 ../src/vstart.sh -n -l ...
> > >  ...
> > >  started.  stop.sh to stop.  see out/* (e.g. 'tail -f out/????')
> for debug
> > >  output.
> > >
> > >  dashboard urls:  http://127.0.0.1:41542/ http://127.0.0.1:41544/
> > >  restful urls:    https://127.0.0.1:41543 https://127.0.0.1:41545
> > >     user/pass:    admin / secret
> > >  ...
> > 
> > I spent a few minutes there wondering why the authentication wasn't
> > working until I realise that the "/" rest endpoint gives you that
> "Use
> > \"ceph tell mgr create_key <key>\" to create a key pair," prompt
> even
> > if you are already authenticated!

Yeah, the / (and /doc) endpoint does not take (nor prompt for)
authentication. That is by design, it does not contain any private
information. There can also be more than one user and the auth message
can help any new users.

> > 
> > > The services are configured via the config-key interface:
> > >
> > > gnit:build (wip-rest-test) 04:06 PM $ bin/ceph config-key list
> > > [
> > >     "mgr/x/dashboard/server_addr",
> > >     "mgr/x/dashboard/server_port",
> > >     "mgr/x/restful/cert",
> > >     "mgr/x/restful/keys",
> > >     "mgr/x/restful/pkey_file",
> > >     "mgr/x/restful/server_addr",
> > >     "mgr/x/restful/server_port",
> > >     "mgr/y/dashboard/server_addr",
> > >     "mgr/y/dashboard/server_port",
> > >     "mgr/y/restful/cert",
> > >     "mgr/y/restful/keys",
> > >     "mgr/y/restful/pkey_file",
> > >     "mgr/y/restful/server_addr",
> > >     "mgr/y/restful/server_port"
> > > ]
> > 
> > Aargh!  All the keys are prefixed with the daemon name!  I missed
> this
> > change (dc15cd60) when it went in.
> > 
> > We should undo that.  The idea of the interface for modules is that
> > they can store things and then retrieve them at any time, including
> > when they have failed over to another daemon.  Modules are very
> much
> > *not* meant to have local state on particular daemons (apart from
> this
> > particular special case we're discussing about certificates for
> things
> > that do HTTP).
> 
> Okay, but I think all of these options (except 'keys') fall into
> that 
> special case.  We can make get_config() look at
> mgr/$id/$module/$option 
> and, if it doesn't exist, then look at mgr/$module/$option... or make
> a 
> special variant of get_config() for the possibly-mgr-localized
> options?
> 
> > > Several comments, questions, as I think there's lots of room for
> > > improvement here:
> > >
> > > - The dashboard is always http; the restful endpoint always
> https.  Should
> > > we make either/both of them support either?
> > >
> > - The crt and key for restful can either come from the 'cert' or
> > > 'pkey' option, a file named by 'cert_file' or 'pkey_file', or
> read out of
> > > /etc/ceph.  The ceph-mgr package currently generates an unsigned
> cert and
> > > puts it in /etc/ceph for that purpose.  Should we instead make
> the restful
> > > service generate its own key and store it on startup if it is not
> stored
> > > on disk?  I'm not a big fan of putting this in /etc/ceph.
> > 
> > I would really prefer modules to be self contained with this kind
> of thing:
> >  - do their own initial setup, rather than having code in the
> packaging
> >  - store their stuff in the get_config/set_config stuff and not on
> the
> > local filesystem
> > 
> > iirc the main reason that the restful module wanted to put the cert
> on
> > the local filesystem at all was just that it was using a library
> that
> > couldn't consume a certificate any way other than a file path?
> 
> That part I fixed with NamedTemporaryFile in
>         https://github.com/liewegas/ceph/commit/c169c8fd461bf94459cd9
> 8b658d2a2d8bf42bca6
> 
> Was there also a python module dependency problem?  If not, it seems
> much 
> more natural for the module to create the crt/key pair the first time
> it 
> stats if it's not present...
> 

I had something like that before, see commit 

https://github.com/ceph/ceph/pull/14457/commits/92fa210aaa87fcc47b2491f
a44f029f7426cd7b9

or

https://github.com/ceph/ceph/pull/14457/commits/f59e9bf0bbda870311c6084
74b9aad63f4323c43

that removed/changed it. I had a couple more implementations that did
all sorts of things. I later removed all of them and went with as
simple solution as possible for two reasons:

- the simple straight-forward solution is usually least likely to bite
us in the future
- it seemed almost impossible to get an agreement on how a more
sophisticated solution should actually look like (it felt like I was
going in the circles)

The one removed in the first linked patch did not require any special
dependencies (make_ssl_devcert is part of werkzeug). The other linked
one required pyOpenSSL which was kinda bad as afaik, pyOpenSSL is not
being properly maintained.

> > > - What if you want the crt/key to be shared across mgr daemons? 
> This
> > > would make sense if you have a load balancer (or some network
> indirection
> > > like that provided by kubernetes) in front of it.  In that case,
> should we
> > > allow these options to come from, say, either
> mgr/$id/$module/$option or
> > > mgr/*/$module/$option?  Or restructure the namespace so that it's
> either
> > > mgr/$module/$option or mgr.$id/$module/$option (more like the
> config
> > > sections work where they are [mgr] or [mgr.$id])?  [1]
> > 
> > I think we should avoid going down the path of complex
> configuration
> > paths (like having a /mgr/foo/ and /mgr/*/ and a rule about which
> has
> > precedence) unless there are cases where it seems absolutely
> > necessary.
> 
> Like this one?
>  
> > Assuming we roll back the change that prefixed every key with the
> > daemon name, modules are free to do their own prefixing for any
> > settings that they want to be per-daemon.  To enable it we just
> need
> > to make sure the module has a way of finding out the name of the
> > daemon where it's currently running.
> 
> The reason why it's there now is that (almost) every config key we've
> used 
> so far need to be able to be mgr-specific.  Do you have an
> alternative 
> proposal (besides trying mgr/$id/$module/$option or 
> mgr.$id/$Module/$option and then mgr/$module/$option)?
> 
> sage

Could we maybe have helper functions for each? i.e. have something like

{get,set}_config(_json)
{get,set}_instance_config(_json)

or the other way around? I think that would make it the most clear.

-boris

Re: mgr dashboard and rest api configs

[Sage Weil]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1463 bytes --]

On Fri, 2 Jun 2017, Boris Ranto wrote:
> On Thu, 2017-06-01 at 22:16 +0000, Sage Weil wrote:
> > That part I fixed with NamedTemporaryFile in
> >         https://github.com/liewegas/ceph/commit/c169c8fd461bf94459cd9
> > 8b658d2a2d8bf42bca6
> > 
> > Was there also a python module dependency problem?  If not, it seems 
> > much  more natural for the module to create the crt/key pair the first 
> > time it  stats if it's not present...
> > 
> 
> I had something like that before, see commit 
> 
> https://github.com/ceph/ceph/pull/14457/commits/92fa210aaa87fcc47b2491f
> a44f029f7426cd7b9
> 
> or
> 
> https://github.com/ceph/ceph/pull/14457/commits/f59e9bf0bbda870311c6084
> 74b9aad63f4323c43
> 
> that removed/changed it. I had a couple more implementations that did
> all sorts of things. I later removed all of them and went with as
> simple solution as possible for two reasons:
> 
> - the simple straight-forward solution is usually least likely to bite
> us in the future
> - it seemed almost impossible to get an agreement on how a more
> sophisticated solution should actually look like (it felt like I was
> going in the circles)
> 
> The one removed in the first linked patch did not require any special
> dependencies (make_ssl_devcert is part of werkzeug). The other linked
> one required pyOpenSSL which was kinda bad as afaik, pyOpenSSL is not
> being properly maintained.

I like the make_ssl_devcert approach personally... John?

sage

Re: mgr dashboard and rest api configs

[Sage Weil]

On Fri, 2 Jun 2017, John Spray wrote:
> >> Assuming we roll back the change that prefixed every key with the
> >> daemon name, modules are free to do their own prefixing for any
> >> settings that they want to be per-daemon.  To enable it we just need
> >> to make sure the module has a way of finding out the name of the
> >> daemon where it's currently running.
> >
> > The reason why it's there now is that (almost) every config key we've used
> > so far need to be able to be mgr-specific.  Do you have an alternative
> > proposal (besides trying mgr/$id/$module/$option or
> > mgr.$id/$Module/$option and then mgr/$module/$option)?
> 
> My proposal is mgr/$module/<whatever the module wants>
> ...which in practice for TLS certs would be mgr/$module/$id/$option
> 
> Then, enable the modules to retrieve their current daemon's config by
> giving them a function that tells them the name of the currently
> running daemon.

That sounds fine to me.

> One day, it would be nice to have modules consume their config from
> the new key/val config store on the mon.  Then, they could just have
> whatever level of cleverness was available for configuration in
> general for those settings, and the current "get_config" stuff would
> be exclusively for persistent state as opposed to user-input config.

FWIW my hope here was to adopt a convention here that matched what we were 
going to do for config.  That's why there is the global -> mgr -> mgr.x 
hierarchy thing.  That might be a fool's errand, though, since we 
(will) have a well-defined set of config options, whereas the module 
options are module-defined.

Anyway, I think the suggestion currently is for

    "mgr/dashboard/x/server_addr",
    "mgr/dashboard/x/server_port",
    "mgr/dashboard/y/server_addr",
    "mgr/dashboard/y/server_port",
    "mgr/restful/keys",
    "mgr/restful/x/crt",
    "mgr/restful/x/key",
    "mgr/restful/x/server_addr",
    "mgr/restful/x/server_port",
    "mgr/restful/y/crt",
    "mgr/restful/y/key",
    "mgr/restful/y/server_addr",
    "mgr/restful/y/server_port"

?

Although I'd probably suggest also changing

    "mgr/restful/keys",

to separate per-key keys, like

    "mgr/restful/keys/$user",

(Also, the 'ceph tell mgr.x create_key $name' thing concerns me a bit 
that it's unclear this is for the restful module.  Should we have modules 
prefix their commands by module name by convention where it makes sense?  
e.g., 'ceph tell mgr.x restful create_key $name'?)

sage

Re: mgr dashboard and rest api configs

[John Spray]

On Fri, Jun 2, 2017 at 2:35 PM, Sage Weil <sweil@redhat.com> wrote:
> On Fri, 2 Jun 2017, Boris Ranto wrote:
>> On Thu, 2017-06-01 at 22:16 +0000, Sage Weil wrote:
>> > That part I fixed with NamedTemporaryFile in
>> >         https://github.com/liewegas/ceph/commit/c169c8fd461bf94459cd9
>> > 8b658d2a2d8bf42bca6
>> >
>> > Was there also a python module dependency problem?  If not, it seems
>> > much  more natural for the module to create the crt/key pair the first
>> > time it  stats if it's not present...
>> >
>>
>> I had something like that before, see commit
>>
>> https://github.com/ceph/ceph/pull/14457/commits/92fa210aaa87fcc47b2491f
>> a44f029f7426cd7b9
>>
>> or
>>
>> https://github.com/ceph/ceph/pull/14457/commits/f59e9bf0bbda870311c6084
>> 74b9aad63f4323c43
>>
>> that removed/changed it. I had a couple more implementations that did
>> all sorts of things. I later removed all of them and went with as
>> simple solution as possible for two reasons:
>>
>> - the simple straight-forward solution is usually least likely to bite
>> us in the future
>> - it seemed almost impossible to get an agreement on how a more
>> sophisticated solution should actually look like (it felt like I was
>> going in the circles)
>>
>> The one removed in the first linked patch did not require any special
>> dependencies (make_ssl_devcert is part of werkzeug). The other linked
>> one required pyOpenSSL which was kinda bad as afaik, pyOpenSSL is not
>> being properly maintained.
>
> I like the make_ssl_devcert approach personally... John?

I just had a look at the werkzeug code and it seems to be using
pyOpenSSL too under the hood, so I guess has same issue as the other
approach?

If pyOpenSSL is not viable, maybe invoking the openssl CLI from inside
the module would still be neater than having the module depend on
packaging to do it?

John

>
> sage

Re: mgr dashboard and rest api configs

[John Spray]

On Fri, Jun 2, 2017 at 2:41 PM, Sage Weil <sweil@redhat.com> wrote:
> On Fri, 2 Jun 2017, John Spray wrote:
>> >> Assuming we roll back the change that prefixed every key with the
>> >> daemon name, modules are free to do their own prefixing for any
>> >> settings that they want to be per-daemon.  To enable it we just need
>> >> to make sure the module has a way of finding out the name of the
>> >> daemon where it's currently running.
>> >
>> > The reason why it's there now is that (almost) every config key we've used
>> > so far need to be able to be mgr-specific.  Do you have an alternative
>> > proposal (besides trying mgr/$id/$module/$option or
>> > mgr.$id/$Module/$option and then mgr/$module/$option)?
>>
>> My proposal is mgr/$module/<whatever the module wants>
>> ...which in practice for TLS certs would be mgr/$module/$id/$option
>>
>> Then, enable the modules to retrieve their current daemon's config by
>> giving them a function that tells them the name of the currently
>> running daemon.
>
> That sounds fine to me.
>
>> One day, it would be nice to have modules consume their config from
>> the new key/val config store on the mon.  Then, they could just have
>> whatever level of cleverness was available for configuration in
>> general for those settings, and the current "get_config" stuff would
>> be exclusively for persistent state as opposed to user-input config.
>
> FWIW my hope here was to adopt a convention here that matched what we were
> going to do for config.  That's why there is the global -> mgr -> mgr.x
> hierarchy thing.  That might be a fool's errand, though, since we
> (will) have a well-defined set of config options, whereas the module
> options are module-defined.
>
> Anyway, I think the suggestion currently is for
>
>     "mgr/dashboard/x/server_addr",
>     "mgr/dashboard/x/server_port",
>     "mgr/dashboard/y/server_addr",
>     "mgr/dashboard/y/server_port",
>     "mgr/restful/keys",
>     "mgr/restful/x/crt",
>     "mgr/restful/x/key",
>     "mgr/restful/x/server_addr",
>     "mgr/restful/x/server_port",
>     "mgr/restful/y/crt",
>     "mgr/restful/y/key",
>     "mgr/restful/y/server_addr",
>     "mgr/restful/y/server_port"
>
> ?
>
> Although I'd probably suggest also changing
>
>     "mgr/restful/keys",
>
> to separate per-key keys, like
>
>     "mgr/restful/keys/$user",
>
> (Also, the 'ceph tell mgr.x create_key $name' thing concerns me a bit
> that it's unclear this is for the restful module.  Should we have modules
> prefix their commands by module name by convention where it makes sense?
> e.g., 'ceph tell mgr.x restful create_key $name'?)

Yes, I think so.  We'll have cases like this where the commands are
obviously module-ish, and other cases where modules want to expose
first class/top-level things, so leaving it up to the module to put
its own name at the start seems like the way to go.

John

>
> sage