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

add PRIVATE endpointType #5080

Merged
merged 1 commit into from Jul 3, 2018

Conversation

@ijin
Copy link
Contributor

commented Jun 27, 2018

What did you implement:

Added PRIVATE endpointType for API Gateway

Closes #5063 #5052

How did you implement it:

Added PRIVATE to validEndpointTypes const variable with more specific tests.

How can we verify it:

service: private-endpoint-test
provider:
  name: aws
  runtime: nodejs6.10
  endpointType: private

functions:
  helloWorld:
    handler: handler.hello
    events:
      - http:
          path: hello
          method: get
$ aws --region us-east-1 apigateway get-rest-apis 

{
    "items": [
        {
            "id": "gxq77iec19",
            "name": "dev-private-gw-test",
            "createdDate": 1530087544,
            "apiKeySource": "HEADER",
            "endpointConfiguration": {
                "types": [
                    "PRIVATE"
                ]
            }
        }
    ]
}

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

@ijin ijin force-pushed the ijin:private-endpoint branch from 9bb8275 to 88459f9 Jun 27, 2018
@horike37 horike37 added this to the 1.28 milestone Jun 27, 2018
@ijin ijin force-pushed the ijin:private-endpoint branch from 88459f9 to 38fe02e Jun 27, 2018
Copy link
Member

left a comment

@ijin
Works great!
Thank you for your contribution 💯 🥇
LGTM 😄

@horike37 horike37 merged commit 2416c68 into serverless:master Jul 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.02%) to 89.251%
Details
@mcshiz

This comment has been minimized.

Copy link

commented Aug 10, 2018

After I deployed a private gateway, I still had to log into the console and set a resource policy for the endpoint. When I looked at the cloudformation template the endpoint was set to private
"EndpointConfiguration": { "Types": [ "PRIVATE" ] }
but it appears that we have to also set the policy attribute which is a JSON Object of a Resource Policy.
For example this will allow anything within the VPC

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": "*",
            "Action": "execute-api:Invoke",
            "Resource": "arn:aws:execute-api:##########",
            "Condition": {
                "StringEquals": {
                    "aws:sourceVpce": "##############"
                }
            }
        }
    ]
}

EDIT: On top of that, I also had to go in and create a VPC endpoint pointing to com.amazonaws.us-west-2.execute-api.

@BrandonEvansAsurion

This comment has been minimized.

Copy link

commented Aug 15, 2018

@mcshiz You may already be doing this, but you can set the resource policy within your serverless.yml like so:

provider:
    name: aws
    runtime: nodejs6.10
    endpointType: private
    resourcePolicy:
        - Effect: Allow
          Principal: '*'
          Action: execute-api:Invoke
          Resource: arn:aws:execute-api:##########
          Condition:
              StringEquals:
                 'aws:sourceVpce': ##############
@mcshiz

This comment has been minimized.

Copy link

commented Aug 15, 2018

Yea, thanks @BrandonEvansAsurion. I did end up writing that into my serverless.yml.
I have decided that I will also create the com.amazonaws.us-west-2.execute-api vpc endpoint in the resources section of my file.
I think it would be helpful to add some of this information to the documentation, I'll work on a PR

@afeld afeld referenced this pull request Oct 3, 2018
@jamesleech jamesleech referenced this pull request Oct 8, 2018
@ptrhck

This comment has been minimized.

Copy link

commented Dec 6, 2018

I get a cloudformation circular dependency error if I try the following:

Resource: { "Fn::Join" : ["", [ "arn:aws:execute-api:${self:provider.region}:", { "Ref": "AWS::AccountId" }, ":", { "Ref": "ApiGatewayRestApi" }, "/*" ] ] }

I guess it does not work as it is the event that is being created? Can I solve this somehow such that I do not have to update in manually if redeploying?

What is the consequence if I set it to the following?

arn:aws:execute-api:${self:provider.region}:*

@dschep

This comment has been minimized.

Copy link
Member

commented Dec 6, 2018

@ptrhck, since the name of the ApiGatewayRestApi instance is generated from your service name & stage, you can infer it without using a Ref. So instead of:

Resource:
  "Fn::Join":
    - ""
    -
      - "arn:aws:execute-api:${self:provider.region}:"
      - { "Ref": "AWS::AccountId" }
      - ":"
      - { "Ref": "ApiGatewayRestApi" }
      - "/*"

Use this:

Resource:
  "Fn::Join":
    - ""
    -
      - "arn:aws:execute-api:${self:provider.region}:"
      - { "Ref": "AWS::AccountId" }
      - ":"
      - ${opt:stage, self:provider.stage}-${self:service}
      - "/*"
@luisamador

This comment has been minimized.

Copy link

commented Jan 7, 2019

I'm having exactly the same issue as @ptrhck.

@dschep I'm afraid your solution ends up throwing the same error message, at the end of the day you are referencing the same ApiGatewayRestApi object (circular dependency)

"Error ----------------------------------------- The CloudFormation template is invalid: Circular dependency between resources: blah..."

Any ideas?

@dschep

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

Oh. That's unfortunate @luisamador 😞

@luisamador

This comment has been minimized.

Copy link

commented Jan 7, 2019

I think I will deploy the API Gateway resource policy manually in the meantime.

@brunocascio

This comment has been minimized.

Copy link

commented Jan 26, 2019

@mcshiz

I have decided that I will also create the com.amazonaws.us-west-2.execute-api vpc endpoint in the resources section of my file.

Could you show me that code? :)

@ptrhck

This comment has been minimized.

Copy link

commented Mar 4, 2019

@ptrhck, since the name of the ApiGatewayRestApi instance is generated from your service name & stage, you can infer it without using a Ref. So instead of:

Resource:
  "Fn::Join":
    - ""
    -
      - "arn:aws:execute-api:${self:provider.region}:"
      - { "Ref": "AWS::AccountId" }
      - ":"
      - { "Ref": "ApiGatewayRestApi" }
      - "/*"

Use this:

Resource:
  "Fn::Join":
    - ""
    -
      - "arn:aws:execute-api:${self:provider.region}:"
      - { "Ref": "AWS::AccountId" }
      - ":"
      - ${opt:stage, self:provider.stage}-${self:service}
      - "/*"

This seems not to work anymore, right? Looks like you need arn:aws:execute-api:REGION:ACCOUNTID:*/*

@philipbarile

This comment has been minimized.

Copy link

commented Apr 17, 2019

I also have the same circular dependency issue with attempting to set the resource policy in serverless.yml. There is most likely a better way/I'm possibly doing something wrong, but I worked around it by wrapping my serverless deploy command in a shell script that then updates the resource policy on the API that was deployed, and finally issues a "create-deployment" command through the CLI (this last part is important or it won't work. I found this out the hard way after several hours ha).

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