-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(apigatewayv2): websocket api: grant manage connections #16872
feat(apigatewayv2): websocket api: grant manage connections #16872
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.
Thanks for the PR @tmokmss. See my comments below.
Please add a few sentences to the README describing this new API.
/** | ||
* The ARN of the WebSocket API. | ||
* | ||
* @attribute | ||
*/ | ||
readonly apiArn: string; |
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.
Remove this from here. The Arn is not attribute of IApi
.
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.
OK, thanks. Instead we might want to create some API like arnForExecuteApi
as in apigateway.RestApi
.
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.
Yes, makes sense.
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'll add this API for both httpApi and webSocketApi in a future PR.
@nija-at thank you for the review! now ready for a new review:) |
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.
Looks much better. See some comments below.
* | ||
* @param identity The principal | ||
*/ | ||
public grantManageConnections(identity: IGrantable): Grant { |
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.
Consider adding this method to IWebsocketApi
. Then, this feature will be available to imported APIs as well.
This would mean creating a new base class and moving this implementation to it.
class WebsocketApiBase extends ApiBase {
// ...
public grantManageConnections(...) {
// ...
}
}
The same applies to Stage.
This is strictly not necessary for this PR if you prefer to not do this now.
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.
yes I prefer to do this in another PR. I think we can create a new issue to support importing a webSocketApi and refactor them in a corresponding PR.
@nija-at ready for a new review again :) |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
closes aws#14828 By this PR, we can allow access to management API by the following code. ```ts const api = new WebSocketApi(stack, 'Api'); const defaultStage = new WebSocketStage(stack, 'Stage', { webSocketApi: api, stageName: 'dev', }); const principal = new User(stack, 'User'); api.grantManagementApiAccess(principal); // allow access to the management API for all the stage defaultStage.grantManagementApiAccess(principal); // allow access to the management API for a specific stage ``` We use WebSocket API Management API to send messages to a WebSocket API. [(doc)](https://docs.aws.amazon.com/apigateway/latest/developerguide/apigateway-how-to-call-websocket-api-connections.html) To use the API, we must set IAM statement as below [(doc)](https://docs.aws.amazon.com/apigateway/latest/developerguide/apigateway-websocket-control-access-iam.html): ```json { "Effect": "Allow", "Action": [ "execute-api:ManageConnections" ], "Resource": [ "arn:aws:execute-api:us-east-1:account-id:api-id/stage-name/POST/@connections/*" ] } ``` We need `/*` at the end of resource ARN because there will be arbitrary strings (`connectionId`). i.e. `{apiArn}/{stageName}/POST/@connections/{connectionId}` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
closes #14828
By this PR, we can allow access to management API by the following code.
We use WebSocket API Management API to send messages to a WebSocket API. (doc) To use the API, we must set IAM statement as below (doc):
We need
/*
at the end of resource ARN because there will be arbitrary strings (connectionId
).i.e.
{apiArn}/{stageName}/POST/@connections/{connectionId}
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license