Add client method for initializing from root metadata

[haydentherapper]

This will allow users of go-tuf to initialize a client
without requiring a remote connection. This is useful
for when a root has been updated and the local root
verification keys can no longer verify the the latest
remote root, except by verifying the chain with
client.Update().

Ref #208

Signed-off-by: Hayden Blauzvern [email protected]

[haydentherapper]

cc @asraa

Open question: Should we remove client.Init in favor of this method? Given that go-tuf does not have a 1.x.x release, I'd assume it'd be fine to drop support for client.Init without having to worry about deprecation.

[joshuagl]

Open question: Should we remove client.Init in favor of this method? Given that go-tuf does not have a 1.x.x release, I'd assume it'd be fine to drop support for client.Init without having to worry about deprecation.

The absence of a formal. 1.x.x release does technically absolve us of having to worry about deprecation. However, go-tuf has active users who have been bitten by API changes (i.e. #203). We should be wary about making any breaking API and behaviour changes until we have made a release that can be pinned to and formalised our releases. See #200 and #114

[asraa]

One comment more about client.Init generally: I think, as Hayden suggested from writing this method, that it doesn't always make sense that a client could only init from the latest root keys like client.Init does.

In fact, I think this new method which effectively just loads trusted root metadata matches the TUF spec better (https://github.com/theupdateframework/specification/blob/master/tuf-spec.md#load-trusted-root-metadata--load-trusted-root).

[hosseinsia]

To the point that @joshuagl made about active users, we are using the existing init ... so removing it would break us ...

[trishankatdatadog]

Do we have a release process (which includes making breaking MINOR 0.MINOR.PATCH releases for now) formalized? If so, we should formalize it first before merging this.

[coveralls]

Pull Request Test Coverage Report for Build 1825678182

• 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 70.23%

---

Totals
Change from base Build 1758586329:	0.2%
Covered Lines:	2168
Relevant Lines:	3087

---

💛 - Coveralls

[haydentherapper]

@joshuagl - Do you know when this release process will be formalized? I agree that it'd be nice to document breaking changes in a changelog. One thing to note is this would be a noticeable breaking change, so users who pull in the latest changes would be able to recognize that Init's params have changed.

+1 to @asraa's comment about this following the spec. It calls out that you should be loading a "trusted root metadata file", not just the root keys. It's worth mentioning again that ideally by default, Init would just "load", by initializing state from a known root, and update mechanisms should be kept separate.

@hosseinsia, as part of the refactor, we can change all of our usages of client.Init. I see we're using it in the tuf-client init command. We can deprecate the usage of the root-keys-file flag and switch to a root-metadata flag. This should actually make the command easier to use, since the keys format has some complexity, and users should always have the root metadata file available. Note that this won't impact tuf init, since that uses repo.Init.

[haydentherapper]

I've updated the PR, changing Init to only take the root metadata JSON. This commit could be split into its own PR, but I think that we should decide when to make the breaking change to change the parameters of Init. My preference would be making the change in this PR.

[asraa]

ping on this! @hosseinsia

I would even by happy to keep Init as before and make a new method like InitLocal, and then deprecate/rename Init on a release?

[hosseinsia]

@asraa
Yes please. That would not break us and makes the transition much smoother. Thank you!

[haydentherapper]

@hosseinsia, how should we communicate the differences between Init and InitLocal to convince users to switch over? And furthermore, if users do start switching over to InitLocal, then we'll end up breaking them when we remove the method in the future.

This change shouldn't break anyone who's using a pinned version of go-tuf. Once someone updates to the latest commit in go-tuf, their build will break, but it should be clear what the issue is. Either we can include changelog with this commit, or just assume that the documentation on the function is clear.

It's also worth noting that impact is low according to https://cs.github.com/?scopeName=All+repos&scope=&q=%22github.com%2Ftheupdateframework%2Fgo-tuf%2Fclient%22+AND+%22.Init%22+AND+-path%3Acosign+AND+-repo%3Atheupdateframework%2Fgo-tuf - I've looked through these and mostly see either usage with Sigstore maintainers or go-tuf maintainers.

I'd prefer that we make the change in function signature now. This will make Init align to the TUF specification, since Init does not go through the secure root update flow.

[hosseinsia]

Potentially, in a release note, we can specify that the InitLocal is recommended over the existing method. Also, we can update the comment the init and tag it as deprecated to decentivize the usage (see deprecated here https://go.dev/blog/godoc). regardless of whether we keep or remove the init, we have to do it in two PR to make the roll-back easier. WDYT?

[haydentherapper]

@hosseinsia I've updated the PR and added a deprecation comment to Init. Please take another look!

I'll leave #208 open until a versioned release process is in place, when we can revisit this issue and remove Init.

[hosseinsia]

LGTM. Thanks!

[asraa]

@hosseinsia staticcheck doesn't like seeing usage of Init in the codebase now that it's deprecated. Can we leave Init as an public method of the client API, and switch over the CLI and tests to use InitLocal? That shouldn't break your usage of go-tuf's Client as it's deprecated.

(Also, this still won't break anyone unless they manually update the pinned sha of go-tuf)

@trishankatdatadog

[hosseinsia]

@asraa good resolution. Thanks!

[haydentherapper]

Thanks all for the feedback! Updated the client to switch to InitLocal.

[hosseinsia]

LGTM