Skip to content
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

Added possibility to specify custom S3 key prefix instead of the stan… #5299

Merged
merged 1 commit into from Oct 14, 2018

Conversation

@weeniearms
Copy link
Contributor

commented Sep 16, 2018

What did you implement:

Closes #5293

Ability to specify custom prefix under which artifacts are deployed to S3, which might be useful in scenarios where the default serverless prefix cannot be used (e.g. when using fine grained access control based on prefix).

How did you implement it:

Added getDeploymentPrefix function to awsProvider.js that returns the custom prefix if defined (defaults to serverless if not defined) and replaced the default hardcoded prefix in various files with the call to getDeploymentPrefix.

How can we verify it:

Add the deploymentPrefix key to the provider config as shown in the updated docs, run the deploy command, and check if resources are deployed under the specified prefix in S3

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@ctindel

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2018

Awesome!

@weeniearms weeniearms force-pushed the weeniearms:master branch 3 times, most recently from 00fb7c6 to 40565ae Sep 17, 2018
@weeniearms weeniearms force-pushed the weeniearms:master branch from 40565ae to b5da4d0 Sep 17, 2018
@horike37 horike37 added this to the 1.33.0 milestone Oct 14, 2018
Copy link
Member

left a comment

@weeniearms
LGTM 👍 Thanks for implementing this 😄

@horike37 horike37 merged commit bf2ee2e into serverless:master Oct 14, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.009%) to 88.527%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.