Dictionary.get() by default will return "None" if the key is not
found as documented in:
https://docs.python.org/3/library/stdtypes.html#dict.get
This means we don't get anything by passing the default type.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Remove `bump_expiration()` method, which is unlikely to be used as
is, i.e. bump to "current expiration date plus delta". A more
realistic use case is to bump to "now plus delta" (see #1727 for
details).
Moreover, bump_expiration can either way easily be replaced by a
one-liner expression using the 'datetime' module. A corresponding
code snippet is added to the `expires` property's docstring. Note:
`expires` became a property with a millisec-removing setter (for
spec conformance) in #1712, which further reduces the need for a
convenience bump_expiration method.
This patch also removes a related unit test and updates another
one.
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Remove ambiguous, unspecific, opinionated and trivial 'update'
methods, which can be replaced by feasible one-liners that assign
values directly to the object attribute to be *updated*. (see #1627
for details).
Reasons to have these methods would be increased usability in terms of
- reduced work
- immediate feedback on invalid assignments
However, given above described issues, the reasons against the
methods as they are now seem to outweigh the reasons for them.
Furthermore, it seems easier to re-add similar methods, which
addressed these issues, after the upcoming 1.0.0 release than to
remove or modify them.
This patch also removes the corresponding tests as they become
irrelevant (there is no need to test object assignment). In the
case of the timestamp test, the removal also includes redundant
test logic, which is already tested in `test_metadata_base`.
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
I added "dateutil" as a possible argument type for
`Metadata.bump_expiration()` as we are already testing for this and
implying it should be supported.
The problem is that "dateutil" is not added as a nontest requirement
and after a discussion, we decided we don't want to add it as well.
That's why we decided to remove "dateutil" mentions from the code
and not confuse our users we support it.
We will create a separate issue discussing the validity of
`Metadata.bump_expiration()`.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Change the delta argument type from the tuf/api/metadata.py module
in Signed.bump_expiration() to include relativedelta as
this provides an easier interface for the callers.
We are already testing for that inside test/api line 338.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
This applies the use of constants of top-level rolenames in the
tests instead of the previously hardcoded strings.
Fixes#1648
Signed-off-by: Ivana Atanasova <iyovcheva@iyovcheva-a02.vmware.com>
This is a change in the metadata API to remove hardcoded rolenames
and use constants instead.
Fixes#1648
Signed-off-by: Ivana Atanasova <iyovcheva@iyovcheva-a02.vmware.com>
- Add the check for empty strings in the Delegate Role name
- Remove the comprehensive lists to make the code more readable
- Remove the test for empty file name from
``test_updater_with_simulator``
Signed-off-by: Kairo de Araujo <kdearaujo@vmware.com>
- Reuse the dataset and the existing tests
- Fix the keyids in the tests datasets to be aligned
- Fix the ``ValueError`` message aligned to the existent messages
Signed-off-by: Kairo de Araujo <kdearaujo@vmware.com>
This commit adds the validation in the ``metadata.Delegations``
to prevent that one of the delegate role names given is a top-level
role name.
A ``ValueError`` will be raised if one of the roles names in the
list given to as delegated contains the role name as one of the
top-level roles.
Signed-off-by: Kairo de Araujo <kdearaujo@vmware.com>
Add sanity types checks on Targets delegation paths and
path_hash_prefixes making sure that they are strings.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
_ is often used when a function returns multiple values and you need
a sub-portion of them. Then, those values that are unnecessary can be
named _.
Currently, pylint warns us that this is not a good variable name, so
fix that.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Pylint reported a couple of warnings flagged as "duplicate-code".
We were truly duplicating code - one of the examples was when we
imported the same objects from tuf/api/metadata.py:
MetaFile, Role, Root, Snapshot, TargetFile, Targets, and Timestamp
in two separate modules.
So, I thought we do want to be repetitive here and include that code at
both modules. The problem is that besides importing the above
classes the modules imported other classes from tuf.api.metadata.py
and there was no way to disable this check.
I searched and found out that this is a known problem:
https://github.com/PyCQA/pylint/issues/214.
That's why the only solution I see is to disable this warning
temporarily and hoping that one day when this issue is fixed we will
remember to turn it on again.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Mark roles as a Mapping to indicate that users should not add or remove
values from the dictionary during the lifetime of the Root object)
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
By default pylint does format checks:
https://pylint.pycqa.org/en/latest/technical_reference/features.html?highlight=format#format-checker
The problem is we also use black and isort who have format checkers as
well. This makes pylint format checks obsolete.
Also, it's possible that you would want to disable a warning and you
can end up in the situation where you will have to disable it for
two tools altogether.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Validate that root role names are 4 and that they are exactly
"root", "snapshot", "targets" and "timestamp" as described in
the spec:
https://theupdateframework.github.io/specification/latest/#root-role
Additionally, fix the valid_roots dataset, so each of the cases contains
the top metadata role names inside the roles dictionary.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
This may have been required by a linter at some point, but isn't
anymore: Not annotating makes the documentation look better.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Situation before
* constructor args are not documented
* object attributes are documented
* sphinx cannot show object attribute type annotations
* attribute docs take a lot of vertical space
Now:
* constructor args are documented
* sphinx can show annotated types of constructor args
* class docstring now explains the attributes are the same as
constructor args (and attributes are not explicitly documented)
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
This is an API change to the exceptions thrown in Root.add_key()
and Root.remove_key().
The reason for that change is that in my opinion the correct exceptions
in these cases should be "ValueError" instead of "KeyError" as
the problems are in the given values - role doesn't exist or
key is not used by a particular role.
Additionally, document the thrown exceptions in "Root.add_key" and
add a test which invokes that exception.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Root class has the functionality to add and remove keys for delegated
metadata (add_key()/remove_key()) but the other delegator Targets does
not.
It should provide the same/similar functionality.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
The spec does not say anything about role name uniqueness in a
delegations object, but I believe we cannot safely allow multiple roles
with the same role name in the roles array of a delegations object.
If we did then the roles could have different keyids, and then we would
end up in a situation where metadata may be both a valid delegation
and an invalid delegation at the same time, depending on how the role
gets chosen and that does not seem like the intention of the design.
There is an issue open in the specification with number 167 about
that issue.
Regardless of the Metadata API, I think we should enforce role name
uniqueness.
I chose to change the data structure containing roles to
OrderedDict, where keys are role names and values are DelegatedRole
instances.
This made sense to me as role names are the unique identifier of a role
and their order is important to the way they are traversed afterward.
Note: we can't use OrderedDict as type annotation until we drop support
for Python 3.6:
https://docs.python.org/3/library/typing.html#typing.OrderedDict
That's why I used quotes around "OrderedDict" annotation, because I
can't import it.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
In Timestamp, the only valid "meta" value is the dictionary representing
meta information for the snapshot file. This makes the API unnecessarily
complicated and requires validation that only information about snapshot
is available inside "meta".
Together with the python-tuf maintainers, we decided that snapshot meta
information will not be represented by a "meta" dictionary but instead
by a MetaFile instance and with this it will diverge from the
specification.
Additionally, to prevent confusion, I will rename the "meta" attribute
to "snapshot_meta" as this attribute will be related only to meta
information about snapshot.
This decision is coherent with ADR9 and the rationale
behind it is to provide easier, safer, and direct access to the
snapshot meta information.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Clarify the purpose of metadata API and that it's a low-level API
and as such it doesn't use concepts like "repository" or
"trusted collection of metadata" and don't implement the repository
logic or client updater workflow.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
This is a repository tooling use case but also helpful when testing.
It could be useful when we need to update the targets object.
Signed-off-by: Velichka Atanasova <avelichka@vmware.com>
The pylint warning W0703:broad-except was raised only
when six was used and python 2 was still supported.
The warning is no longer raised, the exceptions are
handled/raised correctly and the disabling can be removed.
Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
For targetpath: we don't want to support corner cases such as
file paths starting with separator.
Why this case should be threated specially than any other case where
you have multiple "/" for example "foo//bar/tar.gz"?
For pathpattern: it's recommended that the separator in the pathpattern
should be "/":
see https://theupdateframework.github.io/specification/latest/#targetpath
I believe it could lead to issues for a client implementation if it
supports arbitrary separators - every implementation needs to choose one
and stick with it.
Then, if we decide that "/" is our separator using lstrip on "os.sep" is
wrong, because the os separator from the server could be different that
the one used in the client.
Because of the above arguments, it makes sense to just remove
lstrip on os separators.
Additionally, document that the target_filepath and the DelegatedRole
paths are expected to be in their canonical forms and only "/" is
supported as target path separator.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
in the public API that we only support "/" as a
separator and don't handle corner cases such as leading separators
in either pathpattern or target_filepath.
By explicitly denoting the expected type of Metadata.signed
we help mypy understand our intentions and correctly figure
out types. This is entirely a typing feature and has no
runtime effect.
Modify the return type of Metadata.from_dict to match the
other factory methods (from_*).
Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
Needed in order to be compatible with the return type of
download_file (TemporaryFile is typed as IO[bytes]).
BinaryIO is a subclass of IO[bytes].
Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
According to the recently updated version of the specification the shell
style wildcard matching is glob-like (see https://github.com/theupdateframework/specification/pull/174),
and therefore a path separator in a path should not be matched by a
wildcard in the PATHPATTERN.
That's not what happens with `fnmatch.fnmatch()` which doesn't
see "/" separator as a special symbol.
For example: fnmatch.fnmatch("targets/foo.tgz", "*.tgz") will return
True which is not what glob-like implementation will do.
We should make sure that target_path and the pathpattern contain the
same number of directories and because each part of the pathpattern
could include a glob pattern we should check that fnmatch.fnmatch() is
true on each target and pathpattern directory fragment separated by "/".
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
After the addition of "path" argument in the TargetFile class the
filename argument in Targets.update() became redundant.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
