sam-goodwin / punchcard

Type-safe AWS infrastructure.
Apache License 2.0
507 stars 20 forks source link

Provide a way to access either the context of Api Integration "addMethod" or a better way to modify the headers #99

Open pocesar opened 4 years ago

pocesar commented 4 years ago

I'm trying to set the headers from inside a:

const api = new p.ApiGateway.Api(stack, 'api', {
  defaultCorsPreflightOptions: {
    allowOrigins: ['api.subdomain.com'], // custom subdomain that is later set on Route 53
    allowCredentials: true,
    allowHeaders: ['Accept', 'Authorization', 'Content-Type'],
  },
})

const res = api.addResource('resource')

res.setGetMethod({
  /*...*/
  handle: async (r) => {
    try {
      return p.ApiGateway.response(p.ApiGateway.StatusCode.Ok, {
        result: await getResource(r.code)
      })
    } catch (e) {
      return p.ApiGateway.response(p.ApiGateway.StatusCode.InternalError, {
        result: null,
        error: e.message
      })
    }
  }
})

The options request works (pre-flight), but the actual GET response is missing the CORS header. one way would to provide those depending on the defaultCorsPreflightOptions. another way would to be able to pass a third parameters to ApiGateway.response as an object to be appended to the headers. even though my use case is CORS, might be useful to provide more headers as needed

I know you're underway with #71 but you might figure something midway through, otherwise I'd send you a PR

sam-goodwin commented 4 years ago

I plan on re-writing the API gateway stuff once I'm done with this decorator change for Shapes. Something like type-graphql might be better.

The current implementation uses velocity templates in a hacky/bad way to map a request into a JSON that represents the "Shape" of the request, so we'd have to update that to also set headers.

API gateway's proxy integration might be easier to work with than velocity templates? Is there a good reason not to use proxy?

pocesar commented 4 years ago

yeah, I was going to take a wild shot there, and realized that the string templates are created on-the-fly. CDK itself should provide a better way to create those velocity templates, in an idiomatic way like you're doing with the DSL for the shapes. when "opening up" yourself to the greediness of the {proxy+} path / ANY, the drawback would be to have to provide an internal routing/validating code mechanism, that always call code, regardless instead of stopping short on missing/wrong/unexpected parameters, I guess? but thinking "punchcard", that does build/runtime separation, it shouldn't be a bad idea to have all your code in one lambda for multiple paths, I can think of perks like less frequent cold starts

sam-goodwin commented 4 years ago

Having access to the CDK, I was operating under the tenet that Punchcard should rely on managed solutions (over runtime ones) wherever possible. "Lean on AWS as much as possible". Validation at the API GW level is desired over runtime validation in Lambda. We'll be able to support it - it just needs a little bit TLC. Looking forward to the re-write with classes and decorators!