openstack
/
keystone
Code Issues Proposed changes

sql: Fix incorrect columns 

In these instances, we take the migrations to be the "official" version
- since they're stricter in almost all cases - updating the models to
suit.

This change highlights a slight issue in our use of a config option in
our database schema, which we shouldn't really do. A TODO is left to
address this later. We can also remove a now-unnecessary TODO from our
initial migration related to the same issue: we have our own tooling for
migrations that *does* load and register config options so there is no
longer an issue here.

Change-Id: I906cb8f7b76833c880a40c1aa0584fe7ab93cb7a
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
This commit is contained in:
Stephen Finucane 2023-04-06 11:38:22 +01:00
parent a5544ea337
commit 2bf70a10a2
5 changed files with 51 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
keystone/common/sql/migrations/env.py
View File

@ -34,19 +34,6 @@ target_metadata = core.ModelBase.metadata
def include_object(object, name, type_, reflected, compare_to):
BORKED_COLUMNS = (
# nullable values are incorrect
('credential', 'encrypted_blob'),
('credential', 'key_hash'),
('federated_user', 'user_id'),
('federated_user', 'idp_id'),
('local_user', 'user_id'),
('nonlocal_user', 'user_id'),
('password', 'local_user_id'),
# default values are incorrect
('password', 'created_at_int'),
('password', 'self_service'),
('project', 'is_domain'),
('service_provider', 'relay_state_prefix'),
)
BORKED_UNIQUE_CONSTRAINTS = (

11
keystone/common/sql/migrations/versions/27e647c0fad4_initial_version.py
View File

@ -39,15 +39,6 @@ LOG = log.getLogger(__name__)
NULL_DOMAIN_ID = '<<keystone.domain.root>>'
# FIXME(stephenfin): Remove this as soon as we're done reworking the
# migrations. Until then, this is necessary to allow us to use the native
# alembic tooling (which won't register opts). Alternatively, maybe
# the server default *shouldn't* rely on a (changeable) config option value?
try:
service_provider_relay_state_prefix_default = CONF.saml.relay_state_prefix
except Exception:
service_provider_relay_state_prefix_default = 'ss:mem:'
def upgrade():
bind = op.get_bind()
@ -510,7 +501,7 @@ def upgrade():
'relay_state_prefix',
sql.String(256),
nullable=False,
server_default=service_provider_relay_state_prefix_default,
server_default=CONF.saml.relay_state_prefix,
),
mysql_engine='InnoDB',
mysql_charset='utf8',

4
keystone/credential/backends/sql.py
View File

@ -30,9 +30,9 @@ class CredentialModel(sql.ModelBase, sql.ModelDictMixinWithExtras):
user_id = sql.Column(sql.String(64),
nullable=False)
project_id = sql.Column(sql.String(64))
_encrypted_blob = sql.Column('encrypted_blob', sql.Text(), nullable=True)
_encrypted_blob = sql.Column('encrypted_blob', sql.Text(), nullable=False)
type = sql.Column(sql.String(255), nullable=False)
key_hash = sql.Column(sql.String(64), nullable=True)
key_hash = sql.Column(sql.String(64), nullable=False)
extra = sql.Column(sql.JsonBlob())
@hybrid_property

21
keystone/federation/backends/sql.py
View File

@ -12,18 +12,31 @@
# License for the specific language governing permissions and limitations
# under the License.
import oslo_config.cfg
from oslo_log import log
from oslo_serialization import jsonutils
from sqlalchemy import orm
from keystone.common import sql
import keystone.conf
from keystone import exception
from keystone.federation.backends import base
from keystone.i18n import _
CONF = keystone.conf.CONF
LOG = log.getLogger(__name__)
# FIXME(stephenfin): This is necessary to allow Sphinx to auto-generate
# documentation. Sphinx's autogen extension doesn't/can't initialise
# oslo.config and register options which means attempts to retrieve this value
# fail. Using a configurable option for server_default feels like a bad idea
# and we should probably remove server_default in favour of setting the value
# in code.
try:
service_provider_relay_state_prefix_default = CONF.saml.relay_state_prefix
except oslo_config.cfg.NoSuchOptError:
service_provider_relay_state_prefix_default = 'ss:mem:'
class FederationProtocolModel(sql.ModelBase, sql.ModelDictMixin):
__tablename__ = 'federation_protocol'
@ -156,7 +169,11 @@ class ServiceProviderModel(sql.ModelBase, sql.ModelDictMixin):
description = sql.Column(sql.Text(), nullable=True)
auth_url = sql.Column(sql.String(256), nullable=False)
sp_url = sql.Column(sql.String(256), nullable=False)
relay_state_prefix = sql.Column(sql.String(256), nullable=False)
relay_state_prefix = sql.Column(
sql.String(256),
nullable=False,
server_default=service_provider_relay_state_prefix_default,
)
@classmethod
def from_dict(cls, dictionary):

