-
Notifications
You must be signed in to change notification settings - Fork 7
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
s3: Support S3 compatible services like Google Storage #221
base: master
Are you sure you want to change the base?
Conversation
related to work i'm also trying to do here: advancedtelematic/treehub#75 |
Services like Google Storage expose s3 compatible APIs that can accessed with a non-aws URL. Signed-off-by: Andy Doan <[email protected]>
Thanks for the pr. Can you run |
.withRegion(credentials.region) | ||
.build() | ||
protected lazy val s3client = { | ||
if(credentials.endpointUrl.length() > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be:
protected lazy val s3client = {
val builder = AmazonS3ClientBuilder.standard().withCredentials(credentials)
if(credentials.endpointUrl.nonEmpty)
builder.withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(credentials.endpointUrl, credentials.region.getName))
else
builder
}.build()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else
builder.withRegion(credentials.region)
Oder?
Signed-off-by: Andy Doan <[email protected]>
I think this last push addresses the comments. I'm barely capable of writing Scala, so this is just the minimal way I can find to make it work |
ping. |
@@ -50,4 +51,33 @@ class S3StorageResourceIntegrationSpec extends TufReposerverSpec | |||
header("Location").get.value() should include("amazonaws.com") | |||
} | |||
} | |||
|
|||
test("uploading a target changes targets json with custom endpoint url") { | |||
lazy val credentials = new S3Credentials("", "", "", Regions.fromName("us-central-1"), "https://storage.googleapis.com") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test runs with my credentials, which are just aws credentials, not google, something wrong here?
@@ -17,6 +17,7 @@ storage { | |||
bucketId = ${?TUF_REPOSERVER_AWS_BUCKET_ID} | |||
region = "eu-central-1" | |||
region = ${?TUF_REPOSERVER_AWS_REGION} | |||
endpointUrl = ${?TUF_REPOSERVER_S3_URL} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs an extra line:
endpointUrl = ""
Otherwise wont run without TUF_REPOSERVER_S3_URL
set.
@doanac If you still want to merge this please open a PR at https://github.com/uptane/ota-tuf We can get this merged soon. Thanks |
Services like Google Storage expose s3 compatible APIs that can
accessed with a non-aws URL.
Signed-off-by: Andy Doan [email protected]