-
Notifications
You must be signed in to change notification settings - Fork 2
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
Post registration api gateway #296
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks right, but you might want a more seasoned terraformer to cast their eye too
|
||
# 400 Bad Request | ||
|
||
resource "aws_api_gateway_method_response" "users_userid_registration_put_400" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have modules for configuring these error responses; see https://github.com/wellcomecollection/terraform-aws-api-gateway-responses
In particular, they'll set up the JSON error responses we use in the rest of our APIs, e.g.
{
"errorType": "http",
"httpStatus": "400",
"label": "Bad Request",
"description": "You didn't combobulate the widget wrangler",
"type":"Error"
}
For an internal-only API this is likely overkill and this is fine as-is, but I mention it for information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw - I probably wouldn't use that here, because it's not used for the rest of this APIGW setup (this is a biiiig file, which isn't great but it is easy to copy and paste - and delete). Better would be to refactor the whole stack to be more modular in a separate piece of work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, agreed – consistency within this stack is better than using this module.
resource_id = aws_api_gateway_resource.users_userid_registration.id | ||
http_method = aws_api_gateway_method.users_userid_registration_options.http_method | ||
|
||
integration_http_method = "POST" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this POST when the comment says OPTIONS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question – this was broadly copy/paste from the users/:user_id/password
block above, where it is also POST
under the same circumstances. Do both of these need updating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No - this needs to be POST, it isn't related to the request method at all. See https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/api_gateway_integration#integration_http_method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. Thanks for clarifying
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hoops: jumped
Part of wellcomecollection/wellcomecollection.org#7808
Add API gateway required things (I think).