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

Move Tap14 to deferred with some additional clarification of repository operations #162

Merged
merged 8 commits into from
Mar 19, 2024

Conversation

mnm678
Copy link
Contributor

@mnm678 mnm678 commented Oct 4, 2022

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.

Copy link
Member

@JustinCappos JustinCappos left a 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...

JustinCappos
JustinCappos previously approved these changes Oct 11, 2022
Copy link
Member

@lukpueh lukpueh left a 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 version X.Y.Z metadata lives in an X directory.

  • How does root-filename evolve over root versions? E.g. 2/2.root.json and 2/20.root.json can't both say {"version": 3, "root-filename": "1.root.json", ...}, because 3/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 between 2/2.root.json and 2/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 the supported_versions entries for 2 and 3 likely look like in 3/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.

Signed-off-by: Marina Moore <[email protected]>
Signed-off-by: Marina Moore <[email protected]>
@mnm678
Copy link
Contributor Author

mnm678 commented Oct 20, 2022

@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]>
@mnm678
Copy link
Contributor Author

mnm678 commented Oct 31, 2022

The implementation now reflects the changes in this TAP. @lukpueh @JustinCappos this is ready for review

@mnm678 mnm678 marked this pull request as ready for review September 12, 2023 19:11
@mnm678 mnm678 requested a review from JustinCappos September 12, 2023 19:11
Copy link
Member

@JustinCappos JustinCappos left a 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.)

tap14.md Outdated Show resolved Hide resolved
Signed-off-by: Marina Moore <[email protected]>
JustinCappos
JustinCappos previously approved these changes Nov 6, 2023
Copy link
Member

@JustinCappos JustinCappos left a 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]>
@mnm678 mnm678 changed the title Move Tap14 to accepted with some additional clarification of repository operations Move Tap14 to deferred with some additional clarification of repository operations Mar 19, 2024
Copy link
Member

@JustinCappos JustinCappos left a 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This field will serve two purpose.
This field will serve two purposes.

@lukpueh lukpueh merged commit 80e6734 into theupdateframework:master Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants