-
Notifications
You must be signed in to change notification settings - Fork 20
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
Move Tap14 to deferred with some additional clarification of repository operations #162
Conversation
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.
FYI: the main index of TAPs also needs to be updated to change the status to accepted...
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.
Some comments, which I think should be addressed before accepting the TAP
-
This PR has conflicts.
-
The TAP does not point to a prototype implementation, and the existing prototype implementation (PoC: Tap 14 python-tuf#2114) does not represent the current state of the TAP yet.
-
The recently merged PR #158 had a few spelling mistakes and typos in the last commit (e.g.
supported-versions
with dash instead of underscore) -
The
supported_versions.path
field seems to be redundant with the convention that spec versionX.Y.Z
metadata lives in anX
directory. -
How does
root-filename
evolve over root versions? E.g.2/2.root.json
and2/20.root.json
can't both say{"version": 3, "root-filename": "1.root.json", ...}
, because3/1.root.json
needs to be signed by "the root key from the most recent metadata [at the time of creating the new root (??)] in the previous major specification version" and keys may have changed between2/2.root.json
and2/20.root.json
.Does this mean that root updates need to be synchronized across multiple versions and
root-filename
needs to be bumped on each root update? IMO that seems necessary for clients, in order to update spec version coming from different root versions. But it also makes the different spec versions less autonomous. Either way I think the TAP should clarify this in the "repository updates" section.Besides, how does
root-filename
evolve for the own and previous versions? E.g. if a repo supports spec versions 2, 3 and 4, what would thesupported_versions
entries for 2 and 3 likely look like in3/8.root.json
? -
I'd generally like the client update process to be more algorithmic also with regards to the existing client workflow. E.g. @trishankatdatadog suggests that the client should rotate root before looking at
supported_versions
. Is that the case? The TAP does not make that clear.
ad1c6b8
to
4870cea
Compare
Signed-off-by: Marina Moore <[email protected]>
Signed-off-by: Marina Moore <[email protected]>
4870cea
to
202cd0a
Compare
Signed-off-by: Marina Moore <[email protected]>
Signed-off-by: Marina Moore <[email protected]>
Signed-off-by: Marina Moore <[email protected]>
@lukpueh I addressed your text concerns, and will be updating the prototype implementation soon. Hopefully the implementation will further clarify the root update process. |
Signed-off-by: Marina Moore <[email protected]>
The implementation now reflects the changes in this TAP. @lukpueh @JustinCappos this is ready for review |
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.
I looked and the recent changes are fine with me. (Modulo a minor edit below.)
Signed-off-by: Marina Moore <[email protected]>
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.
I approve
Signed-off-by: Marina Moore <[email protected]>
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.
Moving to deferred. Lukas and others will add other changes.
@@ -601,7 +631,7 @@ in the specification section. | |||
# Augmented Reference Implementation | |||
|
|||
Semantic Versioning was added to the TUF Reference Implementation in [#914](https://github.com/theupdateframework/python-tuf/pull/914). | |||
The rest of this proposal has not yet been implemented. | |||
The rest of the proposal is implemented as a proof of conceps in [#2114](https://github.com/theupdateframework/python-tuf/pull/2114). The pr can be finalized once this TAP is accepted. |
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.
The rest of the proposal is implemented as a proof of conceps in [#2114](https://github.com/theupdateframework/python-tuf/pull/2114). The pr can be finalized once this TAP is accepted. | |
The rest of the proposal is implemented as a proof of concept in [#2114](https://github.com/theupdateframework/python-tuf/pull/2114). The pr can be finalized once this TAP is accepted. |
@@ -253,21 +256,74 @@ specification versions in new | |||
directories. In addition, root metadata on a repository MUST add a `supported_versions` field | |||
to indicate which specification versions are supported. | |||
|
|||
In order to fascilitate the upgrade to a new specification version, root metadata for all supported spec versions MUST add a `supported_versions` field before upgrading to spec version 2.0.0. | |||
This field will serve two purpose. |
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.
This field will serve two purpose. | |
This field will serve two purposes. |
Requires #158
Please ignore all but the last 3 commits, this is already rebased onto #158 to prevent conflicts. I'll clean up the history here once that is merged.
This pr adds some requested clarification of repository operations, and proposes moving this TAP to accepted based on the PoC in python-tuf.