fix(lambda): Use Alias ARN directly

[RomainMuller]

Use the Alias resource's ARN instead of building one using Fn::Join, so
that referencing the ARN implies a dependency on the alias.

Additionally, forward the underlying function's role to the Alias' role
property.

Fixes #2084

[eladb]

Test please?

[rix0rrr]

functionName is a horrible name here. It always was, of course, but can we please fix this before we 1.0 this accidentally?

Can you do me a favour and extend the scope of this PR slightly to add invokeArn: string to IFunction ?

[RomainMuller]

I'd actually want to keep the actual Function Name (and make the Alias' one actually a name - like ${this. underlyingLambda.functionName}:${props.aliasName}, or parsing it out the Alias' functionArn even).

Having the name can be useful if you want to include it in a Dashboard or something... And functionArn is your invokeArn.

[sam-goodwin]

Is the following configuration appropriate?

on lambda.Function:

• functionName is the actual function name
• functionArn is the function ARN
• invokeArn is also the functionArn

on lambda.Alias:

• functionName is the underlying function name
• functionArn is the underlying function ARN
• invokeArn is the alias's ARN
• aliasArn (not on IFunction) is another reference to invokeArn

[sam-goodwin]

Diving deeper into this, I agree with @RomainMuller that invokeArn doesn't make sense.

Looking at the documentation for FunctionName on AWS::Lambda::Permission

The name (physical ID), Amazon Resource Name (ARN), or alias ARN of the Lambda function that you want to associate with this statement. Lambda adds this statement to the function's access policy.

This suggests we shouldn't forward the addPermission call to the underlying lambda function - we should use the aliasArn for the functionName when constructing the AWS::Lambda::Permission:

https://github.com/awslabs/aws-cdk/blob/4e105a3904c6bd91f66d60e2d77ab42afe36549b/packages/%40aws-cdk/aws-lambda/lib/function-base.ts#L175-L182

The documentation for Versioning, Aliases, and Permissions suggests that we should include the aliasName in the Alias's functionName:

Assuming that you grant permission, the next question is, "can an event source invoke a function version using any of the associated ARNs?" The answer is, it depends on how you identified function in your add permissions request (see AddPermission). The key to understanding this is that the permission you grant apply only to the ARN used in the add permission request:

Specifically,

If you use an alias name (such as helloworld:BETA), the permission is valid only for invoking the helloworld function using the BETA alias ARN (using any other ARNs results in a permission error, including the function version ARN to which the alias points).

I suggest the following:

lambda.Function:

• functionName is the actual function name (Ref).
• functionArn is the function ARN

lambda.Alias:

• functionName is ${underlyingFunction.functionName}:${aliasName}
• functionArn is the alias ARN
• No specialized behavior for addPermission - don't forward to underlying lambda.
• Specialized behavior for metric so the alias's ARN is included in the dimensions:

{
  FunctionName: this.underlyingLambda.functionName,
  Resource: this.functionArn // alias ARN
}

as opposed to (see lambda-augmented.generated.ts):

dimensions: { FunctionName: this.functionName },

lambda.FunctionBase:

• Use this.functionArn as the functionName when creating permissions

[rix0rrr]

And functionArn is your invokeArn.

Yes, except all Lambda APIs where you pass in what Lambda to invoke, call the argument functionName (because it accepts both name and ARN, but they named the field after the trivial case).

So this means that everywhere you want to invoke a Lambda you now need to do:

new ConstructThatCallsLambda(this, {
  functionName: myFunctionLike.functionArn  // <-- Resist typing functionName here, which looks correct but is wrong!
});

Maybe then we call it functionInvokeName or something.

[rix0rrr]

• functionName is ${underlyingFunction.functionName}:${aliasName}

This needs a mechanism to add a dependency from the consumer to the alias then, probably hooking into Token in some way. Maybe Elad was right about references needing a generic callback, we could have used that :).

• Specialized behavior for metric so the alias's ARN is included in the dimensions:

Looking at the metrics in beta right now, I don't see Alias names in the metrics, just plain function names. Where did you get that the Alias name is used in the metrics?