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

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

Open
pocesar opened this issue Jan 14, 2020 · 3 comments

Comments

@pocesar
Copy link
Contributor

pocesar commented Jan 14, 2020

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
Copy link
Owner

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
Copy link
Contributor Author

pocesar commented Jan 16, 2020

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
Copy link
Owner

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!

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

No branches or pull requests

2 participants