Re: [openstack-dev] [all] Lets not assume everyone is using the global `CONF` object (zaqar broken by latest keystoneclient release 1.0)
- Original Message - From: Doug Hellmann d...@doughellmann.com To: OpenStack Development Mailing List (not for usage questions) openstack-dev@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 keystoneclientrelease 1.0) On Dec 19, 2014, at 7:17 AM, Flavio Percoco fla...@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[0] option, which needs to be registered in order for it to work properly. In keystoneclient, it's been correctly added a function[1] to register this option in a conf object. keystonemiddleware was then updated to support the above and a call to the register function[1] was then added to the `auth_token` module[2]. 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[3] 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. Jamie Doug Cheers, Flavio [0] https://github.com/openstack/python-keystoneclient/blob/41afe3c963fa01f61b67c44e572eee34b0972382/keystoneclient/auth/conf.py#L20 [1] https://github.com/openstack/python-keystoneclient/blob/41afe3c963fa01f61b67c44e572eee34b0972382/keystoneclient/auth/conf.py#L49 [2] https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token.py#L356 [3] https://review.openstack.org/143063 -- @flaper87 Flavio Percoco ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org
[openstack-dev] [all] Lets not assume everyone is using the global `CONF` object (zaqar broken by latest keystoneclient release 1.0)
Greetings, DISCLAIMER: The following comments are neither finger pointing the author of this work nor the keystone team. 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[0] option, which needs to be registered in order for it to work properly. In keystoneclient, it's been correctly added a function[1] to register this option in a conf object. keystonemiddleware was then updated to support the above and a call to the register function[1] was then added to the `auth_token` module[2]. 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[3] 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. Cheers, Flavio [0] https://github.com/openstack/python-keystoneclient/blob/41afe3c963fa01f61b67c44e572eee34b0972382/keystoneclient/auth/conf.py#L20 [1] https://github.com/openstack/python-keystoneclient/blob/41afe3c963fa01f61b67c44e572eee34b0972382/keystoneclient/auth/conf.py#L49 [2] https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token.py#L356 [3] https://review.openstack.org/143063 -- @flaper87 Flavio Percoco pgpZwjK8HSrg1.pgp Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
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 fla...@redhat.com wrote: Greetings, DISCLAIMER: The following comments are neither finger pointing the author of this work nor the keystone team. 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[0] option, which needs to be registered in order for it to work properly. In keystoneclient, it's been correctly added a function[1] to register this option in a conf object. keystonemiddleware was then updated to support the above and a call to the register function[1] was then added to the `auth_token` module[2]. 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[3] 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 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. Doug Cheers, Flavio [0] https://github.com/openstack/python-keystoneclient/blob/41afe3c963fa01f61b67c44e572eee34b0972382/keystoneclient/auth/conf.py#L20 [1] https://github.com/openstack/python-keystoneclient/blob/41afe3c963fa01f61b67c44e572eee34b0972382/keystoneclient/auth/conf.py#L49 [2] https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token.py#L356 [3] https://review.openstack.org/143063 -- @flaper87 Flavio Percoco ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] Lets not assume everyone is using the global `CONF` object (zaqar broken by latest keystoneclient release 1.0)
On 19/12/14 09:07 -0500, Doug Hellmann wrote: On Dec 19, 2014, at 7:17 AM, Flavio Percoco fla...@redhat.com wrote: Greetings, DISCLAIMER: The following comments are neither finger pointing the author of this work nor the keystone team. 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[0] option, which needs to be registered in order for it to work properly. In keystoneclient, it's been correctly added a function[1] to register this option in a conf object. keystonemiddleware was then updated to support the above and a call to the register function[1] was then added to the `auth_token` module[2]. 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[3] 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 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. Fully agree! -- @flaper87 Flavio Percoco pgpYoDTaszEE1.pgp Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
