feat: Add Google Drive storage backend with custom endpoint support

[luizguilhermesj]

Relates to #471

• Implement Google Drive storage backend using service account authentication
• Add support for custom Google Drive API endpoints for testing
• Include comprehensive test setup with mock services
• Add configuration options for Google Drive credentials and folder ID
• Update documentation with Google Drive configuration examples

[m90]

Thanks a lot for following up on this so quickly. Two questions before I have a closer look:

1. the integration test you added is failing, it seems pruning is either broken or expectations are wrong, do you have any idea why?
2. The linter left a tiny complaint about using a deprecated package, let me know if you need any help with fixing that

[m90]

Is it also possible to pass the JSON as a string instead of as a file location here? In case yes, we could leverage the existing configuration mechanism so that:

1. People can pass a JSON string to GOOGLE_DRIVE_CREDENTIALS_JSON if they don't want to / can't mount the file
2. People can pass a file location using GOOGLE_DRIVE_CREDENTIALS_JSON_FILE which is automatically provided by the current configuration package

[luizguilhermesj]

I was trying to aim to only accept FILE, as it is an oddly long string to be putting on an envvar, but gemini got lost at some point and added it for me.
Also, I know _FILE is a pattern everywhere, but I have not seen it on other variables here, should we add it?

Let me know if you think just the JSON is enough, or if you prefer both so I an either remove that logic or fix the envvars and docs.

[m90]

Also, I know _FILE is a pattern everywhere, but I have not seen it on other variables here, should we add it?

_FILE is supported automatically. If you define FOO_BAR, FOO_BAR_FILE will be accepted as well. I would also think passing the entire JSON blob as an env var will rarely be done, but like this, it'd be consistent with what's already there.

[luizguilhermesj]

Thanks a lot for following up on this so quickly. Two questions before I have a closer look:

1. the integration test you added is failing, it seems pruning is either broken or expectations are wrong, do you have any idea why?
2. The linter left a tiny complaint about using a deprecated package, let me know if you need any help with fixing that

