Skip to content

Commit

Permalink
Refactor update_snapshot & _run_online_roles_bump (#432)
Browse files Browse the repository at this point in the history 
* Refactor update_snapshot & _run_online_roles_bump

The goal of the "update_snapshot" refactoring is to:
1. make a better separation between the two usages of "update_snapshot"
- the first is to update snapshot and all roles that were given with the
"targets_roles" argument
- the second is to update snapshot and ALL bin target roles
In the end, I wanted to be able to call "update_snapshot" with
"bump_all" without having to pass the "targets_roles" argument.
2. fix a bug where when calling "update_snapshot" the resulting new
snapshot meta didn't contain updated information for targets.json
This resulted in a bug that when calling "_run_online_roles_bump", which
updated targets.json, we saved the new targets.json, but the new
snapshot.json didn't reference it.

The second part of this refactoring is about "_run_online_roles_bump".
Again there were two goals of this refactoring:
1. make sure we call "update_snapshot" correctly. This means when we
force an update of all target bins we shouldn't also pass the
"targets_roles" list.
2. make it clearer how we use the "force" argument:
- when "force = True" we force a bump of all bins and targets.json
- when "force = False" we bump only those bins and targets.json which
have expired.

Finally, I did some small tweaks on our tests with the idea to improve
them our asserts and checks.

Signed-off-by: Martin Vrachev <[email protected]>

* Remove "force" check from a specific use case

In this specific "if" check we don't need to check the value of the
"force" argument as that case is already handled from a previous
check where if "force" is True we call update_snapshot with "bump_all".

Signed-off-by: Martin Vrachev <[email protected]>

* Move "force" check outside timedelta

Currently, if the check if snapshot expiration is below a certain
timedelta and "force" are True we wouldn't taken into account the
value of force.
With this fix, this is changed.

Signed-off-by: Martin Vrachev <[email protected]>

* Add some logging

Signed-off-by: Martin Vrachev <[email protected]>

* Remove StorageError exception handling

We decided that it's best to not handle a StorageError exception
and instead allow the test to fail. Fast API will fail if there is an
exception from the worker and given that the StorageError exception
has enough details there is no need for us to handle it.

Additionally, this allowed us to remove the boolean return value
from _run_online_roles_bump and bump_snapshot.

Signed-off-by: Martin Vrachev <[email protected]>

---------

Signed-off-by: Martin Vrachev <[email protected]>
Co-authored-by: Martin Vrachev <[email protected]>
  • Loading branch information
@MVrachev
MVrachev and MVrachev authored Jan 5, 2024
1 parent 08e5b40 commit 0463b08
Show file tree
Hide file tree
Showing 2 changed files with 345 additions and 210 deletions.
218 changes: 112 additions & 106 deletions repository_service_tuf_worker/repository.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@
from celery.exceptions import ChordError
from celery.result import AsyncResult, states
from dynaconf.loaders import redis_loader
from securesystemslib.exceptions import ( # type: ignore
StorageError,
UnverifiedSignatureError,
)
from securesystemslib.exceptions import UnverifiedSignatureError
from securesystemslib.signer import Signature, SSlibKey
from tuf.api.exceptions import (
BadVersionNumberError,
Expand Down Expand Up @@ -286,69 +283,83 @@ def _update_snapshot(
Returns the new snapshot version.
Args:
bump_all: Wheter to bump all bin target roles.
target_roles: List of roles to bump. If provided, 'bump_all' arg
will NOT be taken into account.
bump_all: Wheter to bump all bin target roles. If provided, then
'target_roles' arg is NOT taken into acount.
"""
snapshot: Metadata[Snapshot] = self._storage_backend.get(Snapshot.type)

db_target_roles: List[targets_models.RSTUFTargetRoles] = []
if target_roles:
db_target_roles: List[targets_models.RSTUFTargetRoles] = []
if bump_all:
db_target_roles = targets_crud.read_all_roles(self._db)
for db_role in db_target_roles:
rolename = db_role.rolename
bins_md: Metadata[Targets] = self._storage_backend.get(
db_role.rolename
)
# update expiry, bump version and persist to the storage
self._bump_and_persist(bins_md, BINS, persist=False)
self._persist(bins_md, db_role.rolename)
db_target_roles = targets_crud.read_roles_joint_files(
self._db, target_roles
)

snapshot.signed.meta[f"{rolename}.json"] = MetaFile(
version=bins_md.signed.version
)
else:
db_target_roles = targets_crud.read_roles_joint_files(
self._db, target_roles
for db_role in db_target_roles:
rolename = db_role.rolename
bins_md: Metadata[Targets] = self._storage_backend.get(
rolename
)
bins_md.signed.targets.clear()
bins_md.signed.targets = {
file.path: TargetFile.from_dict(file.info, file.path)
for file in db_role.target_files
if file.action == targets_schema.TargetAction.ADD
# Filtering the files with action 'ADD' cannot be done
# in CRUD. If a target role doesn't have any target
# files with an action 'ADD' (only 'REMOVE') then using
# CRUD will not return the target role and it won't be
# updated. An example can be when there is a role with
# one target file with action "REMOVE" and the CRUD
# will return None for this specific role.
}

# update expiry, bump version and persist to the storage
self._bump_and_persist(bins_md, BINS, persist=False)
self._persist(bins_md, rolename)
# update targetfile in db
# note: It update only if is not published see the CRUD.
targets_crud.update_files_to_published(
self._db, [file.path for file in db_role.target_files]
)

for db_role in db_target_roles:
rolename = db_role.rolename
bins_md: Metadata[Targets] = self._storage_backend.get(
rolename
)
bins_md.signed.targets.clear()
bins_md.signed.targets = {
file.path: TargetFile.from_dict(file.info, file.path)
for file in db_role.target_files
if file.action == targets_schema.TargetAction.ADD
# Filtering the files with action 'ADD' cannot be done
# in CRUD. If a target role doesn't have any target
# files with an action 'ADD' (only 'REMOVE') then using
# CRUD will not return the target role and it won't be
# updated. An example can be when there is a role with
# one target file with action "REMOVE" and the CRUD
# will return None for this specific role.
}

# update expiry, bump version and persist to the storage
self._bump_and_persist(bins_md, BINS, persist=False)
self._persist(bins_md, rolename)
# update targetfile in db
# note: It update only if is not published see the CRUD.
targets_crud.update_files_to_published(
self._db, [file.path for file in db_role.target_files]
)
snapshot.signed.meta[f"{rolename}.json"] = MetaFile(
version=bins_md.signed.version
)

snapshot.signed.meta[f"{rolename}.json"] = MetaFile(
version=bins_md.signed.version
)
bins = "".join(target_roles)
logging.info(f"Bumped all expired target 'bin' roles: {bins}")

elif bump_all:
db_target_roles = targets_crud.read_all_roles(self._db)
for db_role in db_target_roles:
rolename = db_role.rolename
bins_md: Metadata[Targets] = self._storage_backend.get(
db_role.rolename
)
# update expiry, bump version and persist to the storage
self._bump_and_persist(bins_md, BINS, persist=False)
self._persist(bins_md, db_role.rolename)

snapshot.signed.meta[f"{rolename}.json"] = MetaFile(
version=bins_md.signed.version
)
logging.info("Bumped all target 'bin' roles")

if len(db_target_roles) > 0:
targets_crud.update_roles_version(
self._db, [int(db_role.id) for db_role in db_target_roles]
)

targets: Metadata[Targets] = self._storage_backend.get(Targets.type)
snapshot.signed.meta[f"{Targets.type}.json"] = MetaFile(
version=targets.signed.version
)

# update expiry, bump version and persist to the storage
self._bump_and_persist(snapshot, Snapshot.type)
logging.info("Bumped version of 'Snapshot' role")

return snapshot.signed.version

Expand Down Expand Up @@ -946,7 +957,7 @@ def remove_targets(
},
)

def _run_online_roles_bump(self, force: Optional[bool] = False) -> bool:
def _run_online_roles_bump(self, force: Optional[bool] = False):
"""
Bumps version and expiration date of all online roles (`Targets`,
`Succinct Delegated` targets roles, `Timestamp` and `Snapshot`).
Expand All @@ -958,68 +969,70 @@ def _run_online_roles_bump(self, force: Optional[bool] = False) -> bool:
signed and persisted using the configured key and storage services.
Args:
force: force target roles bump if they don't match the hours before
expire (`self._hours_before_expire`)
force: force all target roles bump even if they have more than
`self._hours_before_expire` hours to expire.
"""
targets_roles: List[str] = []

try:
targets = self._storage_backend.get(Targets.type)
except StorageError:
logging.error(f"{Targets.type} not found, not bumping.")
return False

targets: Metadata = self._storage_backend.get(Targets.type)
timestamp: Metadata
snapshot_bump = False
if self._settings.get_fresh("TARGETS_ONLINE_KEY") is None:
logging.critical("No configuration found for TARGETS_ONLINE_KEY")

elif self._settings.get_fresh("TARGETS_ONLINE_KEY") is False:
logging.warning(
f"{Targets.type} don't use online key, skipping 'Targets' role"
)

else:
if (targets.signed.expires - datetime.now()) < timedelta(
if force or (targets.signed.expires - datetime.now()) < timedelta(
hours=self._hours_before_expire
):
logging.info("Bumped version of 'Targets' role")
self._bump_and_persist(targets, Targets.type)
targets_roles.append(Targets.type)

targets_succinct_roles = targets.signed.delegations.succinct_roles
for bins_name in targets_succinct_roles.get_roles():
bins_role: Metadata[Targets] = self._storage_backend.get(bins_name)

if (bins_role.signed.expires - datetime.now()) < timedelta(
hours=self._hours_before_expire or force is True
):
targets_roles.append(bins_name)
snapshot_bump = True

if len(targets_roles) > 0:
logging.info(
"[scheduled targets bump] Targets and delegated Targets roles "
"version bumped: {targets_meta}"
)
if force:
# Updating all bin target roles.
timestamp = self._update_timestamp(
self._update_snapshot(targets_roles, bump_all=True)
self._update_snapshot(bump_all=True)
)
snapshot_bump = True
logging.info("Targets and delegated Targets roles version bumped")
else:
# Updating only those bins that have expired.
bin_roles: List[str] = []
targets_succinct_roles = targets.signed.delegations.succinct_roles
for bin in targets_succinct_roles.get_roles():
bin_role: Metadata[Targets] = self._storage_backend.get(bin)
if (bin_role.signed.expires - datetime.now()) < timedelta(
hours=self._hours_before_expire
):
bin_roles.append(bin)

if len(bin_roles) > 0:
timestamp = self._update_timestamp(
self._update_snapshot(target_roles=bin_roles)
)
snapshot_bump = True
bins = "".join(bin_roles)
logging.info(f"Bumped versions of expired bin roles: {bins}")
else:
logging.debug(
"[scheduled bump] All bin roles have more than "
f"{self._hours_before_expire} hour(s) to expire, "
"skipping"
)

if snapshot_bump:
snapshot_v = timestamp.signed.snapshot_meta.version
logging.info(
"[scheduled targets bump] Snapshot version bumped: "
f"{timestamp.signed.snapshot_meta.version}"
f"[scheduled bump] Snapshot version bumped: {snapshot_v}"
)
timestamp_v = timestamp.signed.version
logging.info(
"[scheduled targets bump] Timestamp version bumped: "
f"{timestamp.signed.version} new expire "
f"{timestamp.signed.expires}"
)
else:
logging.debug(
"[scheduled targets bump] All more than "
f"{self._hours_before_expire} hour(s) to expire, "
"skipping"
f"[scheduled bump] Timestamp version bumped: {timestamp_v}"
)

return True

def bump_snapshot(self, force: Optional[bool] = False) -> bool:
def bump_snapshot(self, force: Optional[bool] = False):
"""
Bumps version and expiration date of TUF 'snapshot' role metadata.
Expand All @@ -1034,15 +1047,10 @@ def bump_snapshot(self, force: Optional[bool] = False) -> bool:
expire (`self._hours_before_expire`)
"""

try:
snapshot = self._storage_backend.get(Snapshot.type)
except StorageError:
logging.error(f"{Snapshot.type} not found, not bumping.")
return False

snapshot = self._storage_backend.get(Snapshot.type)
if (snapshot.signed.expires - datetime.now()) < timedelta(
hours=self._hours_before_expire or force is True
):
hours=self._hours_before_expire
) or force:
timestamp = self._update_timestamp(self._update_snapshot())
logging.info(
"[scheduled snapshot bump] Snapshot version bumped: "
Expand All @@ -1061,8 +1069,6 @@ def bump_snapshot(self, force: Optional[bool] = False) -> bool:
f"{self._hours_before_expire} hour, skipping"
)

return True

def bump_online_roles(self, force: Optional[bool] = False) -> bool:
"""
Bump online roles (Snapshot, Timestamp, Targets and BINS).
Expand Down Expand Up @@ -1242,7 +1248,7 @@ def metadata_update(
False,
{
"message": "Metadata Update Failed",
"error": "Metadata Update requires a complete bootstrap",
"error": "Metadata Update requires a completed bootstrap",
},
)

Expand Down
Loading

0 comments on commit 0463b08

Please sign in to comment.