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

Omit base path mapping from the path in a proxied request #4

Closed
wants to merge 9 commits into from
Closed

Omit base path mapping from the path in a proxied request #4

wants to merge 9 commits into from

Conversation

piotrkubisa
Copy link

@piotrkubisa piotrkubisa commented Jan 23, 2018

First, I'd like to say thank you @tj for your work!

I have been using apex/gateway for last few days while I was taking care of moving serverfull to serverless API. I have noticed two things, for which I've addressed fixes in this PR submission:

  1. Removing addr string which was passed as a parameter to ListenAndServe but I haven't noticed any usage of this variable.
  2. Making possible to work with both: custom Domain Name with a Base Path Mapping and default, auto-generated execute-api API endpoints.

Explanation of problem (2): Payload which you can receive via execute-api and this on your custom domain name with Base Path Mapping set is not consistent. Let /v2 will be our BASE_PATH, and also let / (ListItems) and /{id} (GetItem) routes be registered in API Gateway and http.Handler. If you will call execute-api with GET https://{apiId}.execute-api.{aws}.com/{stage}/v2/ you will see response for ListItems, but if you use api.example.com as domain name and if you call it by GET https://api.example.com/v2/ you will be calling GetItem where v2 will be mapped parameter for /{id}.

TL;DR:

  • GET https://{apiId}.execute-api.{aws}.com/{stage}/v2/ - call correctly ListItems handler
  • GET https://api.example.com/v2/ - call incorrectly GetItem handler

Similar issue was reported in:

And also implemented in official APIG-Lambda proxy - awslabs/aws-lambda-go-api-proxy: https://github.com/awslabs/aws-lambda-go-api-proxy/blob/5aaec6761420bbbd4b1f4cf9e2d6eff552413986/core/request.go#L74-L96

Sample usage (example from actual Readme.md will be valid too):

package main

import (
	"fmt"
	"log"
	"net/http"
	"os"

	"github.com/apex/gateway"
	"github.com/go-chi/chi"
)

func main() {
	// Feel free to replace chi.Router with any http.Handler
	r := chi.NewRouter()
	r.Get("/", hello)

	// i.e. BASE_PATH was provided as env variable in SAM CFN definition
	// base path could be a `v1`, `users` or just an empty string
	g := &gateway.Gateway{
		BasePath: os.Getenv("BASE_PATH"),
		Handler:  r,
	}
	log.Fatal(g.ListenAndServe())
}

func hello(w http.ResponseWriter, r *http.Request) {
	fmt.Fprintln(w, "Hello World from Go")
}

To sum up:

  • Signature of gateway.ListenAndServe does not change.
  • If somebody is interested in BasePathMapping then he just need to define it via struct field, which is very straightforward in Go.
  • Defaults will be okay - non breaking anything.
  • addr string is marked as ignored via blank identifier - again, non breaking signature Unfortunately, in the example presented in Readme.md tells to use port-binding mapping, which may lead to misinformation, what exactly this variable do (err.. nothing).
  • Allows creating simple unit tests (if somebody is not interested in SAM local), thanks to separated anonymous function as a Gateway.Serve method.

Note: Registering route twice for different resources is workaround, which works on both type of endpoints.

@piotrkubisa
Copy link
Author

piotrkubisa commented Feb 28, 2018

I am closing this PR, because I think it is better to configure the AWS Lambda function to work only with one type of the endpoint (custom domain or *.execute-api) and define a sub-routing with base path or not. Generally speaking: Less conditions = less problems.

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

Successfully merging this pull request may close these issues.

1 participant