aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.71k stars 3.94k forks source link

@aws-cdk/aws-cloudfront.experimental: incorrect documentation on role parameter #21959

Open JudgedPluto opened 2 years ago

JudgedPluto commented 2 years ago

Describe the bug

Lambda@Edge functions does not return the role object when using cloudfront.experimental.EdgeFunction construct.

Expected Behavior

EdgeFunction.role should return an iam.Role/iam.IRole object.

Current Behavior

EdgeFunction.role returns undefined

Reproduction Steps

1. Initiate a typescript project.

mkdir cdk-project && cd cdk-project && cdk init --language typescript

2. Add required dependencies to the project.

import * as cloudfront from "aws-cdk-lib/aws-cloudfront";

3. Create an EdgeFunction resource.

const func = new cloudfront.experimental.EdgeFunction(this, "LambdaAtEdge", {...lambdaProps});

Possible Solution

role property on the EdgeFunction must be initialized.

Additional Information/Context

No response

CDK CLI Version

2.40.0

Framework Version

No response

Node.js Version

18.3

OS

Windows 10 Version: 21H2 Build: 19044

Language

Typescript

Language Version

3.9.7

Other information

No response

peterwoodworth commented 2 years ago

It's not clear to me if this was ever intended to be used. It is likely required to be a property due to EdgeFunction implementing IVersion, but if it never has any reason to be used then it will forever be an undefined property. This may be something that has to be part of our API if EdgeFunction needs to implement IVersion, which it might have to (I'm unsure though)

I am marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

DhwanishShah commented 2 years ago

Hi i want to work on this issue. I am new to this open source community and i want to start with this project. Please assign this issue to me @peterwoodworth

peterwoodworth commented 2 years ago

Thanks for volunteering for this @DhwanishShah,

On a second and deeper look at this, I don't think there's any part of the code base we need to fix. However what we do need to fix is the documentation - the documentation for the role prop is certainly incorrect since it copies the FunctionProps role documentation

default: A unique role will be generated for this lambda function.

Ideally we would override this, but I'm not sure how we can override this documentation since it's being extended, meaning we aren't explicitly declaring this. The documentation for this construct isn't great to begin with, and not having a readme certainly doesn't help. (The documentation you see on the API reference is autogenerated from comments in the code). I would need to look into how to best accomplish this

If you'd like to work on an actual bugfix or feature request, try filtering our issues by good first issue label, most of those should be good starting points for jumping into contributing 🙂

DhwanishShah commented 2 years ago

@peterwoodworth i will look into that documentation part.