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

feat(s3)!: enable native s3 support via the rust sdk #89

Merged
merged 1 commit into from Feb 15, 2024

Conversation

BogdanVidrean
Copy link
Collaborator

@BogdanVidrean BogdanVidrean commented Jan 15, 2024

Problem

Dali only supports processing images that could be downloaded from an HTTP server without any authentication layer. This isn't very optimal when the images that need to be processed are stored in a private S3 bucket while Dali runs in an EKS cluster. The normal access strategy is using the EKS OIDC provider to assume an IAM role that is allowed to read from the bucket while being mapped to a K8S service account.

One alternative is to use a gateway endpoint, but this requires VPC whitelisting at the bucket level. This can cause dangerous side effects, such as potential undesired access of multiple services running in the same VPC like the service that needs S3 access.

Solution

The premise of this PR originally was to introduce the S3 sdk to Dali in order to take advantage of the OIDC authentication of the EKS cluster.

Unfortunately this couldn't have been achieved with the current setup of Dali as even the oldest AWS Rust SDK was not compatible with Dali's 4 years old runtime actix-rt. The first natural approach was to upgrade Actix (the web library behind Dali), but it proves that the new version isn't running well at all as it doesn't seem to handle memory very well due to some changes in the caching mechanism (issue and issue). In our case we reproduced a similar behavior when the pods would eventually run out of memory.

Due to the previously mentioned issue, I've switched the web library from Actix to Axum and the runtime to tokio from actix-rt. Axum grew a lot in the Rust web ecosystem due to it's very good performance and easiness to be configured. As part of this, I've also tried to restructure the project a bit to make it more readable.

The next step, is configuring the S3 SDK in an abstract way hence Dali continues to support an HTTP server as well while leaving it open closed for extension in case of the need to support other storage providers. For this I've created the ImageProvider trait.

Also, this PR conveniently introduces a local development environment that emulates both image providers for Dali:

  1. HTTP server
  2. S3 bucket

It consists of a MinIO server running two buckets:

  1. A public one whose objects can be fetched via HTTP
  2. A private one whose objects can be fetched using the S3 SDK as MinIO is fully compatible with it.

PS: The PR is still marked as Draft due to some GitHub Actions that need to be slightly changed, but from the code's perspective it's ready to be reviewed.

@BogdanVidrean BogdanVidrean changed the title feat(s3): enable native s3 support via the rust sdk feat(s3)!: enable native s3 support via the rust sdk Feb 7, 2024
Copy link

github-actions bot commented Feb 12, 2024

Hey there and thank you for opening this pull request! 👋🏼

You can pull a preview container image of dali with the below command:

docker pull ghcr.io/olxgroup-oss/dali/dali:feat-s3--enable-s3-support
docker pull ghcr.io/olxgroup-oss/dali/dali-s3:feat-s3--enable-s3-support

@BogdanVidrean BogdanVidrean marked this pull request as ready for review February 13, 2024 18:24
BREAKING CHANGE: replaced actix with axum
@bassco bassco merged commit 5e514a7 into master Feb 15, 2024
4 checks passed
@bassco bassco deleted the feat(s3)-enable-s3-support branch February 15, 2024 15:40
@bassco
Copy link
Collaborator

bassco commented Feb 15, 2024

This PR is included in version 2.0.0 🎉

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.

None yet

2 participants