aws-samples / image-optimization

Simple, performant and cost efficient solution for optimizing images using Amazon CloudFront, Amazon S3 and AWS Lambda
MIT No Attribution
195 stars 116 forks source link

how to skip objects that served from the bucket that is not image? #31

Closed yangcheng closed 3 months ago

yangcheng commented 8 months ago

I saw url_rewriter has checks for suffix, but I still get

Input buffer contains unsupported image format when i try to visit a txt or pdf file.

How to make the function skip objects without image suffix?

Thanks

achrafsouk commented 8 months ago

For the best and optimal solution to avoid processing files other than images by this solution, is to create seperate CloudFront cache behaviors with/without this solution according to your files. For example, you can create behaviors for .jpg .png etc extensions and only apply the solution there, and for the default behavior just point to the bucket without any processing.

yangcheng commented 8 months ago

Thanks, that is indeed a good for way separation of concerns.

yangcheng commented 8 months ago

@achrafsouk if I want to add the config into stack, so the behavior is created during cdk deploy, where should I change the code? Thanks a lot

achrafsouk commented 8 months ago

You can make the change in https://github.com/aws-samples/image-optimization/blob/main/lib/image-optimization-stack.ts, specifically in by changing this section:

const imageDelivery = new cloudfront.Distribution(this, 'imageDeliveryDistribution', {
      comment: 'image optimization - image delivery',
      defaultBehavior: imageDeliveryCacheBehaviorConfig
});
krashnakant commented 8 months ago

can you suggest the sample code please ? as pathPattern: "*.jpeg", did not possible in Distribution. thanks

achrafsouk commented 8 months ago

sure, it will be something like

const myDistribution = new cloudfront.Distribution(this, 'imageDeliveryDistribution', {
      additionalBehaviors: {
        '*.jpg': imageDeliveryCacheBehaviorConfig,
        '*.jpeg': imageDeliveryCacheBehaviorConfig,
        '*.png': imageDeliveryCacheBehaviorConfig
      },
      defaultBehavior: {
        origin: new origins.S3Origin(YOUR_DEFAULT_S3_BUCKET),
      }
});
alexei commented 4 months ago

Thanks for the suggestion, @achrafsouk Following your example, what should be used for YOUR_DEFAULT_S3_BUCKET? When I'm using:

const imageDelivery = new cloudfront.Distribution(this, 'imageDeliveryDistribution', {
  comment: 'image optimization - image delivery',
  defaultBehavior: {
    origin: new origins.S3Origin(originalImageBucket)
  },
  additionalBehaviors: {
    '*.jpg': imageDeliveryCacheBehaviorConfig,
    '*.jpeg': imageDeliveryCacheBehaviorConfig,
    '*.png': imageDeliveryCacheBehaviorConfig,
    '*.gif': imageDeliveryCacheBehaviorConfig
  }
})

I'm getting "Invalid request provided: AWS::CloudFront::Distribution: Illegal configuration: The origin type and OAC origin type differ" until I set origin to imageOrigin. However, I think that's the bucket where the transformed images are stored. What do we do?

achrafsouk commented 4 months ago

Since I made that comment, I made a change to the code to support OAC with Lambda URL, notably the following line:

cfnImageDelivery.addPropertyOverride(`DistributionConfig.Origins.${(STORE_TRANSFORMED_IMAGES === 'true')?"1":"0"}.OriginAccessControlId`, oac.getAtt("Id"));

When you are creating a new origin, I suspect that according to the above code, the OAC is wrongly applied to the S3 bucket, cuasing issues. Try to change to:

cfnImageDelivery.addPropertyOverride(`DistributionConfig.Origins.${(STORE_TRANSFORMED_IMAGES === 'true')?"2":"1"}.OriginAccessControlId`, oac.getAtt("Id"));

alexei commented 4 months ago

Thanks @achrafsouk I can confirm your suggestions work:

    const imageDelivery = new cloudfront.Distribution(this, 'imageDeliveryDistribution', {
      comment: 'image optimization - image delivery',
      defaultBehavior: {
        origin: new origins.S3Origin(originalImageBucket)
      },
      additionalBehaviors: {
        '*.jpg': imageDeliveryCacheBehaviorConfig,
        '*.jpeg': imageDeliveryCacheBehaviorConfig,
        '*.png': imageDeliveryCacheBehaviorConfig,
        '*.gif': imageDeliveryCacheBehaviorConfig
      }
    });

    ...

    cfnImageDelivery.addPropertyOverride(`DistributionConfig.Origins.${(STORE_TRANSFORMED_IMAGES === 'true')?"2":"1"}.OriginAccessControlId`, oac.getAtt("Id"));
alexei commented 4 months ago

@achrafsouk what do you think about adding this config, maybe behind a parameter e.g. CLOUDFRONT_FILTER_IMAGES?

achrafsouk commented 4 months ago

I will consider it in the roadmap. Can you tell me what is the application you are using it for?

alexei commented 4 months ago

We store all user uploaded files in the same bucket. They're a mix of images and documents (e.g. PDFs) and it would be difficult to store/serve them separately.

gxolin commented 4 months ago

I have the same use case : On a front application, I upload both my code and my assets in the same S3 bucket served over cloudfront. Not having two separate cloudfront (one for optimised images, the other one for other assets) could be a nice addition. (It's not critical, I can use the optimised images for all images and the default one for the rest, but I will have to establish two connections : optimised-images.cloudfront.net and assets.cloudfront.net instead of just assets.cloudfront.net)

Edit for additional information: I also have a media bucket for uploads, containing both images and other files like PDFs. The same applies for them, so in total without conditional behaviors I would end with 4 buckets : optimized-images-assets, assets, optimised-images-media, media instead of just two : assets and media distributions

alexei commented 4 months ago

@gxolin thanks for adding to this. I want to point out, however, this is talking about user uploaded files, not site assets. Separating site assets from user uploads can be done during development and as such I think it's easier than distinguishing image uploads from non-image ones. Of course YMMV depending on framework etc.

alexei commented 3 months ago

@achrafsouk I re-deployed the latest changes in main, and found the CDN responds with a 502 for non-image assets. Indeed, I checked the latest changes and there's no trace of fixing this. Does closing the issue mean there's no intention of fixing it, and each developer will have to do it themselves following instructions found here?