The tests were mainly copied from dropbox. But they were anchored by messages from storage.go, so I think it would be better to decouple it and fail with any ERROR message.
So I simplified it a lit bit and I think it is reporting both errors and success more clearly now.
Plese let me know if there`s a better way or if it should respect the previous pattern.

I'm not sure if its a pattern, but the set -e makes the tests pass without errors, despite logs=$(docker compose exec backup backup) failing.
So I wrapped that command with set +e and set -e to output the error to the log make it fail when it should, WDYT?

For the lint issue, I was about to ask for your help, but then I asked Gemini and he did this.
The tests are still passing, so I think it is correct? 😅

[m90]

So I wrapped that command with set +e and set -e to output the error to the log make it fail when it should, WDYT?

This must mean that now the backup command exits non-zero in this scenario, which is unwanted (hence, the Dropbox test doesn't need it). I'll need to look into the code why that happens.

For the lint issue, I was about to ask for your help, but then I asked Gemini and he did this.

Exactly this. Thanks,

[m90]

Looking into the tests more closely, I think the underlying problem is that the mock is not fully featured yet.

What the test tries to do:

• Create an initial backup
• Setting a retention policy of 0, run another backup and see that the image refuses to delete all backups (as all were eligible)
• Using a retention policy of 7, run another backup and see that things work as intended

What the test currently does:

• Create an initial backup
• Query the mock Google Drive for existing files, receive 0 files, so there's nothing to prune (fails the Dropbox assertions)
• Query the mock Google Drive for existing files, receive 0 files again, so there's nothing to prune

This is why there are assertions on the log output which are currently missing from the googledrive test case. An alternative would be looking at the files itself which is what the s3 test case does.

[luizguilhermesj]

hey @m90 thanks for your kind review.
Let me take a better look at it during the weekend (when I have more time) and I'll pay closer attention to what the AI is doing so I can stop pushing silly mistakes like that 😅
(and also properly implement the _FILE)

[luizguilhermesj]

hey @m90, comming back to it =]

So I wrapped that command with set +e and set -e to output the error to the log make it fail when it should, WDYT?

This must mean that now the backup command exits non-zero in this scenario, which is unwanted (hence, the Dropbox test doesn't need it). I'll need to look into the code why that happens.

I took a better look at dropbox, and it seems like it is the same behavior =/

1. I've changed the user_v2.yaml there so the response for creating a folder would fail: screenshot
2. Then I run the tests: screenshot
3. Then I added set +e and re-run: screenshot

Without further looking into code, I guess we don't want non-zero exit codes in order to prevent the container from dying due to one backup failure, so the schedule (and possibly other backends) can carry on, am I right?

With that guess, I figured it would be good to test this failure on a normal run as well, but it didn't fail, it actually behaved like I was expecting the tests to (keeping the container alive and displaying the error message): screenshot

I have yet to learn how to properly debug golang, so my hope is that you can easily spot what is going on 😅
My best guess would be that there's something in the normal flow preventing exit codes from overflowing to the container, but in the test env it is either not implemented or not working properly.

Any ideas?

I didn't have much time this weekend 😢 so I couldn't look into the other 2 issues, but:

• _FILE:
  • It seems easy, I'll do it as soon as possible
• Tests:
  • The AI generated a very dummy mock api that doesn't really do anything, therefore the tests are also not testing anything. I'll take some time (next weekend) to think what test cases would be useful here and how to properly structure them.

[m90]

I think the Dropbox tests work as expected. When querying for what files exist, the mock will always return two existing files, which are backdated by these lines:

sed -i 's/SERVER_MODIFIED_1/'"$(date "+%Y-%m-%dT%H:%M:%SZ")/g" $SPEC_FILE
sed -i 's/SERVER_MODIFIED_2/'"$(date "+%Y-%m-%dT%H:%M:%SZ" -d "14 days ago")/g" $SPEC_FILE

so the tests go kind of like this

• Create an backup and test if this exits cleanly, pruning is not considered
• Query the mock Dropbox for existing files, receive 2 files, everything would be pruned, which is not done, printing a warning. Assert that the warning is there
• Query the mock Dropbox for existing files, receive 2 files again, one of the old enough to be pruned, print the expected line. Assert that the corresponding log line is printed

When I change the mock like you shown in the screenshot, I get a failing test (exits 1), which is expected.

My best guess would be that there's something in the normal flow preventing exit codes from overflowing to the container, but in the test env it is either not implemented or not working properly.

Since all test cases use set -e, failures propagate correctly. There are also test cases that check for non-zero exit codes, calling set +e beforehand (e.g. collision). Are you sure you aren't confusing what set -e and set +e do?

---

Even if it's not perfect, I think we could get the same done for Google Drive by adding a proper response here: https://github.com/offen/docker-volume-backup/pull/613/files#diff-3af2262cc057a6c13dfc4c4b0924326705ddfee2a56a7492bc7690194437dc73R95-R119

[luizguilhermesj]

When I change the mock like you shown in the screenshot, I get a failing test (exits 1), which is expected.

Sorry, I just checked and it does exit with code 1.

I assumed it didn't because there's no output.
Why do we have output on a normal run but not on tests?
Is there a reason for that?
(please forgive my ignorance, I'm a front-end guy with poor knowledge on shell scripts and ZERO on golang)

But anyways, I was stuck on this thinking I was doing something wrong, but turns out that GoogleDrive behaves the same.
So if it is expected, we are all good 🎉

Are you sure you aren't confusing what set -e and set +e do?

Bro, I had to google it to understand what it does and I'm not completly sure I got it. I just reported what I did with the help of AI 😅 which was to "wrap" the backup exec with it, like this.

Even if it's not perfect, I think we could get the same done for Google Drive by adding a proper response here: https://github.com/offen/docker-volume-backup/pull/613/files#diff-3af2262cc057a6c13dfc4c4b0924326705ddfee2a56a7492bc7690194437dc73R95-R119

That's the plan!
I'll get back to it this weekend 😉

[m90]

Why do we have output on a normal run but not on tests? Is there a reason for that?

This is not ideal, but there is indeed a reason for that. Looking at the code:

logs=$(docker compose exec -T backup backup)

echo "$logs"
# Do some assertions on the log output

This means $logs will only be printed when the docker compose exec -T backup backup commands exits 0. If it does not exit 0, the script will exit immediately as set -e is set. Thus, the line where logs would actually get printed is never reached.

Maybe it'd be a good time now to fix this and use some tee (as described here: https://unix.stackexchange.com/a/724029/437580)

logs=$(docker compose exec -T backup backup | tee /dev/stderr)
# Do some assertions on the log output

---

Let me know if there are any open questions still, happy to help out.

[luizguilhermesj]

I updatde my fork main and the PR was automatically closed 😅

Maybe it'd be a good time now to fix this and use some tee

Awesome! That solves it! =]

Tests are updated, working exactly like the dropbox one.

Will add the _FILE now.

[luizguilhermesj]

_FILE support fixed!

Please let me know if there's something else that I missed or could be improved =]

[m90]

Looks great, there's one tiny issue with the test output still, but if this is fixed, I would say this is ready to be shipped. Thanks a lot!

[m90]

This (and the two occurences above) isn't needed anymore now that tee is used and currently duplicates the log output, which can be a bit confusing. It's probably better to remove these.

[luizguilhermesj]

aw, true, I missed that.
Done!
Thank you =]

[m90]

This is now released in v2.44.0, thank you 🎩