[openstack-dev] [all] Lets not assume everyone is using the global `CONF` object (zaqar broken by latest keystoneclient release 1.0)
jamielennox at redhat.com
Mon Dec 22 06:02:53 UTC 2014
----- Original Message -----
> From: "Doug Hellmann" <doug at doughellmann.com>
> To: "OpenStack Development Mailing List (not for usage questions)" <openstack-dev at lists.openstack.org>
> Sent: Saturday, 20 December, 2014 12:07:59 AM
> Subject: Re: [openstack-dev] [all] Lets not assume everyone is using the global `CONF` object (zaqar broken by latest
> keystoneclient release 1.0)
> On Dec 19, 2014, at 7:17 AM, Flavio Percoco <flavio at redhat.com> wrote:
> > Greetings,
> > DISCLAIMER: The following comments are neither finger pointing the
> > author of this work nor the keystone team.
That was me.
> > RANT: We should really stop assuming everyone is using a global `CONF`
> > object. Moreover, we should really stop using it, especially in
> > libraries.
> > That said, here's a gentle note for all of us:
> > If I understood the flow of changes correctly, keystoneclient recently
> > introduced a auth_section option, which needs to be registered in
> > order for it to work properly. In keystoneclient, it's been correctly
> > added a function to register this option in a conf object.
> > keystonemiddleware was then updated to support the above and a call to
> > the register function was then added to the `auth_token` module.
> > The above, unfortunately, broke Zaqar's auth because Zaqar is not
> > using the global `CONF` object which means it has to register
> > keystonemiddleware's options itself. Since the option was registered
> > in the global conf instead of the conf object passed to
> > `AuthProtocol`, the new `auth_section` option is not bein registered
> > as keystoneclient excepts.
> > So, as a gentle reminder to everyone, please, lets not assume all
> > projects are using the global `CONF` object and make sure all libraries
> > provide a good way to register the required options. I think either
> > secretly registering options or exposing a function to let consumers
> > do so is fine.
> > I hate complaining without helping to solve the problem so, here's a
> > workaround to provide a, hopefully, better way to do this. Note that
> > this shouldn't be the definitive fix and that we also implemented a
> > workaround in zaqar as well.
That will fix the immediate problem - and i assume is fixing the issue that oslo.config sample config generator must not be picking up those options if it's not there.
> That change will fix the issue, but a better solution is to have the code in
> keystoneclient that wants the options handle the registration at runtime. It
> looks like keystoneclient/auth/conf.py:load_from_conf_options() is at least
> one place that’s needed, there may be others.
So auth_token middleware was never designed to work this way - but we can fix it to. The reason AuthProtocol.__init__ takes a conf dict (it's not an oslo.config.Cfg object) is to work with options being included via paste file, these are expected to be overrides of the global CONF object. Putting these options in paste.ini is something the keystone team has been advising against for a while now and my understanding from that was that we were close to everyone using the global CONF object.
Do you know if there are any other projects managing CONF this way? I too dislike the global CONF, however this is the only time i've seen a project work to not use it.
The problem with the proposed solution is that we are moving towards pluggable authentication in keystonemiddleware (and the clients). The auth_section option is the first called but the important option there is the auth_plugin option which specifies what sort of authentication to perform. The options that will be read/registered on CONF are then dependent on the plugin specified by auth_plugin. Handling this manually from Zaqar and having the correct options registered is going to be a pain.
Given that there are users, I'll have a look into making auth_token middleware actually accept a CONF object rather than require people to hack things through in the override dictionary.
> > Cheers,
> > Flavio
> > 
> > https://github.com/openstack/python-keystoneclient/blob/41afe3c963fa01f61b67c44e572eee34b0972382/keystoneclient/auth/conf.py#L20
> > 
> > https://github.com/openstack/python-keystoneclient/blob/41afe3c963fa01f61b67c44e572eee34b0972382/keystoneclient/auth/conf.py#L49
> > 
> > https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token.py#L356
> >  https://review.openstack.org/143063
> > --
> > @flaper87
> > Flavio Percoco
> > _______________________________________________
> > OpenStack-dev mailing list
> > OpenStack-dev at lists.openstack.org
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
More information about the OpenStack-dev