Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Other: repository is cluttered with task result literals #412

Open
1 task done
lukpueh opened this issue Oct 6, 2023 · 1 comment
Open
1 task done

Other: repository is cluttered with task result literals #412

lukpueh opened this issue Oct 6, 2023 · 1 comment
Assignees

Comments

@lukpueh
Copy link
Collaborator

lukpueh commented Oct 6, 2023

What do you want to share with us?

Task interface methods in repository.py return TaskResult. Repetitive literal definition of these objects (their attributes), together with a multi return pattern, clutter the code.

E.g.:

def bootstrap(
self,
payload: Dict[str, Any],
update_state: Optional[
Task.update_state
] = None, # It is required (see: app.py)
) -> Dict[str, Any]:
"""
Bootstrap the Metadata Repository
"""
tuf_settings = payload.get("settings")
if tuf_settings is None:
return self._task_result(
TaskName.BOOTSTRAP,
False,
{
"message": "Bootstrap Failed",
"error": "No 'settings' in the payload",
},
)
root_metadata = payload.get("metadata")
if root_metadata is None:
return self._task_result(
TaskName.BOOTSTRAP,
False,
{
"message": "Bootstrap Failed",
"error": "No 'metadata' in the payload",
},
)
bootstrap_status = self._settings.get_fresh("BOOTSTRAP")
if bootstrap_status is not None and "pre-" not in bootstrap_status:
return self._task_result(
TaskName.BOOTSTRAP,
False,
{
"message": "Bootstrap Failed",
"error": f"Bootstrap state is {bootstrap_status}",
},
)
root: Metadata[Root] = Metadata.from_dict(root_metadata[Root.type])
if len(root.signatures) == 0:
self.write_repository_settings("BOOTSTRAP", None)
return self._task_result(
TaskName.BOOTSTRAP,
False,
{
"message": "Bootstrap Failed",
"error": "Metadata requires at least one valid signature",
},
)
for signature in root.signatures.values():
if self._validate_signature(root, signature) is False:
self.write_repository_settings("BOOTSTRAP", None)
return self._task_result(
TaskName.BOOTSTRAP,
False,
{
"message": "Bootstrap Failed",
"error": "Bootstrap has invalid signature(s)",
},
)
# save settings
self.save_settings(root, tuf_settings)
task_id: str = payload["task_id"]
signed = self._validate_threshold(root)
if signed:
self._bootstrap_finalize(root, task_id)
message = f"Bootstrap finished {task_id}"
logging.info(message)
else:
self.write_repository_settings("ROOT_SIGNING", root.to_dict())
self.write_repository_settings("BOOTSTRAP", f"signing-{task_id}")
message = f"Root v{root.signed.version} is pending signature"
logging.info(message)
return self._task_result(
TaskName.BOOTSTRAP,
True,
{"message": "Bootstrap Processed", "bootstrap": message},
)

Existing remedies -- thin helper wrappers that generalise some common fields (see _task_result, _result) -- are not very effective.

Possible improvements:

  • Use single return pattern in interface methods, where the task result is constructed only once at the end of the method, e.g. based on state variables assigned in previous check clauses, or based on exceptions raised in subroutines. (related: don't construct task results in subroutines)

  • Define TaskResult subclasses with fixed values for re-occurring attributes.

Overall goal:

Better distinguish TUF routines from RSTUF service logic. Important to assess correctness and thus security critical!

References

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

2 participants