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

Support AWS GovCloud and China region deployments #4665

Merged
merged 16 commits into from Apr 3, 2018

Conversation

@mikepugh
Copy link
Contributor

commented Jan 18, 2018

What did you implement:

Closes #3748

Add support for US-GovCloud and China region deployments.

How did you implement it:

Using the region information from the AWS provider, build an S3 endpoint that will work for the region.

Also removed the AccelerateConfiguration entry from the CF template since this option is not yet supported in GovCloud (not sure if it is supported in China, but excluded it from there as well).

How can we verify it:

I've tested it locally against my own GovCloud based account, but I cannot test against China regions.

Sample snippet from my serverless.yml file:

provider:
  name: aws
  runtime: python3.6
  profile: ${self:custom.profiles.${self:provider.stage}}
  region: ${self:custom.regions.${self:provider.stage}}
  stage: ${opt:stage, self:custom.defaultStage}

custom:
  regions:
    dev: us-east-1
    prod-east: us-east-1
    prod-gov: us-gov-west-1

Screenshot of successful deployment to regular US-East region:
image

Screenshot of successful deployment to GovCloud:
image

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

@mikepugh mikepugh changed the title Support GovCloud and China region deployments Support AWS GovCloud and China region deployments Jan 18, 2018
@horike37

This comment has been minimized.

Copy link
Member

commented Jan 19, 2018

@mikepugh

Awesome! Thank you for working on this 👍
Those regions support are eagerly wanted by many users 😄

China region might not support for some resources of CloudFormation according to #4463 and #4615, so it would be good to do some test to confirm if functions and events supported by the framework are working fine.
Can anyone do the test on China region?
cc: @serverless/vip

Seems that it is working fine on US-GovCloud. Thank you for testing 👍
Additionally, are AWS events working fine as well? Could you test that like http, scheduled, streams event etc?

@mikepugh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 19, 2018

I found some issues with hardcoded partition ids and url suffixes as I started testing out things like API Gateway, etc. Adding some more commits and will test it a bit more.

There will be limitations on certain events, for example API Gateway cannot be used in the default EDGE mode and has to be set to REGIONAL.

IoT, Alexa also are not supported in GovCloud.

There are still some failing unit tests that I'm working on fixing as well.

mikepugh added 2 commits Jan 19, 2018
@mikepugh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 20, 2018

@horike37

The following serverless.yml file events successfully deployed to both GovCloud and US-East using this PR:

image

The limitations for GovCloud are:

  • API Gateway Endpoint Type has to be set to REGIONAL instead of the default EDGE
  • Alexa Skill & Smart Home events are not supported
  • Cognito User Pool is not supported
  • Cognito authorizers are not supported
  • IoT is not supported

Please let me know if you have any other serverless.yml event configs that you'd like me to test out in GovCloud.

Update: I did test with deploying a lambda function within a VPC/Subnets and it worked fine as well in GovCloud.

@horike37

This comment has been minimized.

Copy link
Member

commented Jan 22, 2018

Awesome @mikepugh 👍 💯
Thank you for the update and investigating deeply!!

GovCloud looks good since you have done the deep testing!
Imo, it would be good to separate PRs from GovCloud and China regions since we can merge GovCloud stuff as soon as finishing our code review. However, China regions stuff may take a long time to be merged since we need to wait for someone can test in the region.

What do you think?

@mikepugh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2018

@horike37

So the only thing that is China region specific would be the S3 endpoint returned in getS3EndpointForRegion.js. All of the other code changes just remove hardcoded string values and replace them with the CloudFormation pseudo parameters that pick up the right AWS Partition or the URL Suffix. This means there really isn't much to separate. Since the tool isn't working for China right now, I don't see much harm in releasing it untested for that region - worst case it continues to not work.

@horike37

This comment has been minimized.

Copy link
Member

commented Jan 23, 2018

@mikepugh
Ok, make sense 👍 We should leave it as it is without separating them.

We will review the code and do a test if this PR doesn't introduce any side effect as soon as possible. Thanks your valuable efforts 💯 👍

@doshitan

This comment has been minimized.

Copy link

commented Jan 23, 2018

