localstack / serverless-localstack

⚡ Serverless plugin for running against LocalStack
511 stars 82 forks source link

fix LocationConstraint added for us-east-1 by the SDK #214

Closed bentsku closed 1 year ago

bentsku commented 1 year ago

This PR adds a patch to the aws-sdk, using its own utilities to overwrite an S3 method. It fixes https://github.com/localstack/localstack-demo/issues/52

In aws-sdk/lib/services/s3.js, there this createBucket method:

  createBucket: function createBucket(params, callback) {
    // When creating a bucket *outside* the classic region, the location
    // constraint must be set for the bucket and it must match the endpoint.
    // This chunk of code will set the location constraint param based
    // on the region (when possible), but it will not override a passed-in
    // location constraint.
    if (typeof params === 'function' || !params) {
      callback = callback || params;
      params = {};
    }
    var hostname = this.endpoint.hostname;
    // copy params so that appending keys does not unintentioinallly
    // mutate params object argument passed in by user
    var copiedParams = AWS.util.copy(params);
    if (hostname !== this.api.globalEndpoint && !params.CreateBucketConfiguration) {
      copiedParams.CreateBucketConfiguration = { LocationConstraint: this.config.region };
    }
    return this.makeRequest('createBucket', copiedParams, callback);
  },

However, the if (hostname !== this.api.globalEndpoint && !params.CreateBucketConfiguration) check to add the LocationConstraint evaluate the endpoint (to see if it's a region specific one) against the global endpoint for us-east-1.

This patch changes the logic to simply check the region set in the config, and if different than us-east-1 and a configuration is not set, sets it.

I can now properly deploy the demo with the new S3 v2 provider which enforces the specific fact that you cannot set the LocationConstraint to us-east-1.

To reproduce, you can deploy https://github.com/localstack/localstack-demo/ with and without the fix, with PROVIDER_OVERRIDE_S3=v2 when starting LocalStack.

I however have no idea how to add a test for this.