Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't attach bucket policy to imported Buckets that are created using CDK but made in other stacks (With proposed solution) #785

Open
whatsrupp opened this issue Mar 13, 2023 · 3 comments
Labels

Comments

@whatsrupp
Copy link

First of all, this project has saved me a lot of time, so thank you for that.

I'm opening this issue because I'm finding that I need to be able to control certain aspects of the Lambda Function in order to use it across other stacks that use it.

I mainly need:

  1. The ability to give the Scanning Lambda and the Scanning Lambda Role a concrete name so that I can use them to create bucket policies using CDK in imported bucket stacks with confidence.

In short:
Proposed Solution - Parameterise the Scan Lambda Name and Role Name .
Would this cause issues?

In Long:
The problem I am having without (1) is that I can't reference the lambda in other stacks where I create S3 buckets.
I have existing buckets. They are made in CDK in other stacks. There are a good amount (20ish) of buckets. This can't be changed.

I am importing those buckets into the stack that creates the ServerlessClamscan construct.

// Bucket Stack

const bucket = new S3.Bucket(this, "bucketID", "BucketA")
// Clamscan Stack
    const importedBucket = S3.Bucket.fromBucketName(this, `imported-bucket-${bucketName}`, "BucketA");

    const sc = new ServerlessClamscan(this, "s3-bucket-antivirus-scanner", {
      buckets: [importedBucket],
      acceptResponsibilityForUsingImportedBucket: true,
    });

In the bucket Stack I would like to be attach the scan policy with CDK. this would require having access to the scan function rolename and lambda name. To avoid a circular dependency I would like to pass in the Scan Lambda Name and Scan Lambda Role Name into both the ServerlessClamscan stack and the Bucket Stack. This would allow me to not have manual deploy steps

// Bucket Stack

const bucket = new S3.Bucket(this, "bucketID", "BucketA")

   
    getPolicyStatementForBucket(){
       // manually reimplement in the Bucket Stack OR export a helper function that consistently generates the correct bucket policy 
       // possible if we pass in the scan function role name and scan function name
    }
    const result = bucket.addToResourcePolicy(
        getPolicyStatementForBucket(bucket),
     );

@whatsrupp whatsrupp changed the title Can't access Lambda Metadata or extend the Lambda Function Can't attach bucket policy to imported Buckets that are created using CDK but made in other stacks (With proposed solution) Mar 13, 2023
@dontirun
Copy link
Contributor

While I understand that this would be useful in your use case, I don't think it's a good idea to make these values configurable. It's generally considered an anti-pattern in the CDK to statically define resource names. This seems like a niche case where it may be helpful and I don't want to encourage an anti-pattern by exposing these values through the props.

With that in mind I believe you can use escape hatches in your case to override the names of the resources that need to be statically named

@whatsrupp
Copy link
Author

whatsrupp commented Mar 15, 2023

Ok, fair enough, good to know about the escape hatches cheers!!

Interesting that you think it's a niche use case, but take the point!!

I'd raised it as I thought it would probably be quite likely that people adding the antivirus scanner into an existing cdk ecosystem wouldn't want to manually attach the scan policy to bucket in their existing cdk stacks manually.
It would be pretty powerful to be able to do it with CDK.

If I'm understanding correctly - It's kind of deferring the need to sort out that problem to the other stack, where you'd have to hardcode the inline policy. or do a look up for the lambda execution role/ function by manually written arn or name, which we're not exposing as part of the public interface. So it's kind of indirectly encouraging that sort of hard coded resource name antipattern in other stacks rather than in this stack.

Have now read your other comment in this issue about this originally not allowing external buckets - so I guess it makes sense this hasn't been addressed.

@dontirun
Copy link
Contributor

If I'm understanding correctly - It's kind of deferring the need to sort out that problem to the other stack, where you'd have to hardcode the inline policy. or do a look up for the lambda execution role/ function by manually written arn or name, which we're not exposing as part of the public interface.

Not really. Escape hatches are a less convenient way of doing what you wanted to do. You're giving the lambda function and role the same hardcoded values that would be exposed through a property and then doing the same operations you would be on the S3 Stack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants