Feat: Implement Log Groups properties for CloudFrontAuthorization

[pierresouchay]

Logs and naming is currently problematic in CloudFront authorization@edge

• lambdas are named with random IDs, making it difficult to identify easily the deployed lambdas
• log groups are created without tags and offer no possibilies of setting their retention
• logs are impossible to identify: each stack being deployed has random IDs, so not possible to correlate logs easily

As LogGroup sucks with CloudFormation, because LogGroup has to be created before the lambda which creates the log group if not present, this MR follows this strategy:

• Use a common ResourceSuffix (initialized with the StackId) => Possible not to use an ID but a name for user
• Re-Use the suffix for both LogGroupName and Lambda, so IDs are predictable and can be hardcoded for logs
• Use default log retention policy of 10 years to avoid bad suprises for existing users

[ottokruse]

This looks AWESOME and thank you very much!!

One annoying caveat though. CloudFront logs for Lambda@Edge functions are peculiar, they do not actually store the logs in the usual log group with name similar to the Lambda function name in the region of the Lambda function. Instead Lambda@Edge logs get created in the region where the edge function happens to run (can be any region, depends on which edge location the user's request ended up hitting), and the log group name will be prefixed with us-east-1.

Example log group name: /aws/lambda/us-east-1.MyStackName-AuthAtEdge-CheckAuthHandler-923RGl34OrvM

Have you seen that behavior? What's your take here? This is pretty impossible to fix in a reasonable way using CloudFormation I guess.

Anyways, semi fixing the Lambda function names might still be a good idea! Maybe we should only do it if the parameter prefix is given, and use AWS::NoValue otherwise. I like that better than a default of AWS::StackId, what do you think?

[pierresouchay]

Have you seen that behavior? What's your take here? This is pretty impossible to fix in a reasonable way using CloudFormation I guess.

Hum yes... this sounds complicated... damned CloudFormation... so likely, not possible :(

Anyways, semi fixing the Lambda function names might still be a good idea! Maybe we should only do it if the parameter prefix is given, and use AWS::NoValue otherwise. I like that better than a default of AWS::StackId, what do you think?

I took this approach as it allows to have a similar behavior as the existing one: if you deploy twice the stack, it simply work. Without StackId, it would not as it would create twice the same lambda for 2 different stacks

[ottokruse]

Because of Lambda@Edge log behaviors (as discussed) adding the log groups in this PR is not useful is it? So then we better take them out again.

Naming the lambda's with a ResourceSuffix is still useful but would you mind using the AWS::NoValue if ResourceSuffix is not given.

I took this approach as it allows to have a similar behavior as the existing one: if you deploy twice the stack, it simply work. Without StackId, it would not as it would create twice the same lambda for 2 different stacks

Not sure what went wrong there but CloudFormation is (mostly) deterministic. If you don's provide a lambda function name it will generate one, but it uses a deterministic method for that, and next time you deploy it will be the same name. So using AWS::NoValue should work (you basically make the current way of not providing a lambda function name explicit then).

Also, for exiting users in this stack, with AWS::NoValue this PR would be transparant, it would not recreate Lambdas upon redeployment. Only if you start specifying a ResourceSuffix.

Does that all make sense? Thank you for contributing @pierresouchay

[pierresouchay]

stackID is deterministic. So it would be the same accross deployments. But using no Value would mean that by default, there is a clash between 2 stacks. That is why using StackID is IMHO better

[ottokruse]

Why a clash? I can deploy as many auth@edge stacks in the same account as I want without a clash currently, there is no clash. Under the hood CloudFormation uses stack id to generate the lambda function name (if you don't set it, or, same, set it to AWS:NoValue). So you don't have to explicitly do that. Don't think there's harm in it, but no advantage eithe.

[pierresouchay]

Why a clash?

Because of

Properties:
      LogGroupName:
        Fn::Join:
          - ""
          - ["/aws/lambda/", "CheckAuthHandler", !Ref: "ResourceSuffix"]

=> If Resource Suffix := NoValue => 2 stacks would have the same ID. Initializing with StackID would remove this issue

[pierresouchay]

@ottokruse Pushed my changes without the logs in 2nd commit

[ottokruse]

Ah that's what you mean. We can solve that with:

Conditions:
  UseResourceSuffix: !Not [!Equals [!Ref ResourceSuffix, ""]] # Use "" as default value for ResourceSuffix

...
  Properties:
    FunctionName:
      !If
        - UseResourceSuffix
        - Fn::Join:
          - ""
          - ["/aws/lambda/", "CheckAuthHandler", !Ref: "ResourceSuffix"]
        - AWS::NoValue

[ottokruse]

Do you want to make that change @pierresouchay ?

[pierresouchay]

Do you want to make that change @pierresouchay ?

Done

[ottokruse]

Thanks!

[ottokruse]

Will release to SAR soon

[pierresouchay]

Will release to SAR soon

thank you

[ottokruse]

Published v2.1.9 to the SAR. FYI had to do some additional work to make ResourceSuffix work in #256