fix: Check standard logs in AwsSolutions-CFR3 #1877
Conversation
|
@dontirun Another question which may be out of scope of this PR, but because the logs templates here need to be deployed to Do you know if there's a way to look up this in the Stack? I can see a resource with the ID of |
|
I've just pushed a commit to resolve SSM parameters as well by traversing the stack again. If it's out of scope for this PR we can just revert the commit, but I think others will also be using parameters to try and avoid any cross-stack reference errors as I detailed above, so it could be useful. |
|
@dontirun Have you had time to take an initial look into this? I hope I haven't confused you with too many questions! I'm hoping that the initial issue explains what I'm trying to do, and we can always descope it if we think it's too much. |
|
Sorry about the delay. I'll try to get a review in this week |
I don't think there is a good way to this. In this particular case |
Thanks, I thought it might be out of scope for this PR. I'll remove it, but do you think it'll be useful to implement in a separate PR later down the line? I feel like a few rules could benefit from tracking parameters and/or stack exports and we may be able to seamlessly integrate it into the traversal flow. |
Yes, it's definitely something worth looking into |
|
I've updated |
You'll need to update the version in the The |
|
Ah I see, does that look good to you now? |
| let deliverySourceName; | ||
| for (const child of Stack.of(node).node.findAll()) { | ||
| if (child instanceof CfnDeliverySource) { | ||
| const deliverySourceArn = flattenCfnReference( | ||
| Stack.of(child).resolve(child.resourceArn) | ||
| ); | ||
| const logType = Stack.of(child).resolve(child.logType); | ||
| if ( | ||
| deliverySourceArn === distributionArn && | ||
| logType === 'ACCESS_LOGS' | ||
| ) { | ||
| deliverySourceName = Stack.of(child).resolve(child.name); | ||
| } | ||
| } |
There was a problem hiding this comment.
This doesn't take into account if the stack has multiple delivery sources defined that reference the distribution. If multiple are found then it will always pick the last one
| for (const child of Stack.of(node).node.findAll()) { | ||
| if (child instanceof CfnDelivery) { | ||
| if ( | ||
| deliverySourceName === | ||
| Stack.of(child).resolve(child.deliverySourceName) | ||
| ) { | ||
| return NagRuleCompliance.COMPLIANT; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This should be a nested for loop. We don't need this to execute if a matching delivery source has not been found
Fixes #1876
Opening as a draft as the base implementation works, but I've ran into an issue checking if any
CfnDeliverySourceresources are defined in the entire stack. I'll explain in a code comment.I've also had to update
aws-cdk-libto2.167.0, which is the first version to includeoutputFormatinCfnDeliveryDestination. As I'm only using this in the tests, am I alright not updating it in thepeerDependenciesas well?