Added an unsafe method for loading the tuf metadata on disk

[kommendorkapten]

No description provided.

[codecov]

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (3b7ccba) 71.65% compared to head (19d9861) 71.43%.

Files	Patch %	Lines
metadata/updater/updater.go	58.33%	10 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
- Coverage   71.65%   71.43%   -0.23%     
==========================================
  Files          10       10              
  Lines        1976     2013      +37     
==========================================
+ Hits         1416     1438      +22     
- Misses        453      463      +10     
- Partials      107      112       +5     

Flag	Coverage Δ
Go-1.21	71.43% <59.45%> (-0.23%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rdimitrov]

@kommendorkapten - Thanks for this 💯

What would you think about:

• Creating a setting in the updater configuration that we can toggle for allowing to work in this unsafe mode
• Make this a private method to updater
• Extract the current content of the Refresh into another private method
• Upon calling Refresh, we'll decide which method to call based on the setting (default being the safe one)?

Other:

• What is the best way to incorporate that setting with enabling Refresh to be called multiple times without failing? Perhaps we can have separate settings for each (one to work in offline mode, the other to allow for multiple calls of Refresh?) or something else?

[kommendorkapten]

Yeah, having this as a setting seems more nice.

[kommendorkapten]

@rdimitrov updated to use a parameter instead. Note that calling Refresh multiple times will still fail, I can work around that so it's not important I believe.

[kommendorkapten]

If you think this is good, I'll start to work on getting the tests updated.

[rdimitrov]

Looks good! 👍

While doing the tests can you verify if this option works together with DisableLocalCache and if they affect each other, add a check that we cannot set both at the same time?

[kommendorkapten]

While doing the tests can you verify if this option works together with DisableLocalCache and if they affect each other, add a check that we cannot set both at the same time?

Is this really needed?

• DisableLocalCache makes sure nothing is persisted to the filesystem.
• UnsafeLocalMode makes sure nothing is fetched from the network.

I think it makes sense to allow them together, as even with UnsafeLocalMode, the provided root.json is actually written to disk.

[rdimitrov]

While doing the tests can you verify if this option works together with DisableLocalCache and if they affect each other, add a check that we cannot set both at the same time?

Is this really needed?

• DisableLocalCache makes sure nothing is persisted to the filesystem.
• UnsafeLocalMode makes sure nothing is fetched from the network.

I think it makes sense to allow them together, as even with UnsafeLocalMode, the provided root.json is actually written to disk.

Actually I did not put a lot of thought on it while I asked that, just thought to bring that up to you so if someone enables both it doesn't accidentally interfere with the behaviour of this patch 👍