openstack/barbican
Code Issues Proposed changes

Fix unit tests and migration to unblock gate

Browse Source 
Currently there are 2 failing unit tests:

- test_soft_deleting_expired_secrets: Caused by passing a column instead
  of a table on the query creation.

- test_should_raise_for_pycrypto_stored_key_no_private_key: Caused by
  the conjunction of Barbican using scoped sessions and SQLAlchemy's
  identity mapping.

And a migration issue on add_secret_consumers.

This patch fixes all those issues to unblock the gate.

Story: 2008967

Change-Id: I6dc7d2671f2ba9d97af42d3155ae2bf3a8e33453
This commit is contained in:
Gorka Eguileor
2021-06-14 15:54:47 +02:00
parent b5d10b3c03
commit 1ae4badb1a
4 changed files with 9 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
barbican/model/clean.py
View File

@@ -187,7 +187,7 @@ def _soft_delete_expired_secrets(threshold_date):
""" """
current_time = timeutils.utcnow() current_time = timeutils.utcnow()
session = repo.get_session() session = repo.get_session()
query = session.query(models.Secret.id) query = session.query(models.Secret)
query = query.filter(~models.Secret.deleted) query = query.filter(~models.Secret.deleted)
query = query.filter( query = query.filter(
models.Secret.expiration <= threshold_date models.Secret.expiration <= threshold_date

4
barbican/model/migration/alembic_migrations/versions/0f8c192a061f_add_secret_consumers.py
View File

@@ -30,10 +30,8 @@ import sqlalchemy as sa
def upgrade(): def upgrade():
ctx = op.get_context()
con = op.get_bind() con = op.get_bind()
table_exists = ctx.dialect.has_table(con.engine, table_exists = sa.inspect(con.engine).has_table("secret_consumer_metadata")
"secret_consumer_metadata")
if not table_exists: if not table_exists:
op.create_table( op.create_table(
"secret_consumer_metadata", "secret_consumer_metadata",

4
barbican/model/models.py
View File

@@ -353,10 +353,10 @@ class Secret(BASE, SoftDeleteMixIn, ModelBase):
datum.delete(session) datum.delete(session)
for secret_ref in self.container_secrets: for secret_ref in self.container_secrets:
session.delete(secret_ref) secret_ref.delete(session)
for secret_acl in self.secret_acls: for secret_acl in self.secret_acls:
session.delete(secret_acl) secret_acl.delete(session)
def _do_extra_dict_fields(self): def _do_extra_dict_fields(self):
"""Sub-class hook method: return dict of fields.""" """Sub-class hook method: return dict of fields."""

7
barbican/tests/tasks/test_certificate_resources.py
View File

@@ -664,8 +664,6 @@ class WhenIssuingCertificateRequests(BaseCertificateRequestsTestCase):
self.result_follow_on) self.result_follow_on)
def test_should_raise_for_pycrypto_stored_key_no_private_key(self): def test_should_raise_for_pycrypto_stored_key_no_private_key(self):
self.order_meta.update(self.stored_key_meta)
private_key = rsa.generate_private_key( private_key = rsa.generate_private_key(
public_exponent=65537, public_exponent=65537,
key_size=2048, key_size=2048,
@@ -693,6 +691,11 @@ class WhenIssuingCertificateRequests(BaseCertificateRequestsTestCase):
secret_repo.delete_entity_by_id( secret_repo.delete_entity_by_id(
self.private_key.id, self.external_project_id) self.private_key.id, self.external_project_id)
# We need to commit deletions or we'll get deleted objects with deleted
# set to True. This is caused by SQLAlchemy's identity mapping and our
# use of scoped_session.
repositories.commit()
self.order.meta.update(self.stored_key_meta)
self.assertRaises(excep.StoredKeyPrivateKeyNotFound, self.assertRaises(excep.StoredKeyPrivateKeyNotFound,
cert_res.issue_certificate_request, cert_res.issue_certificate_request,
self.order, self.order,