mock_s3 fails to check for syntatically valid but invalid keys

[amarjandu]

mock_s3 fails to check if a VersionID specified is syntactically correct yet invalid key.

In the two requests below:

1. A syntactically invalid VersionID invalid is used, this throws a InvalidVersionID as expected.
2. When a random (yet valid) AWS VersionID is used the s3 api returns a NoSuchVersion exception. (This VersionID was grabbed from a random versioned object on s3)

AWS behavior:

>>> import boto3
>>> s_client = boto3.client('s3')
>>> resp = s_client.get_object(Bucket='amar-test-delete', Key='space_kirby.gif', VersionId='invalid')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/amar/dev/hca/azul/.venv/lib/python3.8/site-packages/botocore/client.py", line 357, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/Users/amar/dev/hca/azul/.venv/lib/python3.8/site-packages/botocore/client.py", line 676, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (InvalidArgument) when calling the GetObject operation: Invalid version id specified

>>> resp = s_client.get_object(Bucket='amar-test-delete', Key='space_kirby.gif', VersionId='VWVT9JkWTreQ95JbRmQt6T3LWrljLpRZ')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/amar/dev/hca/azul/.venv/lib/python3.8/site-packages/botocore/client.py", line 357, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/Users/amar/dev/hca/azul/.venv/lib/python3.8/site-packages/botocore/client.py", line 676, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (NoSuchVersion) when calling the GetObject operation: The specified version does not exist.
>>> exit()

Expected: A random UUID4 string passed in as a VersionID should throw a NoSuchVersion exception as seen above.

Replication of bug on moto https://gist.github.com/amarjandu/334ffd668dd849991ba9949a32dcc3b8 Related to https://github.com/spulec/moto/issues/2710

[amarjandu]

Raised a PR for this https://github.com/spulec/moto/pull/3886 😄

[tsugumi-sys]

@bblommers I think this issue was already fixed via the following PR. https://github.com/getmoto/moto/pull/3887

[amarjandu]

@bblommers

I think this issue was already fixed via the following PR.

https://github.com/getmoto/moto/pull/3887

Just a heads up before someone closes this issue, there was a discussion thread https://github.com/getmoto/moto/pull/3886#pullrequestreview-648057742 here. We might want to confirm if this conversation is still relevant.

[tsugumi-sys]

@amarjandu Thanks for telling me :)

The ideal situation/fix would be for Moto to use exactly the same format as AWS. Without knowing exactly what that is, however, I prefer Moto to be feature incomplete, rather than inconsistent.

I agree with this comment by @bblommers. I checked the document and there is no update for version formatting of s3 object.

How about adding a comment about this incompleteness to the code?

[bblommers]

From a comment I made in one of the earlier threads, this is how S3 behaves in regards to error messages:

If the version ID is not known: Invalid version. If the version ID is known, but does not belong to this key: Unknown key.

If I remember correctly, the Unknown key exception is thrown when:

• the version ID belongs to a different key
• the version ID has already been deleted
• presumably also if the version ID exists to a key in a different account

Moto should behave the same way IMO, so we differentiate in which error we show, depending on whether the VersionID exists anywhere. Once that is implemented, I think we can close this.