Skip to content

Commit

Permalink
Pass signoff_verifier to verify_signoffs
Browse files Browse the repository at this point in the history
  • Loading branch information
@michellemounde
michellemounde committed Feb 29, 2024
1 parent db6d210 commit 2369aa1
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 32 deletions.
63 changes: 40 additions & 23 deletions src/auslib/db.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def process_result_value(self, value, dialect):
return cls


def verify_signoffs(potential_required_signoffs, signoffs):
def verify_signoffs(potential_required_signoffs, signoffs, signoff_verifier):
"""Determines whether or not something is signed off given:
* A list of potential required signoffs
* A list of signoffs that have been made
Expand All @@ -168,11 +168,10 @@ def verify_signoffs(potential_required_signoffs, signoffs):
for signoff in signoffs:
signoffs_given[signoff["role"]] += 1
for rs in potential_required_signoffs:
signoff_verifier = "role" if "role" in rs else "permission"
required_signoffs[rs[signoff_verifier]] = max(required_signoffs.get(rs[signoff_verifier], 0), rs["signoffs_required"])
for role, signoffs_required in required_signoffs.items():
if signoffs_given[role] < signoffs_required:
raise SignoffRequiredError("Not enough signoffs for role '{}'".format(role))
raise SignoffRequiredError(f"Not enough signoffs for {signoff_verifier}: {role}")


class AUSTransaction(object):
Expand Down Expand Up @@ -1607,6 +1606,7 @@ class RequiredSignoffsTable(AUSTable):
rows to determine whether or not that change needs signoff."""

decisionColumns = []
signoff_verifier = "role"

