Skip to content

Adding Dynamic Path Override #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 18, 2019

Conversation

Hans-Seek
Copy link
Contributor

We recently faced a problem with the serverless proxy integration, namely when the s3 has a couple of folders and the folders should be mapped via path parameters. We found out that the solution to this would be to slightly adjust this plugin (by making the path override dynamic with fallback to the default functionality)

The main change was made in lib/package/s3/compileMethodsToS3.js:

    let pather = '{bucket}/{object}'

    if (_.has(http, 'pathoverride')) {
      pather = '{bucket}/' + http.pathoverride
    }

    const integration = {
      IntegrationHttpMethod: httpMethod,
      Type: 'AWS',
      Credentials: roleArn,
      Uri: {
        'Fn::Sub': ['arn:aws:apigateway:${AWS::Region}:s3:path/' + pather, {}]
      },
      PassthroughBehavior: 'WHEN_NO_MATCH',
      RequestParameters: _.merge(requestParams, http.requestParameters)
    }

This will allow paths like {folder}/{subfolder}/{item} to be mapped to an s3 endpoint like for example {folder}/{subfolder}/{item}.xml

For example if the API Gateway gets called with function/english/data it will look for the file in s3 at the following point: {bucket}/function/english/data.xml

It can easily be adjusted via serverless.yml by adding the pathoverride attribute (string) to a s3 object. I also added these details into the Readme.md into the S3 section. Please let me know about your thoughts and also let me know if anything needs to be changed/verified before it could be considered to be merged into the plugin.

@taylorreece
Copy link
Contributor

Lol, funny we both submit a PR for the same thing the same day :-) #50

@horike37
Copy link
Collaborator

@Hans-Seek
Thank you for this PR! Will take a look at it soon 👍

@taylorreece
Copy link
Contributor

@horike37 I think I can do with #49 what I set out to do with #50, so we can probably close #50 in favor of #49

@horike37
Copy link
Collaborator

@taylorreece
OK! thank you for confirming 👍

@Hans-Seek
Copy link
Contributor Author

Might be worth while noting in the readme that you could use a greedy {myPath+} if your files are more than one folder deep. Then you could

path: /s3/{myPath+}
pathoverride: 'something/{myPath}.xml

Or whatever and translate /s3/a/b/c to something/a/b/c.xml

Will add that into the readme, good point actually.

Camel case?

Will change it to pathOverride

@taylorreece
Copy link
Contributor

I imported these changes last night and verified that they:

  1. Don't break existing deployments
  2. Allow me to do what I wanted in Allow s3 service users to prefix s3 objects with a path #50

Copy link
Collaborator

@horike37 horike37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hans-Seek @taylorreece
works as expected and the code is no problem from my view 👍
Thanks!

@horike37 horike37 merged commit 9212b17 into serverless-operations:master Nov 18, 2019
@horike37
Copy link
Collaborator

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants