-a only support up to 10 versions

[timff]

With 1.7 release, 'credstash put foo bar -a' only support up to 10 versions.

After that 'credstash put foo bar -a' always update the value of 10th version,
and 'credstash get foo' always return the value of the 9th version.

[alex-luminal]

The root of the issue here is that we store the version as a string, and when we ask DDB to sort by range key (version), it does so lexicographically (which is a reasonable thing to do), when we want it sorted numerically.

My knee-jerk response is that we should change the table schema to store versions as numbers instead of as strings. That would be a breaking change and require that everyone recreate their DDB table. We could make it easy by having a migrate function that copies all the old table values to the new table, but it's still going to be a hassle.

The alternative would be to fetch all the versions for a given secret, and then do the sort locally. The big downside here is that as people have wither lots of secret versions, or larger secrets, this operation will consume more read capacity.

Both options have downsides. Requiring a table migration is a pain once, but avoids a bunch of pain in the future. Doing the sort locally means you dont have to migrate the table, but you pay a penalty on every operation forever.

I'm a fan of doing a simple migration command and moving version to numbers.

What do other people think?

[dom-at-fugue]

Changing version to numbers will also break credstash for anyone using a non-numerical versioning scheme, so dates, code names, and n.n.n releases would be right out.

From a cursory glance at the versioning code, I think there might be another alternative. Could you modify the autoversioning (line 468) to left-pad integer strings so that they are of a fixed size large enough to accommodate a common value of sys.maxint? Though it feels a bit hacky, it's not a suggestion given out of laziness; rather, it would offer support for the most use cases, and the overhead & complexity is relatively small.

I'm also curious to know if any credstash users actually are using custom, non-numerical versioning schemes. If nobody is doing that, then I'm +1 to migration to numbers.

[nathanatluminal]

Assuming dynamo sorts what dom proposes properly, that seems to be the best all-around fix. Given that this is a version value that credstash is maintaining, creating an arbitrary rule that has the side effect of correct behavior seems permissible.

One caveat would be that if credstash ever got ported to a different datastore, sort results might be different.

[alex-luminal]

I had thought about left-padding, but couldn't figure out how far to go. sys.maxint on amazon linux or something is probably a good bet. Thanks for the suggestion @dom-at-luminal. I'll play with that

[chothia]

Just fyi, I am seeing this in version 1.6 as well.

We don't use custom, non-numerical versioning and would be fine with doing a migration.

[alex-luminal]

@chothia what we're looking at doing is having the -a flag left-pad numbers with zeros so that they sort properly, and then have a migration script to pad existing versions. See #54

probably going to merge it this weekend. Just want to do more testing and update docs

[michel-slm]

Is this bug fixed? We are looking at deploying credstash here at work, so this issue is of some interest.

[alex-luminal]

@michel-slm there is a PR open with a fix. I want us to add some clarification to the README about how auto-versions will work after the patch. The patch should get merged in in the next few days