def __init__(self, db, dialect):
self.table.append_column(Column("role", String(50), primary_key=True))
Expand Down Expand Up @@ -1646,7 +1646,7 @@ def insert(self, changed_by, transaction=None, dryrun=False, signoffs=None, **co

if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([columns], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return super(RequiredSignoffsTable, self).insert(changed_by=changed_by, transaction=transaction, dryrun=dryrun, **columns)

Expand All @@ -1661,7 +1661,7 @@ def update(self, where, what, changed_by, old_data_version, transaction=None, dr

if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([rs, new_rs], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return super(RequiredSignoffsTable, self).update(
where=where, what=what, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun
Expand All @@ -1674,7 +1674,7 @@ def delete(self, where, changed_by=None, old_data_version=None, transaction=None
if not dryrun:
for rs in self.select(where=where, transaction=transaction):
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([rs], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return super(RequiredSignoffsTable, self).delete(
where=where, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun
Expand Down Expand Up @@ -1759,6 +1759,9 @@ def delete(self, where, changed_by=None, transaction=None, dryrun=False, reset_s


class Rules(AUSTable):

signoff_verifier = "role"

def __init__(self, db, metadata, dialect):
self.table = Table(
"rules",
Expand Down Expand Up @@ -1842,7 +1845,7 @@ def insert(self, changed_by, transaction=None, dryrun=False, signoffs=None, **co

if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([columns], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

ret = super(Rules, self).insert(changed_by=changed_by, transaction=transaction, dryrun=dryrun, **columns)
if not dryrun:
Expand Down Expand Up @@ -1982,7 +1985,7 @@ def update(self, where, what, changed_by, old_data_version, transaction=None, dr
potential_required_signoffs = [
obj for v in self.getPotentialRequiredSignoffs([current_rule, new_rule], transaction=transaction).values() for obj in v
]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return super(Rules, self).update(
changed_by=changed_by, where=where, what=what, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun
Expand All @@ -2000,12 +2003,15 @@ def delete(self, where, changed_by=None, old_data_version=None, transaction=None
if not dryrun:
for current_rule in self.select(where=where, transaction=transaction):
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([current_rule], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

super(Rules, self).delete(changed_by=changed_by, where=where, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun)


class Releases(AUSTable):

signoff_verifier = "role"

def __init__(self, db, metadata, dialect, history_buckets, historyClass):
self.domainAllowlist = []

Expand Down Expand Up @@ -2206,7 +2212,7 @@ def insert(self, changed_by, transaction=None, dryrun=False, signoffs=None, **co

if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([columns], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

ret = super(Releases, self).insert(changed_by=changed_by, transaction=transaction, dryrun=dryrun, **columns)
if not dryrun:
Expand Down Expand Up @@ -2247,7 +2253,7 @@ def update(self, where, what, changed_by, old_data_version, transaction=None, dr
potential_required_signoffs = [
obj for v in self.getPotentialRequiredSignoffs([current_release, new_release], transaction=transaction).values() for obj in v
]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)
else:
self.validate_readonly_change(
where, what["read_only"], changed_by, release=current_release, transaction=transaction, dryrun=dryrun, signoffs=signoffs
Expand Down Expand Up @@ -2381,7 +2387,7 @@ def delete(self, where, changed_by, old_data_version, transaction=None, dryrun=F

if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([release], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

super(Releases, self).delete(where=where, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun)
if not dryrun:
Expand Down Expand Up @@ -2425,14 +2431,17 @@ def _map_required_signoffs(required_signoffs):
potential_required_signoffs = _map_required_signoffs(
self.getPotentialRequiredSignoffsForProduct(release["product"], transaction=transaction).values()
)
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

def change_readonly(self, where, is_readonly, changed_by, old_data_version, transaction=None):
self.validate_readonly_change(where, is_readonly, changed_by, transaction=transaction)
super().update(where, {"read_only": is_readonly}, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction)


class ReleasesJSON(AUSTable):

signoff_verifier = "role"

def __init__(self, db, metadata, dialect, history_buckets, historyClass):
self.domainAllowlist = []

Expand Down Expand Up @@ -2498,7 +2507,7 @@ def getPotentialRequiredSignoffsForProduct(self, product, transaction=None):
async def async_insert(self, changed_by, transaction=None, dryrun=False, signoffs=None, **columns):
if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([columns], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return await super(ReleasesJSON, self).async_insert(changed_by=changed_by, transaction=transaction, dryrun=dryrun, **columns)

Expand All @@ -2514,7 +2523,7 @@ async def async_update(self, where, what, changed_by, old_data_version, transact
potential_required_signoffs = [
obj for v in self.getPotentialRequiredSignoffs([row, new_row], transaction=transaction).values() for obj in v
]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return await super(ReleasesJSON, self).async_update(
where=where, what=what, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun
Expand All @@ -2524,14 +2533,17 @@ async def async_delete(self, where, changed_by=None, old_data_version=None, tran
if not dryrun:
for row in self.select(where=where, transaction=transaction):
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([row], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return await super(ReleasesJSON, self).async_delete(
where=where, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun
)


class ReleaseAssets(AUSTable):

signoff_verifier = "role"

def __init__(self, db, metadata, dialect, history_buckets, historyClass):
self.table = Table(
"release_assets",
Expand Down Expand Up @@ -2579,7 +2591,7 @@ def getPotentialRequiredSignoffs(self, affected_rows, transaction=None):
async def async_insert(self, changed_by, transaction=None, dryrun=False, signoffs=None, **columns):
if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([columns], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return await super(ReleaseAssets, self).async_insert(changed_by=changed_by, transaction=transaction, dryrun=dryrun, **columns)

Expand All @@ -2590,7 +2602,7 @@ async def async_update(self, where, what, changed_by, old_data_version, transact

if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([row, new_row], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return await super(ReleaseAssets, self).async_update(
where=where, what=what, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun
Expand All @@ -2600,7 +2612,7 @@ async def async_delete(self, where, changed_by=None, old_data_version=None, tran
if not dryrun:
for row in self.select(where=where, transaction=transaction):
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([row], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

return await super(ReleaseAssets, self).async_delete(
where=where, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun
Expand Down Expand Up @@ -2638,6 +2650,8 @@ class Permissions(AUSTable):
"scheduled_change": ["actions"],
}

signoff_verifier = "permission"

def __init__(self, db, metadata, dialect):
self.table = Table(
"permissions",
Expand Down Expand Up @@ -2701,7 +2715,7 @@ def insert(self, changed_by, transaction=None, dryrun=False, signoffs=None, **co

if not dryrun:
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([columns], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

self.log.debug("granting %s to %s with options %s", columns["permission"], columns["username"], columns.get("options"))
super(Permissions, self).insert(changed_by=changed_by, transaction=transaction, dryrun=dryrun, **columns)
Expand Down Expand Up @@ -2735,7 +2749,7 @@ def update(self, where, what, changed_by, old_data_version, transaction=None, dr
potential_required_signoffs = [
obj for v in self.getPotentialRequiredSignoffs([current_permission, new_permission], transaction=transaction).values() for obj in v
]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

super(Permissions, self).update(
where=where, what=what, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun
Expand All @@ -2752,7 +2766,7 @@ def delete(self, where, changed_by=None, old_data_version=None, transaction=None
potential_required_signoffs = [
obj for v in self.getPotentialRequiredSignoffs([current_permission], transaction=transaction).values() for obj in v
]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

if not dryrun:
super(Permissions, self).delete(changed_by=changed_by, where=where, old_data_version=old_data_version, transaction=transaction)
Expand Down Expand Up @@ -2888,6 +2902,9 @@ def _putWatchdogValue(self, changed_by, value, where=None, transaction=None, dry


class EmergencyShutoffs(AUSTable):

signoff_verifier = "role"

def __init__(self, db, metadata, dialect):
self.table = Table(
"emergency_shutoffs",
Expand Down Expand Up @@ -2923,7 +2940,7 @@ def delete(self, where, changed_by=None, old_data_version=None, transaction=None
if not dryrun:
for current_rule in self.select(where=where, transaction=transaction):
potential_required_signoffs = [obj for v in self.getPotentialRequiredSignoffs([current_rule], transaction=transaction).values() for obj in v]
verify_signoffs(potential_required_signoffs, signoffs)
verify_signoffs(potential_required_signoffs, signoffs, self.signoff_verifier)

super(EmergencyShutoffs, self).delete(changed_by=changed_by, where=where, old_data_version=old_data_version, transaction=transaction, dryrun=dryrun)

Expand Down
25 changes: 16 additions & 9 deletions tests/test_db.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,30 +83,34 @@ def getTempfile(self):

class TestVerifySignoffs(unittest.TestCase):
def testNoRequiredSignoffs(self):
verify_signoffs({}, {})
verify_signoffs({}, {}, "role")

def testNoRequiredSignoffsWithSignoffs(self):
verify_signoffs({}, [{"role": "releng"}, {"role": "relman"}])
verify_signoffs({}, [{"role": "releng"}, {"role": "relman"}], "role")

def testNoSignoffsGiven(self):
required = [{"role": "releng", "signoffs_required": 1}]
signoffs = []
self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs)
signoff_verifier = "role"
self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs, signoff_verifier)

def testMissingSignoffFromOneRole(self):
required = [{"role": "releng", "signoffs_required": 1}, {"role": "relman", "signoffs_required": 1}]
signoffs = [{"role": "releng", "username": "joe"}]
self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs)
signoff_verifier = "role"
self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs, signoff_verifier)

def testNotEnoughSignoffsFromOneRole(self):
required = [{"role": "releng", "signoffs_required": 2}, {"role": "relman", "signoffs_required": 1}]
signoffs = [{"role": "releng", "username": "joe"}, {"role": "relman", "username": "jane"}]
self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs)
signoff_verifier = "role"
self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs, signoff_verifier)

def testExactlyEnoughSignoffsGiven(self):
required = [{"role": "releng", "signoffs_required": 2}, {"role": "relman", "signoffs_required": 1}]
signoffs = [{"role": "releng", "username": "joe"}, {"role": "releng", "username": "jane"}, {"role": "relman", "username": "nick"}]
verify_signoffs(required, signoffs)
signoff_verifier = "role"
verify_signoffs(required, signoffs, signoff_verifier)

def testMoreThanEnoughSignoffsGiven(self):
required = [{"role": "releng", "signoffs_required": 2}, {"role": "relman", "signoffs_required": 1}]
Expand All @@ -116,17 +120,20 @@ def testMoreThanEnoughSignoffsGiven(self):
{"role": "relman", "username": "nick"},
{"role": "relman", "username": "matt"},
]
verify_signoffs(required, signoffs)
signoff_verifier = "role"
verify_signoffs(required, signoffs, signoff_verifier)

def testMultiplePotentialSignoffsForOneGroupWithoutEnoughSignoffs(self):
required = [{"role": "releng", "signoffs_required": 2}, {"role": "releng", "signoffs_required": 1}, {"role": "relman", "signoffs_required": 1}]
signoffs = [{"role": "releng", "username": "joe"}, {"role": "relman", "username": "nick"}]
self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs)
signoff_verifier = "role"
self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs, signoff_verifier)

def testMultiplePotentialSignoffsForOneGroupWithEnoughSignoffs(self):
required = [{"role": "releng", "signoffs_required": 2}, {"role": "releng", "signoffs_required": 1}, {"role": "relman", "signoffs_required": 1}]
signoffs = [{"role": "releng", "username": "joe"}, {"role": "releng", "username": "jane"}, {"role": "relman", "username": "nick"}]
verify_signoffs(required, signoffs)
signoff_verifier = "role"
verify_signoffs(required, signoffs, signoff_verifier)


class TestAUSTransaction(unittest.TestCase, MemoryDatabaseMixin):
Expand Down

0 comments on commit 2369aa1

Please sign in to comment.