-
Notifications
You must be signed in to change notification settings - Fork 3
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
Allow a TUF client to override the system clock #22
Conversation
2b114cf
to
611867e
Compare
Uptane tests continue to pass with this merged, and the Uptane demo continues to run with this merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
You may consider addressing my two minor non-blocking comment-related comments.
Also, I grepped through the repo for time()
and while in the implementation itself it looks like you updated everything, I saw that related unit tests still use time.time()
. I suppose you are aware and checked that the tests still behave as intended?
tuf/repository_lib.py
Outdated
@@ -750,13 +750,13 @@ def _log_warning_if_expires_soon(rolename, expires_iso8601_timestamp, | |||
""" | |||
|
|||
# Metadata stores expiration datetimes in ISO8601 format. Convert to | |||
# unix timestamp, subtract from from current time.time() (also in POSIX time) | |||
# unix timestamp, subtract from from current time (also in POSIX time) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really part of your change, but while you're at the line... s/from from/from/
tuf/client/updater.py
Outdated
current_time = int(time.time()) | ||
# compare it against the current time from tuf.util.get_current_time(), | ||
# which is also in Unix/POSIX time format, although with microseconds | ||
# attached, and allows manual override of the system clock.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't you cutting off microseconds with the int
cast?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right, that's no longer being done in this section, but now in the called function, so I should remove that note.
I test this fork of TUF with the Uptane tests only. 😢 The upcoming Uptane PRs will test this functionality by testing the Primary and Secondary. EDIT: to clarify, I mean the automated CI testing.... I test this fork manually, and use the CI testing for Uptane itself, which is the only client of this code. It's not ideal, but this fork isn't long for this world at this point. |
to some value that they trust and will manually update. This is of use if, say, a trusted time is periodically retrieved from some other source, and the clock will stand still until it is updated this way. For example, Uptane has use for this functionality. Edit: also includes two minor comment corrections suggested by Lukas. Signed-off-by: Sebastien Awwad <sebastien.awwad@gmail.com>
611867e
to
8412e45
Compare
Allow a TUF client to override the system clock to some value that they trust and will manually update.
This is of use if, say, a trusted time is periodically retrieved from some other source, and the clock will stand still until it is updated this way. For example, Uptane has use for this functionality.