Conversation
trend-jack-c-tang
left a comment
There was a problem hiding this comment.
Suggest adding the output format in the doc.
| Metadata: | ||
| AWS::ServerlessRepo::Application: | ||
| Name: cloudone-filestorage-plugin-publish-malware-to-sns | ||
| Description: >- | ||
| According to the scan result from the scanner, publish the malware information | ||
| to the target SNS topic. | ||
| Author: Trend Micro Cloud One File Storage Security |
There was a problem hiding this comment.
We're not going to publish this to AWS serverless repository, so this is not required.
There was a problem hiding this comment.
Will remove in next commit.
| TargetSnsTopicName: | ||
| Type: String | ||
| Description: The name for the SNS topic to which malware information publish. |
There was a problem hiding this comment.
The customer may already have their own SNS topic. Could you figure out a way for using the existing SNS topic?
There was a problem hiding this comment.
I will add 1 more parameter to let user fill in their customer SNS Topic ARN, and create a new one SNS topic if the value is empty string.
| PublishMalwareLambdaName: | ||
| Type: String | ||
| Description: The name for the lambda function publishes the malware information to SNS topic. | ||
| PublishMalwarePolicyName: | ||
| Type: String | ||
| Description: The name for the policy includes sns and account authority. | ||
| PublishMalwareRoleName: | ||
| Type: String | ||
| Description: The name for the role to execute the lambda function. |
There was a problem hiding this comment.
For internal use resources, we usually don't let the customer choose the names. There might be problems, like name already existed, that would block customer from deploying the template. Any purpose for these parameters?
There was a problem hiding this comment.
I will remove these and using the default name (stack name).
| Handler: handler.lambda_handler | ||
| FunctionName: !Ref PublishMalwareLambdaName | ||
| Runtime: python3.8 | ||
| Timeout: 500 |
There was a problem hiding this comment.
How do you choose the timeout for this Lambda?
| Properties: | ||
| FunctionName: !GetAtt PublishMalwareLambda.Arn | ||
| Action: lambda:InvokeFunction | ||
| Principal: events.amazonaws.com |
There was a problem hiding this comment.
This is the endpoint of AWS eventbridge. Should be sns.amazonaws.com?
There was a problem hiding this comment.
Thanks, I'm very confused for why I can't receive the notification from SNS.
| for i, scan_result in enumerate(scan_results) | ||
| ] | ||
|
|
||
| response = sns_client.publish_batch( |
There was a problem hiding this comment.
SNS batch publish supports up to 10 messages, so be careful if the received events count would be more than 10. And you may need to handle partial failure cases.
| --runtime python3.9 \ | ||
| --timeout 30 \ |
There was a problem hiding this comment.
They are not matched with ones in the template. Why?
There was a problem hiding this comment.
Will align them in the next commit.
There was a problem hiding this comment.
The runtime is not aligned still.
There was a problem hiding this comment.
Sorry for the lost, fixed already.
|
|
||
| scan_results.append({ | ||
| 'bucket_name': get_bucket_name(message['file_url']), | ||
| 'account_name': fetch_account_name(), |
There was a problem hiding this comment.
Can the account name be cached?
There was a problem hiding this comment.
Yup, move out from the for loop.
| --runtime python3.9 \ | ||
| --timeout 30 \ |
There was a problem hiding this comment.
The runtime is not aligned still.
|
|
||
| if fails: | ||
| print(fails) | ||
| raise ValueError('fail to publish sns.') |
There was a problem hiding this comment.
Unhandled error will make the Lambda retry the whole batch again. Some messages will be duplicated. In this case, I think we can just log them and let the execution finish successfully.
| - Arn | ||
| Environment: | ||
| Variables: | ||
| TARGET_SNS_ARN: !If [NeedCreateNewSnsTopic, !Ref TargetSnsTopic, !Ref TargetSnsTopicArn, ] |
There was a problem hiding this comment.
| TARGET_SNS_ARN: !If [NeedCreateNewSnsTopic, !Ref TargetSnsTopic, !Ref TargetSnsTopicArn, ] | |
| TARGET_SNS_ARN: !If [ NeedCreateNewSnsTopic, !Ref TargetSnsTopic, !Ref TargetSnsTopicArn ] |
TITLE
Change Summary
PR Checklist
Other Notes