41
keystone/identity/backends/sql_model.py
View File

@ -276,7 +276,7 @@ class LocalUser(sql.ModelBase, sql.ModelDictMixin):
__tablename__ = 'local_user'
attributes = ['id', 'user_id', 'domain_id', 'name']
id = sql.Column(sql.Integer, primary_key=True)
user_id = sql.Column(sql.String(64))
user_id = sql.Column(sql.String(64), nullable=False)
domain_id = sql.Column(sql.String(64), nullable=False)
name = sql.Column(sql.String(255), nullable=False)
passwords = orm.relationship('Password',
@ -301,8 +301,11 @@ class Password(sql.ModelBase, sql.ModelDictMixin):
attributes = ['id', 'local_user_id', 'password_hash', 'created_at',
'expires_at']
id = sql.Column(sql.Integer, primary_key=True)
local_user_id = sql.Column(sql.Integer, sql.ForeignKey('local_user.id',
ondelete='CASCADE'))
local_user_id = sql.Column(
sql.Integer,
sql.ForeignKey('local_user.id', ondelete='CASCADE'),
nullable=False,
)
password_hash = sql.Column(sql.String(255), nullable=True)
# TODO(lbragstad): Once Rocky opens for development, the _created_at and
@ -315,11 +318,19 @@ class Password(sql.ModelBase, sql.ModelDictMixin):
default=datetime.datetime.utcnow)
_expires_at = sql.Column('expires_at', sql.DateTime, nullable=True)
# set the default to 0, a 0 indicates it is unset.
created_at_int = sql.Column(sql.DateTimeInt(), nullable=False,
default=datetime.datetime.utcnow)
created_at_int = sql.Column(
sql.DateTimeInt(),
nullable=False,
default=0,
server_default='0',
)
expires_at_int = sql.Column(sql.DateTimeInt(), nullable=True)
self_service = sql.Column(sql.Boolean, default=False, nullable=False,
server_default='0')
self_service = sql.Column(
sql.Boolean,
default=False,
nullable=False,
server_default='0',
)
@hybrid_property
def created_at(self):
@ -345,10 +356,16 @@ class FederatedUser(sql.ModelBase, sql.ModelDictMixin):
attributes = ['id', 'user_id', 'idp_id', 'protocol_id', 'unique_id',
'display_name']
id = sql.Column(sql.Integer, primary_key=True)
user_id = sql.Column(sql.String(64), sql.ForeignKey('user.id',
ondelete='CASCADE'))
idp_id = sql.Column(sql.String(64), sql.ForeignKey('identity_provider.id',
ondelete='CASCADE'))
user_id = sql.Column(
sql.String(64),
sql.ForeignKey('user.id', ondelete='CASCADE'),
nullable=False,
)
idp_id = sql.Column(
sql.String(64),
sql.ForeignKey('identity_provider.id', ondelete='CASCADE'),
nullable=False,
)
protocol_id = sql.Column(sql.String(64), nullable=False)
unique_id = sql.Column(sql.String(255), nullable=False)
display_name = sql.Column(sql.String(255), nullable=True)
@ -368,7 +385,7 @@ class NonLocalUser(sql.ModelBase, sql.ModelDictMixin):
attributes = ['domain_id', 'name', 'user_id']
domain_id = sql.Column(sql.String(64), primary_key=True)
name = sql.Column(sql.String(255), primary_key=True)
user_id = sql.Column(sql.String(64))
user_id = sql.Column(sql.String(64), nullable=False)
__table_args__ = (
sql.UniqueConstraint('user_id'),
sqlalchemy.ForeignKeyConstraint(