refactor(storage/fs): adjust the declarative storage abstractions

[GeorgeMac]

Fixes FLI-747

I will hold off on doing this until #2538 is delivered and this has been updated to include that work.
To avoid causing any disruption there and so @erka can get his work in 👍 .

This change is going to mix up the abstractions a little.
The motivation is this will help with some work coming up, where a couple of the backends will actually support multiple versions of snapshots (keyed by their git tag / revision or oci tag / digest). Clients will be able to request snapshots by reference or potentiall even revision.

The refactor drops the idea of fs.SnaphotSource altogether.
Instead, it introduces the concept of fs.SnapshotStore:

// SnapshotStore is a type which has a single function View.
// View is a functional transaction interface for reading a snapshot
// during the lifetime of a supplied function.
type SnapshotStore interface {
	// View accepts a function which takes a *StoreSnapshot.
	// The SnapshotStore will supply a snapshot which is valid
	// for the lifetime of the provided function call.
	View(func(storage.ReadOnlyStore) error) error
	fmt.Stringer
}

This new store exposes a kind of funtional transaction interface (similar to e.g. boltdb).
Instead of making the top-level fs.Store managed locking and handle subscribing to changes, it instead pushes the synchronization responsibilities down onto the snapshot stores themselves (previously called sources).
Now the individual stores have to take care of managing how and when to update their snapshots.

To achieve this, the new abstraction exposes a function View() which itself takes a function.
The function provided to View is given the snapshot instead.
It is on the store implementers to call the provided function inside a read lock.
The write lock can and is held by the store when changing the snapshots themselves.

This allows the top-level fs.Store to define what happens per read-lock, without having to own the lock itself.
In subsequent change, the View() function will be updated to take a target reference.
This will allow the various store implementations to support managing multiple snapshots for different references.
Whether or not a store supports arbitrary references, will be up to the store (initially Git and OCI will).

[github-actions]

Uffizzi Ephemeral Environment deployment-42935

☁️ https://app.uffizzi.com/github.com/flipt-io/flipt/pull/2540

📄 View Application Logs etc.

⏰ This Preview will be destroyed in 1 hours at: Fri Dec 15 14:13:06 UTC 2023

What is Uffizzi? Learn more!

[codecov]

Codecov Report

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

Comparison is base (0466c1f) 70.85% compared to head (0ce99f1) 71.32%.

❗ Current head 0ce99f1 differs from pull request most recent head c6e880f. Consider uploading reports for the commit c6e880f to get more accurate results

Files	Patch %	Lines
internal/storage/fs/store.go	71.27%	54 Missing ⚠️
internal/storage/fs/poll.go	0.00%	34 Missing ⚠️
internal/storage/fs/git/store.go	82.00%	11 Missing and 7 partials ⚠️
internal/storage/fs/s3/store.go	76.81%	12 Missing and 4 partials ⚠️
internal/storage/fs/oci/store.go	71.42%	8 Missing and 4 partials ⚠️
internal/storage/fs/local/store.go	84.21%	4 Missing and 2 partials ⚠️
internal/cmd/grpc.go	0.00%	3 Missing ⚠️
internal/storage/fs/snapshot.go	96.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2540      +/-   ##
==========================================
+ Coverage   70.85%   71.32%   +0.47%     
==========================================
  Files          82       82              
  Lines        8193     8053     -140     
==========================================
- Hits         5805     5744      -61     
+ Misses       2042     1958      -84     
- Partials      346      351       +5     

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

[erka]

@GeorgeMac please don't wait for me. It looks like it will take some time to finish Azure blob storage

[GeorgeMac]

Cheers for the heads up @erka 🙏 If I do end up getting this in before, I am happy to point out how the interfaces have changed. So you can adjust accordingly. I can update the open issue too with more up to date details.

[markphelps]

[GeorgeMac]

Just don't open that new package and it's like all that code doesn't exist.

[markphelps]

this is amazing! love these abstraction changes!!

[markphelps]

@erka let me know if I can help unblock in any way with the Azure blob store changes! Thank you for taking that on

[erka]

@markphelps this PR doesn't impact my work much. The challenge for me is Aruzite, azure-sdk-for-go and auth between them for testing. I also have never used dagger and have white spots.

[markphelps]

@markphelps this PR doesn't impact my work much. The challenge for me is Aruzite, azure-sdk-for-go and auth between them for testing. I also have never used dagger and have white spots.

gotcha. I'm happy to help with getting the Dagger tests working as well with Azurite (never used Azurite but looks cool)