feat: add ability to backup systems that don't expose BulkExport API #597
Conversation
jzelinskie
force-pushed
the
backup-failover
branch
from
4af94d0 to
d7095c5
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #597 +/- ##
==========================================
+ Coverage 39.28% 40.77% +1.49%
==========================================
Files 37 38 +1
Lines 5448 4959 -489
==========================================
- Hits 2140 2022 -118
+ Misses 3063 2687 -376
- Partials 245 250 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jzelinskie
force-pushed
the
backup-failover
branch
from
850a8bb to
6ed4a57
Compare
jzelinskie
force-pushed
the
backup-failover
branch
2 times, most recently
from
9bd2ad4 to
100c8df
Compare
tstirrat15
force-pushed
the
backup-failover
branch
from
100c8df to
c1a0148
Compare
tstirrat15
left a comment
tstirrat15
left a comment
There was a problem hiding this comment.
This looks good to me; I'd also like to get maria's eyes on it.
internal/cmd/backup_restorer_test.go
Outdated
| oneConflictError = []error{errConflict} | ||
| ) | ||
|
|
||
| func TestRestorerFiltered(t *testing.T) { |
There was a problem hiding this comment.
Why did this test need to be broken out?
There was a problem hiding this comment.
I was actually going to break out the filtered vs non-filtered and just totally missed that there's 1 more test case there that uses a filter.
We have lots of duplicated tests that mostly just test filtering. I think it makes sense to try and tease them all out and just unit test the Rewriter exhaustively.
tstirrat15
force-pushed
the
backup-failover
branch
from
c1a0148 to
6f719f8
Compare
| require.NotNil(t, enc.enc) | ||
| } | ||
|
|
||
| func TestOcfEncoder_Append(t *testing.T) { |
There was a problem hiding this comment.
can we have similar tests that test OcfFileEncoder.Append()?
There was a problem hiding this comment.
I'm working on a follow up PR that pulls the rewriter into the encoder/decoder and tests each individually. That should help clean up a lot of these tests
| func (d *OcfDecoder) Next() (*v1.Relationship, error) { | ||
| if !d.dec.HasNext() { | ||
| return nil, nil | ||
| return nil, io.EOF |
|
|
||
| // OcfEncoder implements `Encoder` by formatting data in the AVRO OCF format. | ||
| type OcfEncoder struct { | ||
| w io.Writer |
|
|
||
| // NewRedactor creates a new redactor that will redact the data as it is written. | ||
| func NewRedactor(dec *Decoder, w io.Writer, opts RedactionOptions) (*Redactor, error) { | ||
| func NewRedactor(dec Decoder, w io.Writer, opts RedactionOptions) (*Redactor, error) { |
There was a problem hiding this comment.
Why was the pointer change necessary here?
There was a problem hiding this comment.
Decoder is an interface now and before that was a pointer to a concrete implementation
miparnisari
changed the title
miparnisari
force-pushed
the
backup-failover
branch
from
4af9b26 to
eeb9e9e
Compare
4 participants
Description
This PR adds backup support for systems without the bulk export. This is important for allowing folks to perform backups against AuthZed Serverless (as well as old SpiceDB deployments or ones where their API access is restricted).
I went about this change by refactoring a bit of logic in the create backup command and leaning into the Encoder abstraction in the
backupfmtpackage. Now the logic for persisting to a file and tracking progress on the CLI are isolated and implemented as their own Encoders.I didn't try to deduplicate any logic between the bulk export and legacy backup conditional because I expect us to be able to just delete the legacy backup block once AuthZed Serverless is fully sunset.
I did also change how we storing our cursor in the lock file. Previously we held a file descriptor open and kept rewriting it and now I create a new tempfile and atomically replace the lockfile when there's an update; this strategy is more robust to filesystem errors and means we have less bookkeeping.
As a follow-up, I also refactored how we were rewriting legacy prefixes, filtering prefixes, and added support for replacing prefixes by creating a Rewriter interface. This ensures the flags are also the same across commands.
Testing
I've manually ran this against some permission systems in AuthZed Serverless and written some basic unit tests for the encoder logic that was refactored.