region update doesn't update extras

Bug #1729933 reported by David Lyle
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
Low
Gage Hugo

Bug Description

The region table contains an `extra` column.

Although the API for regions supports setting `extra`. The update method does not support updating extras in the default sql driver backend. This effectively means that while an extra can be set at create time, they cannot be altered once set, nor can additional values be added.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

Fix proposed to branch: master
Review: https://review.openstack.org/517726

Changed in keystone:
assignee: nobody → David Lyle (david-lyle)
status: New → In Progress
David Lyle (david-lyle)
description: updated
Revision history for this message
Lance Bragstad (lbragstad) wrote :

This was a huge issue with the implementation of `extras` and why we no longer encourage its use in any keystone entity that supports it. I think introducing functionality that updates key-value pairs in extras will only confuse users further. Keystone doesn't do anything with the `enabled` key if it is specified in `extras`, but making it update-able won't really change that either. As a deployer, I have the ability to update the property but it doesn't actually change anything in my deployment. Instead, I think we should fix python-keystoneclient to not assume enabled is an actual region property, which is mentioned in bug 1615076.

Allowing regions to support updating enabled means we would be inconsistent with other entities and their ability to update values in `extras`.

Revision history for this message
David Lyle (david-lyle) wrote :

at least in my past life was used to store identity values in keystone that were necessary for a particular deployment that keystone didn't specifically support. I imagine this is not something keystone really likes, but create only extras are weird. Things change. Making this extra settable just makes it consistent with other keystone identity constructs.

Revision history for this message
David Lyle (david-lyle) wrote :

*at least in my past life extras were used

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

Entities that have supported setting extras we have maintained support for the setting of extras. In the case of Regions, it may have just inherited the bad base-sql object and we never actually set them.

There is a hard "no further extras support" within Keystone. Extras are ugly, can add serious load to the DB and mostly were used for data that belonged in CRMS (e.g. Salesforce) and not in Keystone (some exceptions to this happened, but it was infrequent).

If Region ever supported setting of extras, we need to add support back in. If it didn't we will not add the support for it (all new constructs do not support it, and do not have an extras column).

tl;dr - we would remove extras across all of Keystone if it wouldn't break the API contract. Keystone should not just be a bad key-value store, all entities should have clear defined validation.

--
As an aside, I have proposed adding in an operator/deployment specific area (e.g. User['vendordata'] that may be used like extras; in the way extras was implemented previously any attribute (or structure) not in the model (top level, e.g. User['mycooldData']) would be serialized and shoved into the DB in the extras column. This made it had to add any fields as we may stomp on what folks used. We also continually were asked for ways to search/support indexing on the "whatever we shoved into extras". Extras was/is very sloppy (unfortunately).

Revision history for this message
Colleen Murphy (krinkle) wrote :

Regions already support setting extra. The bug is that it only supports create and not update, which is inconsistent with other keystone resources that have an extra column. This isn't about adding a new feature. I think it's completely reasonable to fix this.

Changed in keystone:
assignee: David Lyle (david-lyle) → Gage Hugo (gagehugo)
Gage Hugo (gagehugo)
Changed in keystone:
importance: Undecided → Low
milestone: none → stein-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.openstack.org/517726
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=ef331f46b46b7b9705b2b4be0e62b0ac6a1e694e
Submitter: Zuul
Branch: master

commit ef331f46b46b7b9705b2b4be0e62b0ac6a1e694e
Author: David Lyle <email address hidden>
Date: Fri Nov 3 14:50:58 2017 -0600

    Region update extra support

    The region update API currently does not allow for adding any
    extra values via the update API. This effectively means that while
    an extra can be set at create time, they cannot be altered once
    set, nor can any additional values be added.

    This patch add the missing inclusion of extra to the new ref.

    Change-Id: I6e99764c0e360656ddbb47ebb23fe9576c97b99f
    Closes-Bug: #1729933

Changed in keystone:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/keystone 15.0.0.0rc1

This issue was fixed in the openstack/keystone 15.0.0.0rc1 release candidate.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.