search

falcondev-oss / github-actions-cache-server

Self-hosted GitHub Actions cache server implementation. Compatible with official 'actions/cache' action
https://gha-cache-server.falcondev.io
MIT License
92 stars 4 forks source link

Support for S3 #21

Closed oprypkhantc closed 4 months ago

oprypkhantc commented 4 months ago

Hey again :)

We have runners in different regions; some have local infrastructure, including a locally running Minio cache. Others run on AWS infrastructure and use S3 for caching.

Since the storage drivers are extensible now on on dev branch, would you accept a PR adding support for S3 storage driver?

We've attempted to use existing Minio driver for that in hopes that it would work with S3, but it seems that the Minio SDK isn't compatible with S3: Bucket test-github-cache-server does not exist. That's with the correct region, port, endpoint and SSL all set to mimic the S3 SDK.

DrJume commented 4 months ago

Hey :)

That's weird, because minio should be compatible with all kinds of S3 providers 😮

The error you mentioned is thrown by us, because we don't (yet) create the bucket automatically.

Just to be sure: have you tried to create the bucket before starting the gha cache server? 🙏

oprypkhantc commented 4 months ago

Yes, the bucket was indeed created. It was our devops who tested that, but to make sure it works I tested it myself with my bucket and it seems to be working as long as I also pass in the region in minio.ts 🙃

So seems like my PR is useless 🥲 We'll try again tomorrow. I'll create a different PR with some changes to the docs and adding a MINIO_REGION variable.

DrJume commented 4 months ago

Ok 👍

Yeah, that would be unfortunate :( Maybe we can use the opportunity to also rename all "minio" wording to "S3" (but keeping the more minimal minio SDK) as it is compatible and maybe create the bucket if it does not exist, as it could simplify setup.

oprypkhantc commented 4 months ago

Sure, I was thinking of that too. Would it make sense to also add defaults for S3_ENDPOINT, S3_PORT and S3_USE_SSL pointing to AWS S3?

LouisHaftmann commented 4 months ago

maybe we could keep both minio and s3 drivers because someone might not know that minio is s3 compatible. the code could be the same.

LouisHaftmann commented 4 months ago

Sure, I was thinking of that too. Would it make sense to also add defaults for S3_ENDPOINT, S3_PORT and S3_USE_SSL pointing to AWS S3?

adding defaults would definitely be useful for simpler setup

LouisHaftmann commented 4 months ago

Ok 👍

Yeah, that would be unfortunate :( Maybe we can use the opportunity to also rename all "minio" wording to "S3" (but keeping the more minimal minio SDK) as it is compatible and maybe create the bucket if it does not exist, as it could simplify setup.

i think creating the bucket automatically wouldn't work as the check also fails if we don't have permission to access the bucket. we should probably update the error message (maybe the error contains whether the bucket doesn't exist or we don't have perms)

oprypkhantc commented 4 months ago

maybe we could keep both minio and s3 drivers because someone might not know that minio is s3 compatible. the code could be the same.

I could alias both s3 and minio to the same s3/minio driver and rename the page in the docs. Also I've made sure to allow maintainers to make edits on my branch, so you're free to make any changes if you wish :)

LouisHaftmann commented 4 months ago

i changed my mind, let's just add s3 and mention minio in the docs