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

Add Terraform support #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

MeLlamoPablo
Copy link

@MeLlamoPablo MeLlamoPablo commented May 31, 2020

This PR adds support for creating the entire infrastructure with Terraform. This is great for a number of reasons:

  • It allows users to effortlessly provision the whole infrastructure, instead of having to replicate the tutorial step by step. It should help reduce the number of set up errors, and therefore reduce the developer time spent in providing support (i.e: reduce issues like "Access Denied" in the brower #13)
  • It enables contributors to have a consistent infrastructure. With manual provisioning, configuration errors can easily happen, whereas with IaC forces everyone to have the same infrastructure, which will make development of s3-resizer easier. (i.e: reduce issues like Cyclic redirect with CloudFront #5 - if everyone has the same infra, everyone can "talk in the same language")

Take a look at the new README.md to see how everything works.

While developing this, I made two changes to how the whole flow works. These changes might warrant a new major version.

  • Instead of having one bucket, now there are two buckets: the source and the destionation. The source bucket may be private, while the destination bucket must be public (through CloudFront, the public can not access it direcly).

    I made this change to make it easy for people to integrate s3-resizer with existing infrastructure. Otherwise, the current buckets would have to be imported into the terraform state, which is a bit more complicated.

    Furthermore, I think that this change is a nice security improvement: you can have the original, large files private, and only the resized files accessible to the public. This, in combination with the whitelist, can effectively protect the full-size files.

  • The current guide reccommends to host the lambda from a custom route in a custom stage in an API Gateway, but I found using the $default route and stage to be more simple, because it removes the need for the ?path= shenanigans. The cloudfront route is mapeed 1:1 to the lambda route.

    In consequence, I had to make some modifications to the index.js because now the path variable contains the leading slash.

For the time being, I'll be hosting a custom instance so you can test how it works:

https://d37qm260myncmt.cloudfront.net/500xAUTO/blade-runner-2049.jpg

Thanks for reading! If you have any questions I'll be happy to help :)

In order to comply with new infrastructure, some changes were made to
the code:

- Accept a SOURCE_BUCKET and DESTINATION_BUCKET, instead of keeping it
  everything on the same bucket. This enables easier integration with
  existing infrastructure.

- The "path" is taken from "event.path" instead of
  "event.queryStringParameters.path". This means that it now contains
  the leading slash (i.e. "100x100/foo.jpg" becomes "/100x100/foo.jpg").
@sagidM
Copy link
Owner

sagidM commented Jun 11, 2020

Hi @MeLlamoPablo, I really appreciate your contribution.
I just want to let you know that this PR is taken into consideration.
It is a little hard to make a room for learning Terraform, but once I do it, I will come back.

First thoughts, I would like to keep the project simple and easy to use. So for any new feature, I consider trade-offs between functionality and complexity that the feature brings up.
I have been receiving some requests on the email, but so far, no request about Terraform.

At the same time, I had some thoughts to integrate the project with other stuff services (e.g. Google Cloud), in which case I will have to rename it. But again, I want to keep the current simplicity.

Looking forward to coming back soon.

@sagidM sagidM self-assigned this Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants