Versioned object support

[shabbyrobe]

Thought I should open this as WIP in case there's any feedback, and to make the progress visible.

This will only support versioning in the memory backend for now: the iteration patterns are quite hard to fully satisfy. I think it will be tricky to support in the disk-based backends and I may opt to defer implementing this support there until a later PR if it gets too big/hard/nasty.

TODO:

• Version listing in s3mem
• HEAD version
• Validate assumptions about prefixes and markers in listing API
• Tests
• List versions should work with unversioned bucket (may require further work on pagination)

[codecov-io]

Codecov Report

Merging #28 into master will decrease coverage by 11.49%.
The diff coverage is 69.69%.

@@           Coverage Diff            @@
##           master    #28      +/-   ##
========================================
- Coverage   76.49%    65%   -11.5%     
========================================
  Files          14     26      +12     
  Lines        1021   2260    +1239     
========================================
+ Hits          781   1469     +688     
- Misses        167    568     +401     
- Partials       73    223     +150

Impacted Files	Coverage Δ
backend/s3afero/util.go	33.33% <ø> (ø)
validation.go	85.71% <ø> (+22.55%)	⬆️
time.go	53.33% <0%> (+8.88%)	⬆️
option.go	66.66% <100%> (+4.16%)	⬆️
hash.go	83.87% <100%> (-5.79%)	⬇️
uploader.go	85.39% <100%> (-2.46%)	⬇️
backend/s3mem/versionid.go	100% <100%> (ø)
error.go	68.29% <20%> (+1.62%)	⬆️
backend/s3afero/multi.go	37.54% <34.61%> (ø)
backend/s3afero/single.go	40% <37.5%> (ø)
... and 24 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b60435e...a33f5a9. Read the comment docs.

[shabbyrobe]

This PR is getting pretty epic. I've started adding a little dump-bucket for testing assumptions about how S3 itself works. It's a bit verbose at the moment and doesn't properly clean up after itself; I'll incrementally improve it over time. For now though, it has really helped work through some of the assumptions and corner cases in the S3 API.

[shabbyrobe]

OK I'm pullin' this monster out of WIP 😄 🎉

There ended up being a LOT of stuff that went in to making this work. I've tried to keep the versioning stuff optional for backend implementers. Essentially, I think we can offer a bit of an a-la-carte API to GoFakeS3 for users: implement what you need (as long as you implement the Core backend) and you should be able to get up and running with something mostly S3-ish for your needs without the rest of it. I realised that by the time we have absolutely everything in here somewhere (if that ever happens!), the API surface someone who wants to make a backend would have to implement would be insane!

I haven't implemented versioning for the bolt or afero backends because it will take a lot of extra work to work out how, and I'd like to work on proper bucket listing pagination first (it's one of the next things on my list). This only supports versioning for the in-memory backend for now.

I kept running into situations where I was writing FIXME comments that said "find out what S3 actually does here", so I put together a really simple scratchpad scripting project in the internal/s3assumer directory. It's full of scripts that follow hopefully somewhat commented narratives to explain what detail they try to flush out of S3 itself, with a few assertions thrown in. It works against S3 itself with a real bucket; I recommend setting up a lifecycle that deletes everything it creates after a set number of days (I'll pull a cloudformation script together that does this for you at some point). s3assumer does not do much in the way of cleaning up after itself yet! I figured that was a bit more of a "yak shave" than I could be bothered with, but eventually I suspect I'll add it. Each test script in the assumer starts with a code number which I have then referenced in the gofakes3 code to link back to the explanation. I flushed out a heap of useful detail this way, I was surprised at how much it helped.

I also had some trouble getting decent coverage reports considering the main test cases are more functional than unit tests (unit tests aren't especially useful here for the bulk of what we're doing). Go's testing tool isn't great at reporting coverage properly by default, so I've thrown together a simple script that calculates it correctly (I think) in magefile.go, which is run using mage. I noticed the Makefile for the first time after I added it (oops!) but I'm not sure it's easily converted from Go to shell-in-make. How would you like me to proceed? I can probably convert it to a standalone go run program too if you'd prefer.

From a testing point of view, I've focused primarily on rudimentary functional coverage of all the new stuff. I think there's not nearly enough test cases around the version listing, but once again I'd like to hold off going too test-crazy until I put a proper backend-certifying tester together.

[johannesboyne]

That was a big one again ;-)

I also had some trouble getting decent coverage reports considering the main test cases are more functional than unit tests (unit tests aren't especially useful here for the bulk of what we're doing). Go's testing tool isn't great at reporting coverage properly by default, so I've thrown together a simple script that calculates it correctly (I think) in magefile.go, which is run using mage. I noticed the Makefile for the first time after I added it (oops!) but I'm not sure it's easily converted from Go to shell-in-make. How would you like me to proceed? I can probably convert it to a standalone go run program too if you'd prefer.

Let's remove the Makefile and settle on one tool like mage, we have to check whether the CircleCI stuff is somehow depending on it but I assume this would be a very minor change. Maybe worth adding that change in another PR.

Just for my understanding, is the reported codecov project coverage of 66.06% wrong or did you already changed codecov to use the magefile calculated one?

From a testing point of view, I've focused primarily on rudimentary functional coverage of all the new stuff. I think there's not nearly enough test cases around the version listing, but once again I'd like to hold off going too test-crazy until I put a proper backend-certifying tester together.

As the versioning is only applicable for some backends, the backend-certifying tester is probably more important, I agree!

[shabbyrobe]

That was a big one again ;-)

Yeah, sorry 'bout that, it turned out to be a fairly large feature, much larger than I anticipated.

Just for my understanding, is the reported codecov project coverage of 66.06% wrong or did you already changed codecov to use the magefile calculated one?

I haven't changed codecov, I didn't realise I had access to that otherwise I would've tried! 66% is kinda unsatisfyingly low, really, considering we were already at over 70. Let me see if I can add a couple more tests real quick. Unfortunately, once we get whole-project coverage, the number will drop sharply again though, as we have very patchy coverage of the other stuff in the backends and we won't recover until the backend certifying tester is up and firing (not sure when I'll get to it).

EDIT: codecov says the coverage is 74.04% now, which is much more in line with what I was expecting! Must've had something to do with my Git mistake - the numbers you were seeing sounded like the numbers before I added a bunch more tests.

Are you happy with mage as a runner tool? If so, I can move things from the Makefile to the magefile now. Are there any alternatives you prefer?

[shabbyrobe]

Wait a second, I've broken something pretty badly here. There were changes I pushed up that seem to have vanished? I force-pushed after rebasing of master last night, but I have no idea what I've done wrong as a stack of fixes and extra tests are nowhere to be found.

Sorry about this. Please give me a sec to find and fix what I've done wrong.

[shabbyrobe]

Phew! Fixed it! Git fail... Ooooooops!

[shabbyrobe]

Hey wow! 64.91% is better than I expected! I thought we'd take a much bigger hit. I think there might still be some things not being included in the report but it's definitely showing all of the backends except s3bolt now.

I got rid of the magefile because it was a pain in the backside to get it running in Circle. I've replaced it with a tools script (using the old // +build ignore trick I stole from the Go stdlib)

[johannesboyne]

I got rid of the magefile because it was a pain in the backside to get it running in Circle. I've replaced it with a tools script (using the old // +build ignore trick I stole from the Go stdlib)

Now with the // +build tools command seems to be quite promising!