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

Enable s3 for f6 #134

Merged
merged 7 commits into from
May 31, 2021
Merged

Conversation

emudojo
Copy link
Contributor

@emudojo emudojo commented Apr 1, 2021

No description provided.

@nigelgbanks
Copy link
Contributor

I tried to get this working with minio (see the attached tests), with the hopes of doing some automated testing, but couldn't get it working. That being said it seems to be more so an issue with Fedora and how it's configured to assume s3 and not some s3 compliant API which might explain that.

Additionally the output doesn't seem to match up in that it thinks no access key / secret key is set? Though this again may be some weirdness around Fedora. They do not really document it directly here: https://wiki.lyrasis.org/display/FEDORA6x/Amazon+S3, but rather point to the AWS documentation: https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/credentials.html. Which according to that the implementation here is correct so I don't know what to make of that.

fcrepo_1    | INFO 11:49:46.259 (OcflPropsConfig) Fedora AWS access key: 
fcrepo_1    | INFO 11:49:46.259 (OcflPropsConfig) Fedora AWS secret key set: false
fcrepo_1    | INFO 11:49:46.259 (OcflPropsConfig) Fedora AWS region: us-west-2
fcrepo_1    | INFO 11:49:46.259 (OcflPropsConfig) Fedora OCFL S3 bucket: minio.traefik.me/fcrepo
fcrepo_1    | INFO 11:49:46.259 (OcflPropsConfig) Fedora OCFL S3 prefix: 
fcrepo_1    | INFO 11:49:46.259 (OcflPropsConfig) Fedora OCFL digest algorithm: SHA-512

That being said I haven't tried this against an actual s3 bucket with credentials, but I'm happy to merge this as is, but will leave it for a day or so for anyone else who can / wants to test on an actual s3 bucket.

test.zip

@nigelgbanks
Copy link
Contributor

@emudojo is this ready to be merged or are you still testing against s3?

@elizoller
Copy link
Member

is there anything holding up this PR?

@noahwsmith
Copy link
Contributor

I'll get this back on my radar to test/tweak/merge

@noahwsmith
Copy link
Contributor

Ok, to Nigel's question we're still waiting on Minio support on the Fedora side. The next time they cut a release we can work in a custom endpoint variable to support this. This work has been done here: fcrepo/fcrepo#1894

The other GOTCHA to know about is that you have to use Postgres as your FCCREPO_DB_HOST if you want this to work. That's also just a function of how F6 works right now.

We've been using this Fedora6 image on our Whitman/Lyrasis/IMLS project with no issues for a few weeks now and it works great.

Here are the .env variables we use:

# FCREPO_TAG=d63e8563355b21457a56a8d427af97b82215749e
# FCREPO_BINARYSTORAGE_TYPE=s3
# FCREPO_PERSISTENCE_TYPE=postgresql
# FCREPO_DB_HOST=postgresql
# FCREPO_DB_PORT=5432
# FCREPO_S3_BUCKET=abc123-test
# FCREPO_S3_USER=AK123...
# FCREPO_S3_PASSWORD=1J...
# FCREPO_S3_PREFIX=
# FCREPO_AWS_REGION=us-east-1

I'm not sure what we need to do to get some of this into isle-dc (pinging @nigelgbanks on that front), but I believe this PR can be accepted as-is.

@nigelgbanks nigelgbanks merged commit b7bcc90 into Islandora-Devops:main May 31, 2021
@nigelgbanks
Copy link
Contributor

I've merged it, my only concern was that it had been tested, the changes here should not cause any existing functionality to break.

@nigelgbanks
Copy link
Contributor

@noahwsmith there doesn't appear to be any pull request against isle-dc for changes, I suppose documentation is the only thing required since this use case won't be the default. Though that being said, since this isn't fully fleshed out and is waiting on changes to F6, might as well just wait till it all works before documenting it to reduce the effort.

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

Successfully merging this pull request may close these issues.

4 participants