Nice, I was just coming to make a pull request for our work getting serverless running against GovCloud (https://github.com/opengovfoundation/serverless/tree/govcloud-pr). It's broadly similar to what's here. I'll take a closer look and test this against our config too.

I noticed some build files that snuck in at lib/plugins/create/templates/aws-csharp/obj/Debug/netcoreapp1.0/aws-csharp.*.

@bnusunny

This comment has been minimized.

Copy link

commented Jan 24, 2018

I can help to test this in China region. Just need instruction on how to build this...

@mikepugh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2018

@doshitan Those build files snuck in my original commit but I thought I cleared them out.

Copy link

left a comment

Overall this looks good to me. I'm not a maintainer and didn't test all of serverless extensively, but after fixing the ManagedPolicyArns error, this PR can successfully deploy our setup to GovCloud, which is good enough for me.

There's probably some documentation stuff that could be done, like updating the framework templates to be partition independent by default[1], but that could be in a followup PR or something.

Good work! I'm looking forward to having this merged.

[1]: This was proposed in #4219 and a cleaner approach suggested in #4219 (comment). I did a quick pass of the suggestion in opengovfoundation@6a68bff which built off of #4219.

.ManagedPolicyArns = [
'arn:aws:iam::aws:policy/service-role/AWSLambdaVPCAccessExecutionRole',
];
.ManagedPolicyArns = {

This comment has been minimized.

Copy link
@doshitan

doshitan Jan 24, 2018

Getting An error occurred: IamRoleLambdaExecution - Value of property ManagedPolicyArns must be of type List of String. while deploying. So this just needs wrapped in square brackets.

This comment has been minimized.

Copy link
@mikepugh

mikepugh Jan 25, 2018

Author Contributor

Thanks for catching this, I'll fix it and commit later this evening.

@@ -0,0 +1,23 @@
//------------------------------------------------------------------------------

This comment has been minimized.

Copy link
@doshitan
@@ -0,0 +1 @@
a49277692051e4f935ec0560b75544b62478532d

This comment has been minimized.

Copy link
@doshitan
@@ -0,0 +1 @@
c09d0ed92405f2a8a8bc0718f6fdffa48f6bee17

This comment has been minimized.

Copy link
@doshitan
.then((result) => {
const arn = result.Arn;
const accountId = result.Account;
const partition = arn.substring(arn.indexOf(':') + 1, arn.indexOf(':', 4));

This comment has been minimized.

Copy link
@doshitan

doshitan Jan 24, 2018

Something like arn.split(':')[1] reads easier to me.

This comment has been minimized.

Copy link
@mikepugh

mikepugh Jan 25, 2018

Author Contributor

Agreed - will update tonight, thanks.

@mikepugh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 26, 2018

@doshitan thanks for the review. I've made the suggested updates.

@zzq889

This comment has been minimized.

Copy link

commented Jan 27, 2018

I'm testing AWS China region cn-north-1 now and I've found few issues here:

  • Endpoint type EDGE is not supported (REGIONAL only)
    could be specific in yml file:
provider:
  endpointType: REGIONAL
  • AWS::Lambda::Permission rolls an error:
The provided principal was invalid. Please check the principal and try again.

I'm still digging.

@zzq889

This comment has been minimized.

Copy link

commented Jan 29, 2018

The principal url defined in CloudFormation of apigateway in China doesn't follow the URLsuffix rule like others.
It's apigateway.amazonaws.com but not apigateway.amazonaws.com.cn and I've reported to AWS engineers in China. Let's wait for their responses.

@mikepugh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 29, 2018

@zzq889 oh wow, I didn't see that coming. Thanks for digging into it and contacting support.

@horike37 what do you think? Should we wait to hear back on the API Gateway issue for China or handle that later in a new PR as we learn more? Personally, I'd vote for moving forward with a review/merge as is since it looks like the PR works in GovCloud and mostly works in China except for the API Gateway.

@horike37

This comment has been minimized.

Copy link
Member

commented Jan 29, 2018

Hi @bnusunny, thank you for jumping in!

Just need instruction on how to build this...

You can install the framework code on this PR into your local by running npm install -g americansystems/serverless#sls-govcloud

Thank you for testing @zzq889 👍
Do you see working fine with the other aws events?
We would love to know if lambda function can be deployed properly and which events working fine in China.

@mikepugh
I agree with you. We should release it without waiting for resolving API Gateway issue.

Copy link
Member

left a comment

Hey, @mikepugh.
I just tested locally it other than GovCloud and China regions, and I have confirmed any side effect is not introduced with this PR(I can't test those Regions). It is working fine so far! Awesome 💯

And just commented on for the code improvements from my perspective.
Could you check it?

@@ -23,20 +24,25 @@ module.exports = {

const authorizerLogicalId = this.provider.naming.getAuthorizerLogicalId(authorizer.name);

if (typeof authorizer.arn === 'string' && authorizer.arn.match(/^arn:aws:cognito-idp/)) {
if (typeof authorizer.arn === 'string'
&& authorizer.arn.match(awsArnRegExs.cognitoIdpArnExpr)) {

This comment has been minimized.

Copy link
@horike37

horike37 Feb 1, 2018

Member

Would be good to leverage awsArnRegExs.cognitoIdpArnExpr.test(authorizer.arn) instead of match for consistancy of other places.
Personally, I prefer test method within if-condition since it returns a boolean value.

@@ -18,7 +19,8 @@ module.exports = {

let authorizationType;
const authorizerArn = http.authorizer.arn;
if (typeof authorizerArn === 'string' && authorizerArn.match(/^arn:aws:cognito-idp/)) {
if (typeof authorizerArn === 'string'
&& authorizerArn.match(awsArnRegExs.cognitoIdpArnExpr)) {

This comment has been minimized.

Copy link
@horike37

horike37 Feb 1, 2018

Member

Same here

const authorizer = singlePermissionMapping.event.http.authorizer;
const authorizerPermissionLogicalId = this.provider.naming
.getLambdaApiGatewayPermissionLogicalId(authorizer.name);

if (typeof authorizer.arn === 'string' && authorizer.arn.match(/^arn:aws:cognito-idp/)) {
if (typeof authorizer.arn === 'string'
&& authorizer.arn.match(awsArnRegExs.cognitoIdpArnExpr)) {

This comment has been minimized.

Copy link
@horike37

horike37 Feb 1, 2018

Member

Same here.

@@ -254,7 +255,9 @@ module.exports = {

const integration = this.getIntegration(http);
if (integration === 'AWS_PROXY'
&& typeof arn === 'string' && arn.match(/^arn:aws:cognito-idp/) && authorizer.claims) {
&& typeof arn === 'string'
&& arn.match(awsArnRegExs.cognitoIdpArnExpr)

This comment has been minimized.

Copy link
@horike37

horike37 Feb 1, 2018

Member

Same here.

@@ -0,0 +1,11 @@
'use strict';

module.exports = function getS3EndpointForRegion(region) {

This comment has been minimized.

Copy link
@horike37

horike37 Feb 1, 2018

Member

Would be good to move this module to under lib/plugins/aws/utils/ since this is referenced other than deploy.

@zzq889

This comment has been minimized.

Copy link

commented Feb 2, 2018

@mikepugh @horike37 Sorry for the late reply. AWS engineers still working on the URLSuffix. I'm kind busy these days. I'll do more test. If us-gov just works fine I'd suggest that you can merge it and prepare another PR for the China region.

@horike37 horike37 added this to the 1.27 milestone Feb 9, 2018
@cncolder

This comment has been minimized.

Copy link

commented Feb 15, 2018

Some tips about AWS China:

  1. Lambda supported in Beijing cn-north-1 region only. Ningxia cn-northwest-1 region is not supported yet.

  2. If you have a function named hello with http event. You need patch Cloud Formation API Gateway Principal like this:

functions:
  hello:
    handler: handler.hello
    events:
      - http: GET hello

resources:
  Resources:
    HelloLambdaPermissionApiGateway:
      Properties:
        Principal: apigateway.amazonaws.com
  1. You cannot open your endpoint without ICP Recordal. It always return 403 {"Message": null}. Except your function authorize by IAM:
functions:
  hello:
    handler: handler.hello
    events:
      - http:
          method: get
          path: hello
          authorizer: aws_iam

Consider try postman for test your endpoint with AWS4 Authorization header.

  1. Don't set environment in your provider or functions. It's not supported in cn-north-1 region.
provider:
  name: aws
  region: cn-north-1
  endpointType: REGIONAL
  runtime: nodejs6.10
  # Lambda environment is not supported yet!
  # environment:
    # DYNAMODB_TABLE: ${self:service}-${opt:stage, self:provider.stage}
  1. Don't waste time on Cognito User Pool (trigger or auth). Only Federate Identities available now.
functions:
  preSignUp:
    handler: preSignUp.handler
    events:
      - http:
          path: posts/create
          method: post
          # This ARN is not exists. 
          # authorizer: arn:aws-cn:cognito-idp:cn-north-1:xxx:userpool/cn-north-1_ZZZ
      # This event trigger not work!
      # - cognitoUserPool:
      #     pool: MyUserPool
      #     trigger: PreSignUp
  1. The builtin aws-sdk version is 2.190.0. Doc expired.

  2. to be continued...

@HoangLVQ1204

This comment has been minimized.

Copy link

commented Feb 16, 2018

@cncolder
Thanks you for sharing your tips. Could you share with me how you test endpoint with authorize by IAM. I'm also facing the problem 403 {"Message": null}.

@cncolder

This comment has been minimized.

Copy link

commented Feb 16, 2018

@HoangLVQ1204 Here is a demo. Sign by aws4. More info Signature Version 4

The key of aws4 sign is like this:

{ Host: 'nr1uaybkr2.execute-api.cn-north-1.amazonaws.com.cn',
  'X-Amz-Date': '20180216T153044Z',
  Authorization: 'AWS4-HMAC-SHA256 Credential=YOUR_AWS_ACCESS_KEY_ID/20180216/cn-north-1/execute-api/aws4_request, SignedHeaders=host;x-amz-date, Signature=7a14d876a018c132478445c4182b870f5eb8047588b6a5621081bcb450e866c9' }

If you want "test" your endpoint. The easy way is download postman and follow Use Postman to Call an API

const { parse } = require('url')
const aws4 = require('aws4')
const axios = require('axios')

const url = 'https://nr1uaybkr2.execute-api.cn-north-1.amazonaws.com.cn/dev/hello'

function signHeaders(url) {
    const urlObj = parse(url)
    return aws4.sign({
        region: 'cn-north-1',
        service: 'execute-api',
        host: urlObj.host,
        path: urlObj.path,
    }).headers
}

function get(url) {
    return axios.get(url, {
        headers: signHeaders(url),
    })
}

get(url)
    .then(res => console.log(res.data))
    .catch(console.log)

Run this script by node index.js then you will got:

{ message: 'AWS Serverless (nodejs@6.10) v0.1.0! Your function executed successfully!', ...}
@HoangLVQ1204

This comment has been minimized.

Copy link

commented Feb 27, 2018

@cncolder thanks you for explaining clearly. I also have a question related to this problem. By setting "authorizer: aws_iam" and using iam credential (aws access id and key), I could get the response correctly. Now I'm building a react app as client site. In order to invoke API from this app, I'm going to save the IAM credential into source code. But I wonder this way is whether good or not. Of course I just assign the right permission into this credential (just for invoking API). Thanks a lot.

@cncolder

This comment has been minimized.

Copy link

commented Feb 27, 2018

@HoangLVQ1204 Use by your risk!

I wrote No.3. Bcs someone (#4703) confused what's that {"Message": null} same as me.

The right way is get ICP for your aws account. Even you won't use that domain in your project.

If you cannot or do not want ICP. Put iam secret key in react client. Similar make your api "public". You need implement another user auth layer over iam.

@doshitan

This comment has been minimized.

Copy link

commented Mar 12, 2018

@HyperBrain @horike37 so what's left to do before this can be merged? It looks like all requested changes have been made.

@horike37

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

Hi, @HyperBrain
Could you please a final review so that we can merge this?

@HyperBrain

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

@horike37 @mikepugh Looking good now 😄

@horike37 horike37 merged commit 6cbd0a9 into serverless:master Apr 3, 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.04%) to 88.871%
Details
@jseiser

This comment has been minimized.

Copy link

commented May 2, 2018

Can anyone please explain how to use this?

npm i americansystems/serverless#sls-govcloud
➜ amibackup git:(master) ✗ sls deploy --stage prod --region us-gov-west-1 --aws-profile govcloud-sl
Serverless: Packaging service...
Serverless: Excluding development dependencies...

Serverless Error ---------------------------------------

Could not locate deployment bucket. Error: The specified bucket does not exist

Get Support --------------------------------------------
Docs: docs.serverless.com
Bugs: github.com/serverless/serverless/issues
Forums: forum.serverless.com
Chat: gitter.im/serverless/serverless

Your Environment Information -----------------------------
OS: linux
Node Version: 8.11.1
Serverless Version: 1.26.0

The bucket exists, and this lambda works in my other non gov-cloud accounts.

@erikerikson

This comment has been minimized.

Copy link
Member

commented May 2, 2018

@jseiser perhaps npm i serverless/serverless?

@jseiser

This comment has been minimized.

Copy link

commented May 2, 2018

Same results on 1.26.1 -- The Serverless service works on the other, non gov-cloud account.

@erikerikson

This comment has been minimized.

Copy link
Member

commented May 2, 2018

To be clear, I was recommending installing master, it has been awhile since master has been published.

@HyperBrain

This comment has been minimized.

Copy link
Member

commented May 2, 2018

1.27.0 (master) was published just a few hours ago 😄

@erikerikson

This comment has been minimized.

Copy link
Member

commented May 2, 2018

Awesome! Thanks Frank!

@metrue

This comment has been minimized.

Copy link

commented May 4, 2018

@cncolder amazonaws.com.cn has ICP as I checked, do you know why I still got { Message: null } when I accessed the endpoints generated by apigateway (https://.execute-api.cn-north-1.amazonaws.com.cn/staging/)

@lonsdale8734

This comment has been minimized.

Copy link

commented May 4, 2018

@cncolder I tried the code in your comment, but it's no work.

const { parse } = require('url')
const aws4 = require('aws4')
const axios = require('axios')

function signHeaders(url) {
    const urlObj = parse(url)
    return aws4.sign({
        region: 'cn-north-1',
        service: 'execute-api',
        host: urlObj.host,
        path: urlObj.path,
    }, {
        accessKeyId: 'key_id',
        secretAccessKey: 'secret',
    }).headers
}

function get(url) {
    return axios.get(url, {
        headers: signHeaders(url),
    })
}

get('https://7lxtonylp1.execute-api.cn-north-1.amazonaws.com.cn/prod/sms')
	.then(res => console.log(res.data)).catch(console.log)
@cncolder

This comment has been minimized.

Copy link

commented May 5, 2018

@metrue What's mean of amazonaws.com.cn has ICP? You need a ICP Recordal for your domain name. Open 光环新网ICP for more details. If you're working in china. I think you understand what's that: Send your company license copy with seal. Request curtain and put on wall then stand there take a photo. Waiting gov phone call...

@lonsdale8734 Did you config your lambda authorizer with aws_iam? This code only for you want call your apigateway without ICP.

functions:
  hello:
    handler: handler.hello
    events:
      - http:
          method: get
          path: hello
          authorizer: aws_iam

2018-05-05 12 51 53

@metrue

This comment has been minimized.

Copy link

commented May 5, 2018

@cncolder Thanks for your clarification, I did not bind the apigateway endpoint to my custom domain, so I don't really understand what you said that 'You need a ICP Recordal for your domain name'

@horike37 horike37 referenced this pull request May 5, 2018
6 of 7 tasks complete
@sovanc

This comment has been minimized.

Copy link

commented Nov 30, 2018

Hello,
I'm getting "ServerlessError: The security token included in the request is invalid." when deploying to GovCloud. It's a test Hello function. Is there a specific serverless version that I have to be on? Thanks in advance for your help.

service: hello

provider:
name: aws
runtime: nodejs8.10
stage: prod
region: us-gov-west-1

It works fine in the commercial region.

$ sls deploy --profile gc -v
Serverless: Packaging service...
Serverless: Excluding development dependencies...

Serverless Error ---------------------------------------

ServerlessError: The security token included in the request is invalid.

Get Support --------------------------------------------
Docs: docs.serverless.com
Bugs: github.com/serverless/serverless/issues
Issues: forum.serverless.com

Your Environment Information -----------------------------
OS: darwin
Node Version: 11.1.0
Serverless Version: 1.34.0

@mikepugh mikepugh deleted the americansystems:sls-govcloud branch Dec 10, 2018
@StrandingHeart

This comment has been minimized.

Copy link

commented Feb 15, 2019

@cncolder Hello, now I have encountered the problem of the null message of 403. My aws account was given by someone else, and only the gateway lambda authority is available. I have access to 403 according to the postman configuration, and I would like to ask if this is the person who needs to assign me the right to file an ICP? Another question is whether I can use his IAM account to access postman.

@cncolder

This comment has been minimized.

Copy link

commented Feb 16, 2019

@StrandingHeart Not sure and yes.

I wrote my previous answer one year before. I'm not sure if aws has block that way.

In China. All servers or services that expose 80/443 port. You must get ICP first. Otherwise service provider (AWS) will BLOCK you. Thats the means of error code 403: Forbidden

@Vadorequest

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

I'm trying to understand what do I have to do to make a website external to China available to Chinese users, I opened this SO question. There is both regulations and technical issues at hand:
https://stackoverflow.com/questions/56991822/aws-lambda-serverless-framework-make-website-accessible-from-china

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.