Aws cloudwatchlog event schema #8228
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8228 +/- ##
==========================================
- Coverage 88.16% 88.02% -0.14%
==========================================
Files 248 248
Lines 9430 9312 -118
==========================================
- Hits 8314 8197 -117
+ Misses 1116 1115 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
@thewizarodofoz great thanks for that. It looks very good. I have just few minor remarks, also let's remove inline validation from event implementation
| @@ -233,6 +233,10 @@ class AwsProvider { | |||
| UpdateReplacePolicy: { type: 'string' }, | |||
| Condition: { type: 'string' }, | |||
| }, | |||
| awsLogGroup: { | |||
There was a problem hiding this comment.
Let's name it awsLogGroupName
| @@ -233,6 +233,10 @@ class AwsProvider { | |||
| UpdateReplacePolicy: { type: 'string' }, | |||
| Condition: { type: 'string' }, | |||
| }, | |||
| awsLogGroup: { | |||
| type: 'string', | |||
| pattern: '^/aws/[a-zA-Z0-9-_.]+$', | |||
There was a problem hiding this comment.
It doesn't seem to follow pattern in AWS docs: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-logs-subscriptionfilter.html#cfn-cwl-subscriptionfilter-loggroupname
There was a problem hiding this comment.
@thewizarodofoz thanks for update. Please see my comment, also it'll be great to cleanup inline validation
| @@ -233,6 +233,10 @@ class AwsProvider { | |||
| UpdateReplacePolicy: { type: 'string' }, | |||
| Condition: { type: 'string' }, | |||
| }, | |||
| awsLogGroupName: { | |||
| type: 'string', | |||
| pattern: '[.-_/#A-Za-z0-9]+', | |||
There was a problem hiding this comment.
I believe we should either escape - or put it as last character, as otherwise it'll expand to range between . and _ chars.
Also regex should be enclosed into ^ and $, as otherwise we confirm that any part in strings matches and not string as a whole
|
@thewizarodofoz let me know if it's ready for re-review. We'd love to have that in! |
There was a problem hiding this comment.
@thewizarodofoz great thanks for update!
Please see my comment, additionally we also need to cleanup all related inline validation
| @@ -233,6 +233,10 @@ class AwsProvider { | |||
| UpdateReplacePolicy: { type: 'string' }, | |||
| Condition: { type: 'string' }, | |||
| }, | |||
| awsLogGroupName: { | |||
| type: 'string', | |||
| pattern: '^[a-zA-Z0-9-_.]+$', | |||
There was a problem hiding this comment.
hmmm.. this still doesn't follow pattern at https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-logs-subscriptionfilter.html#cfn-cwl-subscriptionfilter-loggroupname
Closes: #8027