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

s3: Support S3 compatible services like Google Storage #221

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

doanac
Copy link

@doanac doanac commented Apr 12, 2019

Services like Google Storage expose s3 compatible APIs that can
accessed with a non-aws URL.

Signed-off-by: Andy Doan [email protected]

@doanac
Copy link
Author

doanac commented Apr 12, 2019

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]>
@xaviergully xaviergully requested a review from simao April 15, 2019 18:04
@simao
Copy link
Contributor

simao commented Apr 17, 2019

Thanks for the pr.

Can you run S3StorageResourceIntegrationSpec using the google endpointUrl?

.withRegion(credentials.region)
.build()
protected lazy val s3client = {
if(credentials.endpointUrl.length() > 0) {
Copy link
Contributor

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()

Copy link
Contributor

@houcros houcros Apr 17, 2019

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]>
@doanac
Copy link
Author

doanac commented Apr 23, 2019

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

@doanac
Copy link
Author

doanac commented May 21, 2019

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")
Copy link
Contributor

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}
Copy link
Contributor

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.

@simao
Copy link
Contributor

simao commented Jun 25, 2021

@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

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.

3 participants