Tuf: How much metadata behaviour should be implemented on their classes?

Created on 10 Sep 2020  路  4Comments  路  Source: theupdateframework/tuf

The TUF refactor aims at a more idiomatic use of OOP (see #1112). As such it seems reasonable to implement metadata entity behaviour as instance methods on the corresponding classes.

However, not all users of the metadata model need the same behaviour. For instance, a TUF client is likely to only need Metadata.verify and read access on all metadata object attributes, but none of the methods to modify attributes, which are needed by a repository library/tool, (e.g. sign, bump_version, bump_expiration, delegate, add_keys, add_targets, etc.).

This question about how to draw the line is especially important if unneeded functionality requires 3rd party dependencies, which we would have to vendor on a client.

Some possible approaches (brain storming):

  • Use classes exclusively for attributes (except maybe for methods on sslib's Metadata and Signed classes, such as sign, verify, canonicalise) and implement all behaviour on metadata user specific controller classes, e.g. Repository, Client (or something like Project/Developer/Workspace for PEP480.
  • Use Subclasses, e.g. RepositoryTargets, ClientTargets, etc...
  • Expose all methods to all users of the model, but handle missing optional dependencies. E.g. raise something like an UnsupportedLibraryError if a client calls Metadata.sign but does not have the underlying optional 3rd-party dependencies (see https://github.com/secure-systems-lab/securesystemslib/issues/179)

Premise: Find a balance between OOP purism and pragmatism

decision record discussion documentation

Most helpful comment

Related discussion: Is there a need for library functions that should be implemented neither on a metadata class nor on a controller class?

In the current code there are a lot of different functions performing hashing. Probably this can be done using directly securesystemslib but if such a pattern starts to appear again, maybe a helper/utility/wrapper class?
_for the record I don't like going in this direction_

All 4 comments

Related discussion: Is there a need for library functions that should be implemented neither on a metadata class nor on a controller class?

_dropping some thoughts here ..._

On one hand the general preference should be towards composition over inheritance, in this case the controller classes option.
On the other hand, Client needs only a subset of all metadata methods, meaning there is a common base functionality between Client and Repository which repository extends which hints toward inheritance. However, Signed class already has one level of inheritance hierarchy which I find meaningful and I don't want to suggest ruining.

Having said that maybe something like option one:

Use classes exclusively for attributes (except maybe for methods on sslib's Metadata and Signed classes, such as sign, verify, canonicalise)

plus a base Controller class that can be extended by a RepositoryController, if needed, seems like the best compromise.

I like option three too,

Expose all methods to all users of the model, but handle missing optional dependencies. E.g. raise something like an UnsupportedLibraryError if a client calls Metadata.sign but does not have the underlying optional 3rd-party dependencies

mainly for its simplicity but I can't add much knowledge on potential dependencies issues

Related discussion: Is there a need for library functions that should be implemented neither on a metadata class nor on a controller class?

In the current code there are a lot of different functions performing hashing. Probably this can be done using directly securesystemslib but if such a pattern starts to appear again, maybe a helper/utility/wrapper class?
_for the record I don't like going in this direction_

To answer Luke's question, my 馃敟 take is 馃挴

Was this page helpful?
0 / 5 - 0 ratings

Related issues

lukpueh picture lukpueh  路  8Comments

awwad picture awwad  路  5Comments

sechkova picture sechkova  路  8Comments

lukpueh picture lukpueh  路  8Comments

lukpueh picture lukpueh  路  8Comments