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

[JENKINS-40654] Make plugin compatible with storage backends compatible with Amazon S3 (OpenStack Swift...) #100

Merged
merged 2 commits into from Dec 23, 2016

Conversation

Projects
None yet
2 participants
@cyrille-leclerc
Copy link

commented Dec 23, 2016

[JENKINS-40654] Make plugin compatible with storage backends compatible with Amazon S3 (OpenStack Swift...)

Load the list of Amazon S3 regions from c.a.r.RegionMetadataProvider, stop relying on a static lists and constants.

  • Adapt the http proxy logic of the plugin to discover the S3 endpoint hostname with com.amazonaws.regions.Region#getServiceEndpoint("s3") instead of the hardcoded constant s3.amazonaws.com
  • Replace
    • com.amazonaws.regions.Regions by com.amazonaws.regions.RegionUtils#getRegionsForService("s3")
    • com.amazonaws.services.s3.model.Region by com.amazonaws.regions.Region
  • For the default region used in some places of the plugin, introduce the system property hudson.plugins.s3.DEFAULT_AMAZON_S3_REGION to override the default us-east-1. Note that it would be better to no longer rely on a default AWS Region and to ask the user to specify the desired AWS Region.

The list of AWS regions can be overridden specifying a file classpath://com/amazonaws/partitions/override/endpoints.json matching the format defined in https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-core/src/main/resources/com/amazonaws/partitions/endpoints.json .

A solution to add this file in the classpath of Jenkins is to use the java command line parameter -Xbootclasspath/a:/path/to/boot/classpath/folder/ and to locate com/amazonaws/partitions/override/endpoints.json in /path/to/boot/classpath/folder/.

[JENKINS-40654] Load the list of Amazon S3 regions from {{c.a.r.Regio…
…nMetadataProvider}}, stop relying on a static lists and constants.

* Adapt the http proxy logic of the plugin to discover the S3 endpoint hostname with `com.amazonaws.regions.Region#getServiceEndpoint("s3")` instead of the hardcoded constant `s3.amazonaws.com`
* Replace
   * `com.amazonaws.regions.Regions` by `com.amazonaws.regions.RegionUtils#getRegionsForService("s3")`
   * `com.amazonaws.services.s3.model.Region` by `com.amazonaws.regions.Region`
* For the default region used in some places of the plugin, introduce the system property `hudson.plugins.s3.DEFAULT_AMAZON_S3_REGION` to override the default `us-east-1`. Note that it would be better to no longer rely on a default AWS Region and to ask the user to specify the desired AWS Region.

The list of AWS regions can be overridden specifying a file `classpath://com/amazonaws/partitions/override/endpoints.json` matching the format defined in https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-core/src/main/resources/com/amazonaws/partitions/endpoints.json .

A solution to add this file in the classpath of Jenkins is to use the `java` command line parameter `-Xbootclasspath/a:/path/to/boot/classpath/folder/` and to locate `com/amazonaws/partitions/override/endpoints.json` in `/path/to/boot/classpath/folder/`.
@Jimilian
Copy link

left a comment

Sounds awesome! Thanks a lot! (It's interesting who did request such stuff from Cloudbees ;) )

Can you update README, please? I think this information should be there as well (or even better - put it to help for Region selector).

The list of AWS regions can be overridden specifying a file classpath://com/amazonaws/partitions/override/endpoints.json matching the format defined in https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-core/src/main/resources/com/amazonaws/partitions/endpoints.json .

A solution to add this file in the classpath of Jenkins is to use the java command line parameter -Xbootclasspath/a:/path/to/boot/classpath/folder/ and to locate com/amazonaws/partitions/override/endpoints.json in /path/to/boot/classpath/folder/.

{
client.setRegion(getRegionFromString(region));
}
client.setRegion(awsRegion);

This comment has been minimized.

Copy link
@Jimilian

Jimilian Dec 23, 2016

I think this line could be moved inside getClientConfiguration.

This comment has been minimized.

Copy link
@cyrille-leclerc

cyrille-leclerc Dec 23, 2016

Author

I don't think that we can move this inside getClientConfiguration() as the AmazonS3Client is created after the invocation of getClientConfiguration().

This comment has been minimized.

Copy link
@Jimilian

Jimilian Dec 23, 2016

Yes, true. You are right.

*
* @param regionName nullable region name
* @return AWS region, never {@code null}, defaults to {@link com.amazonaws.services.s3.model.Region#US_Standard}
*/
private static Region getRegionFromString(String regionName) {

This comment has been minimized.

Copy link
@Jimilian

Jimilian Dec 23, 2016

If it's nullable maybe it make sense to add @Nullable?

@@ -1,6 +1,9 @@
package hudson.plugins.s3;

import com.amazonaws.regions.Region;
import com.amazonaws.regions.RegionUtils;
import com.amazonaws.regions.Regions;

This comment has been minimized.

Copy link
@Jimilian
[JENKINS-40654] @Jimilian’s recommendations
* Document `classpath://com/amazonaws/partitions/override/endpoints.json` and `hudson.plugins.s3.DEFAULT_AMAZON_S3_REGION` in README.md
* Use `@nonnull` and `@nullable` in helper methods (not in the whole ClientHelper class to not change too many things at once)
* Cleanup imports
@cyrille-leclerc

This comment has been minimized.

Copy link
Author

commented Dec 23, 2016

Thanks @Jimilian, I have updated the code and docs as suggested.

@Jimilian Jimilian merged commit 945f331 into jenkinsci:master Dec 23, 2016

1 check passed

Jenkins This pull request looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.