Re: [openstack-dev] Change in openstack/keystone[master]: Implement domain specific Identity backends

2013-08-08 Thread Mark McLoughlin 
Hi Henry,

On Tue, 2013-08-06 at 22:10 +0100, Henry Nash wrote:
 Hi Mark,
 
 Of particular interest are your views on the changes to 
 keystone/common/config.py.  The requirement is that we need to be able to 
 instantiate multiple conf objects (built from different sets of config 
 files).  We tried two approaches to this:
 
 https://review.openstack.org/#/c/39530/11 which attempts to keep the current 
 keystone config helper apps (register_bool() etc.) by passing on the conf 
 instance, and
 https://review.openstack.org/#/c/39530/12 which removes these helper apps and 
 just calls the methods on the conf itself (conf.register_opt())
 
 Both functionally work, but interested in your views on both approaches.

Definitely prefer more of the latter, since I've proposed it myself
previously :)

  https://review.openstack.org/4547

There are some common patterns of cfg usage which keystone is unusual in
not adopting:

  - declare options as a list, or multiple lists at the top of modules:

  foo_opts = [
  cfg.StrOpt('bar'),
  cfg.ListOpt('foo'),
  ]

  - declare options in the modules in which they're used, rather than 
having a single module which declares all options for the project. 
See this blueprint:
 
  https://blueprints.launchpad.net/nova/+spec/scope-config-opts

for where I moved all of the option declarations out of the 
nova.config module.

I recall this being a problem for keystone recently - I think it 
may have been a keystone.middleware module imported keystone.config 
which defined logging options which may have been defined 
elsewhere. This kind of thing is easier to avoid if the config 
options are scoped to the module which uses them.

  - iff the code in this module needs to only work with cfg.CONF, then 
register the options with cfg.CONF at the top of the module:

  CONF = cfg.CONF
  CONF.register_opts(foo_opts)

otherwise register the options before they're used:

  def bar(conf, ..):
  ...
  conf.register_opts(foo_opts)
  if conf.foo:
  ...

  def blaa(conf, ..):
  ...
  conf.register_opts(foo_opts)
  if conf.bar:
  ...

Hope that helps,
Mark.


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] Change in openstack/keystone[master]: Implement domain specific Identity backends

2013-08-06 Thread Henry Nash 
Hi Mark,

Of particular interest are your views on the changes to 
keystone/common/config.py.  The requirement is that we need to be able to 
instantiate multiple conf objects (built from different sets of config files).  
We tried two approaches to this:

https://review.openstack.org/#/c/39530/11 which attempts to keep the current 
keystone config helper apps (register_bool() etc.) by passing on the conf 
instance, and
https://review.openstack.org/#/c/39530/12 which removes these helper apps and 
just calls the methods on the conf itself (conf.register_opt())

Both functionally work, but interested in your views on both approaches.

Henry
On 6 Aug 2013, at 19:26, ayoung (Code Review) wrote:

 Hello Mark McLoughlin,
 
 I'd like you to do a code review.  Please visit
 
   https://review.openstack.org/39530
 
 to review the following change.
 
 Change subject: Implement domain specific Identity backends
 ..
 
 Implement domain specific Identity backends
 
 A common scenario in shared clouds will be that a cloud provider will
 want to be able to offer larger customers the ability to interface to
 their chosen identity provider. In the base case, this might well be
 their own corporate LDAP/AD directory.  A cloud provider might also
 want smaller customers to have their identity managed solely
 within the OpenStack cloud, perhaps in a shared SQL database.
 
 This patch allows domain specifc backends for identity objects
 (namely User and groups), which are specified by creation of a domain
 configuration file for each domain that requires its own backend.
 
 A side benefit of this change is that it clearly separates the
 backends into those that are domain-aware and those that are not,
 allowing, for example, the removal of domain validation from the
 LDAP identity backend.
 
 Implements bp multiple-ldap-servers
 
 Change-Id: I489e8e50035f88eca4235908ae8b1a532645daab
 ---
 M doc/source/configuration.rst
 M etc/keystone.conf.sample
 M keystone/auth/plugins/password.py
 M keystone/catalog/backends/templated.py
 M keystone/common/config.py
 M keystone/common/controller.py
 M keystone/common/ldap/fakeldap.py
 M keystone/common/utils.py
 M keystone/config.py
 M keystone/identity/backends/kvs.py
 M keystone/identity/backends/ldap.py
 M keystone/identity/backends/pam.py
 M keystone/identity/backends/sql.py
 M keystone/identity/controllers.py
 M keystone/identity/core.py
 M keystone/test.py
 M keystone/token/backends/memcache.py
 M keystone/token/core.py
 A tests/backend_multi_ldap_sql.conf
 A tests/keystone.Default.conf
 A tests/keystone.domain1.conf
 A tests/keystone.domain2.conf
 M tests/test_backend.py
 M tests/test_backend_ldap.py
 24 files changed, 1,028 insertions(+), 372 deletions(-)
 
 
 git pull ssh://review.openstack.org:29418/openstack/keystone 
 refs/changes/30/39530/12
 --
 To view, visit https://review.openstack.org/39530
 To unsubscribe, visit https://review.openstack.org/settings
 
 Gerrit-MessageType: newchange
 Gerrit-Change-Id: I489e8e50035f88eca4235908ae8b1a532645daab
 Gerrit-PatchSet: 12
 Gerrit-Project: openstack/keystone
 Gerrit-Branch: master
 Gerrit-Owner: henry-nash hen...@linux.vnet.ibm.com
 Gerrit-Reviewer: Brant Knudson bknud...@us.ibm.com
 Gerrit-Reviewer: Dolph Mathews dolph.math...@gmail.com
 Gerrit-Reviewer: Jenkins
 Gerrit-Reviewer: Mark McLoughlin mar...@redhat.com
 Gerrit-Reviewer: Sahdev Zala spz...@us.ibm.com
 Gerrit-Reviewer: SmokeStack
 Gerrit-Reviewer: ayoung ayo...@redhat.com
 Gerrit-Reviewer: henry-nash hen...@linux.vnet.ibm.com
 



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev