POC of global tables version 2019

[giograno]

This PR introduces initial support for DynamoDB global tables version 2019.

We simulate replicas as follows:

• We store information about the replicas in a cross-region attribute; this step is done when the user updates a table. The region for this call is going to the "original region" where the table effectively lives. De-facto, we have created a global table with this step.
• We can now call operations (only PutItem and GetItem at the moment) on the created global table from other regions. Before forwarding the call to DDB local, we inspect our cross-region attribute to see if this is a global table and if in the current regions there is a valid replica. If so, we change the context of the call and the credentials in the header to perform a call against the original region (where the table effectively lives) and not to the replica (DDB local won't find it).

Such an approach is still rudimental and probably it does not cover a lot of corner cases, but we are able to replicate the example in the docs in an integration test.

[coveralls]

Coverage increased (+0.02%) to 91.406% when pulling eec14ec on dynamodb-global-tables into 122e414 on master.

[github-actions]

LocalStack integration with Pro

       3 files  ±0         3 suites  ±0   1h 18m 35s ⏱️ + 3m 16s
1 244 tests +5  1 203 ✔️ +5  41 💤 ±0  0 ❌ ±0 
1 669 runs  +5  1 597 ✔️ +5  72 💤 ±0  0 ❌ ±0 

Results for commit eec14ec. ± Comparison against base commit 122e414.

♻️ This comment has been updated with latest results.

[thrau]

eager to get this into v1.1, but i have some concerns about the context modification that i would like to get an extra opinion on first, ideally @alexrashed

[thrau]

modifying the request context this way can lead to unexpected behavior downstream in the handler chain. i'm not sure it will really be a problem. maybe it would be better to create a fresh request context and make a completely new request transparently to the client. but then the response would contain a region that's different to the original request, which may be equally problematic 🤷

i guess we can live with it for now. any maybe @alexrashed has some thoughts about it.

[alexrashed]

I don't think it causes problems directly, but I do agree that these properties of the context should be somewhat immutable.
Imho, these modifications should be handled by creating a new context which is only created for the forwarding.
In fact, the ForwardingFallbackDispatcher already creates a separate context for the request forwarding (in most cases):

localstack/localstack/aws/forwarder.py

Lines 70 to 79 in 4a58a97

	if service_request is not None:
	local_context = create_aws_request_context(
	service_name=context.service.service_name,
	action=context.operation.name,
	parameters=service_request,
	region=context.region,
	)
	local_context.request.headers.update(context.request.headers)
	context = local_context
	return forward_request(context, forward_url_getter)

So the cleanest way would be to refactor the forwarding such that it becomes a bit more flexible, but currently the context modification should not cause any issues.

[thrau]

hey @giograno, thanks for considering my comments! i think this is heading in a good direction, and i think in principle the idea of having a modifier is a good idea!

i remembered that i had built something similar in ext (you can find it in the batch provider), that looks something like this:

@contextmanager
def modified_headers(request: Request, headers: Union[Mapping, Headers]):
    """
    Context manager that extends the original headers of the HTTP request with the given headers in-place. When the
    context manager exits, the original headers are restored.

    TODO: move to localstack.http

    :param request: the request to modify
    :param headers: the modified headers
    :return: a context manager that returns the request context
    """
    original = request.headers
    modified = Headers()
    modified.update(original)
    modified.update(headers)
    request.headers = modified

    yield request

    request.headers = original

which i then call

        with modified_headers(context.request, headers):
            LOG.info("submitting job %s", request)
            return call_moto_with_request(context, request)

maybe we can slightly adapt and repurpose this idea to the particular case of dynamodb. i think a context manager that modifies the original context, makes the request, but then undoes the modifications (rather than copying the request) would be a great compromise.
also, i think it would be good to keep the code in dynamodb for now until situations emerge where we can re use the code.

i hope you find the review constructive, happy to help with the changes of you want!

[thrau]

copying the request context like this is not safe. the context contains the original http request object, which has all sorts of IO internals that may break in unexpected ways when copying. so i would avoid any pattern that relies on copying the context all together.

[thrau]

looks great now! thanks for considering all the comments :-)