Skip to content

Commit

Permalink
JENKINS-47912: Fixed regression
Browse files Browse the repository at this point in the history
  • Loading branch information
hoegertn committed Nov 10, 2017
1 parent 170f324 commit 01c0341
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 25 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ String result = invokeLambda(
# Changelog

## current master
* Fixed regression added by #27 (#JENKINS-47912)

## 1.17
* Add policy for withAWS support - allows an additional policy to be combined with the policy associated with the assumed role.
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/de/taimos/pipeline/aws/AWSClientFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ public static <B extends AwsSyncClientBuilder<?, T>, T> T create(B clientBuilder
}

public static <B extends AwsSyncClientBuilder<?, ?>> B configureBuilder(final B clientBuilder, final EnvVars vars) {
if (clientBuilder == null) {
throw new IllegalArgumentException("ClientBuilder must not be null");
}
if (StringUtils.isNotBlank(vars.get(AWS_ENDPOINT_URL))) {
clientBuilder.setEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(vars.get(AWS_ENDPOINT_URL), vars.get(AWS_REGION)));
} else {
Expand Down
38 changes: 34 additions & 4 deletions src/main/java/de/taimos/pipeline/aws/AbstractS3Step.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

package de.taimos.pipeline.aws;

import java.io.Serializable;

import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
import org.kohsuke.stapler.DataBoundSetter;

Expand Down Expand Up @@ -54,10 +56,38 @@ public void setPayloadSigningEnabled(final boolean payloadSigningEnabled) {
this.payloadSigningEnabled = payloadSigningEnabled;
}

protected AmazonS3ClientBuilder createAmazonS3ClientBuilder() {
return AmazonS3ClientBuilder.standard()
.withPathStyleAccessEnabled(this.isPathStyleAccessEnabled())
.withPayloadSigningEnabled(this.isPayloadSigningEnabled());
protected S3ClientOptions createS3ClientOptions() {
S3ClientOptions options = new S3ClientOptions();
options.setPathStyleAccessEnabled(this.isPathStyleAccessEnabled());
options.setPayloadSigningEnabled(this.isPayloadSigningEnabled());
return options;
}

public static class S3ClientOptions implements Serializable {
private boolean pathStyleAccessEnabled = false;
private boolean payloadSigningEnabled = false;

public boolean isPathStyleAccessEnabled() {
return this.pathStyleAccessEnabled;
}

public void setPathStyleAccessEnabled(final boolean pathStyleAccessEnabled) {
this.pathStyleAccessEnabled = pathStyleAccessEnabled;
}

public boolean isPayloadSigningEnabled() {
return this.payloadSigningEnabled;
}

public void setPayloadSigningEnabled(final boolean payloadSigningEnabled) {
this.payloadSigningEnabled = payloadSigningEnabled;
}

protected AmazonS3ClientBuilder createAmazonS3ClientBuilder() {
return AmazonS3ClientBuilder.standard()
.withPathStyleAccessEnabled(this.isPathStyleAccessEnabled())
.withPayloadSigningEnabled(this.isPayloadSigningEnabled());
}
}

}
2 changes: 1 addition & 1 deletion src/main/java/de/taimos/pipeline/aws/S3DeleteStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public boolean start() throws Exception {
public void run() {
try {
Execution.this.listener.getLogger().format("Deleting s3://%s/%s%n", bucket, path);
AmazonS3 s3Client = AWSClientFactory.create(Execution.this.step.createAmazonS3ClientBuilder(), Execution.this.envVars);
AmazonS3 s3Client = AWSClientFactory.create(Execution.this.step.createS3ClientOptions().createAmazonS3ClientBuilder(), Execution.this.envVars);

if (!path.endsWith("/")) {
// See if the thing that we were given is a file.
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/de/taimos/pipeline/aws/S3DownloadStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import com.amazonaws.event.ProgressEvent;
import com.amazonaws.event.ProgressEventType;
import com.amazonaws.event.ProgressListener;
import com.amazonaws.services.s3.AmazonS3ClientBuilder;
import com.amazonaws.services.s3.transfer.Download;
import com.amazonaws.services.s3.transfer.MultipleFileDownload;
import com.amazonaws.services.s3.transfer.TransferManager;
Expand Down Expand Up @@ -147,7 +146,7 @@ public void run() {
return;
}
}
target.act(new RemoteDownloader(Execution.this.step.createAmazonS3ClientBuilder(), Execution.this.envVars, Execution.this.listener, bucket, path));
target.act(new RemoteDownloader(Execution.this.step.createS3ClientOptions(), Execution.this.envVars, Execution.this.listener, bucket, path));
Execution.this.listener.getLogger().println("Download complete");
Execution.this.getContext().onSuccess(null);
} catch (Exception e) {
Expand All @@ -169,14 +168,14 @@ private static class RemoteDownloader implements FilePath.FileCallable<Void> {

protected static final long serialVersionUID = 1L;

private final transient AmazonS3ClientBuilder amazonS3ClientBuilder;
private final S3ClientOptions amazonS3ClientOptions;
private final EnvVars envVars;
private final TaskListener taskListener;
private final String bucket;
private final String path;

RemoteDownloader(AmazonS3ClientBuilder amazonS3ClientBuilder, EnvVars envVars, TaskListener taskListener, String bucket, String path) {
this.amazonS3ClientBuilder = amazonS3ClientBuilder;
RemoteDownloader(S3ClientOptions amazonS3ClientOptions, EnvVars envVars, TaskListener taskListener, String bucket, String path) {
this.amazonS3ClientOptions = amazonS3ClientOptions;
this.envVars = envVars;
this.taskListener = taskListener;
this.bucket = bucket;
Expand All @@ -186,7 +185,7 @@ private static class RemoteDownloader implements FilePath.FileCallable<Void> {
@Override
public Void invoke(File localFile, VirtualChannel channel) throws IOException, InterruptedException {
TransferManager mgr = TransferManagerBuilder.standard()
.withS3Client(AWSClientFactory.create(this.amazonS3ClientBuilder, this.envVars))
.withS3Client(AWSClientFactory.create(this.amazonS3ClientOptions.createAmazonS3ClientBuilder(), this.envVars))
.build();

if (this.path == null || this.path.isEmpty() || this.path.endsWith("/")) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/taimos/pipeline/aws/S3FindFilesStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public FileWrapper[] run() throws Exception {

Execution.this.listener.getLogger().format("Searching s3://%s/%s for glob:'%s' %s%n", bucket, path, glob, onlyFiles ? "(only files)" : "");

AmazonS3 s3Client = AWSClientFactory.create(Execution.this.step.createAmazonS3ClientBuilder(), Execution.this.envVars);
AmazonS3 s3Client = AWSClientFactory.create(Execution.this.step.createS3ClientOptions().createAmazonS3ClientBuilder(), Execution.this.envVars);

// Construct a PatternMatcher to match the files.
// Essentially, we're going to match against "${path}/${glob}". Obviously,
Expand Down
21 changes: 10 additions & 11 deletions src/main/java/de/taimos/pipeline/aws/S3UploadStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import com.amazonaws.event.ProgressEvent;
import com.amazonaws.event.ProgressEventType;
import com.amazonaws.event.ProgressListener;
import com.amazonaws.services.s3.AmazonS3ClientBuilder;
import com.amazonaws.services.s3.Headers;
import com.amazonaws.services.s3.model.CannedAccessControlList;
import com.amazonaws.services.s3.model.ObjectMetadata;
Expand Down Expand Up @@ -247,7 +246,7 @@ public void run() {
return;
}

child.act(new RemoteUploader(Execution.this.step.createAmazonS3ClientBuilder(), Execution.this.envVars, Execution.this.listener, bucket, path, metadatas, acl, cacheControl));
child.act(new RemoteUploader(Execution.this.step.createS3ClientOptions(), Execution.this.envVars, Execution.this.listener, bucket, path, metadatas, acl, cacheControl));

Execution.this.listener.getLogger().println("Upload complete");
Execution.this.getContext().onSuccess(null);
Expand All @@ -257,7 +256,7 @@ public void run() {
for (FilePath child : children) {
child.act(new FeedList(fileList));
}
dir.act(new RemoteListUploader(Execution.this.step.createAmazonS3ClientBuilder(), Execution.this.envVars, Execution.this.listener, fileList, bucket, path, metadatas, acl, cacheControl));
dir.act(new RemoteListUploader(Execution.this.step.createS3ClientOptions(), Execution.this.envVars, Execution.this.listener, fileList, bucket, path, metadatas, acl, cacheControl));
Execution.this.listener.getLogger().println("Upload complete");
Execution.this.getContext().onSuccess(null);
}
Expand All @@ -279,7 +278,7 @@ public void stop(@Nonnull Throwable cause) throws Exception {
private static class RemoteUploader implements FilePath.FileCallable<Void> {

protected static final long serialVersionUID = 1L;
private final transient AmazonS3ClientBuilder amazonS3ClientBuilder;
private final S3ClientOptions amazonS3ClientOptions;
private final EnvVars envVars;
private final TaskListener taskListener;
private final String bucket;
Expand All @@ -288,8 +287,8 @@ private static class RemoteUploader implements FilePath.FileCallable<Void> {
private final CannedAccessControlList acl;
private final String cacheControl;

RemoteUploader(AmazonS3ClientBuilder amazonS3ClientBuilder, EnvVars envVars, TaskListener taskListener, String bucket, String path, Map<String, String> metadatas, CannedAccessControlList acl, String cacheControl) {
this.amazonS3ClientBuilder = amazonS3ClientBuilder;
RemoteUploader(S3ClientOptions amazonS3ClientOptions, EnvVars envVars, TaskListener taskListener, String bucket, String path, Map<String, String> metadatas, CannedAccessControlList acl, String cacheControl) {
this.amazonS3ClientOptions = amazonS3ClientOptions;
this.envVars = envVars;
this.taskListener = taskListener;
this.bucket = bucket;
Expand All @@ -302,7 +301,7 @@ private static class RemoteUploader implements FilePath.FileCallable<Void> {
@Override
public Void invoke(File localFile, VirtualChannel channel) throws IOException, InterruptedException {
TransferManager mgr = TransferManagerBuilder.standard()
.withS3Client(AWSClientFactory.create(this.amazonS3ClientBuilder, this.envVars))
.withS3Client(AWSClientFactory.create(this.amazonS3ClientOptions.createAmazonS3ClientBuilder(), this.envVars))
.build();
if (localFile.isFile()) {
Preconditions.checkArgument(this.path != null && !this.path.isEmpty(), "Path must not be null or empty when uploading file");
Expand Down Expand Up @@ -384,7 +383,7 @@ public void checkRoles(RoleChecker roleChecker) throws SecurityException {
private static class RemoteListUploader implements FilePath.FileCallable<Void> {

protected static final long serialVersionUID = 1L;
private final transient AmazonS3ClientBuilder amazonS3ClientBuilder;
private final S3ClientOptions amazonS3ClientOptions;
private final EnvVars envVars;
private final TaskListener taskListener;
private final String bucket;
Expand All @@ -394,8 +393,8 @@ private static class RemoteListUploader implements FilePath.FileCallable<Void> {
private final CannedAccessControlList acl;
private final String cacheControl;

RemoteListUploader(AmazonS3ClientBuilder amazonS3ClientBuilder, EnvVars envVars, TaskListener taskListener, List<File> fileList, String bucket, String path, Map<String, String> metadatas, CannedAccessControlList acl, final String cacheControl) {
this.amazonS3ClientBuilder = amazonS3ClientBuilder;
RemoteListUploader(S3ClientOptions amazonS3ClientOptions, EnvVars envVars, TaskListener taskListener, List<File> fileList, String bucket, String path, Map<String, String> metadatas, CannedAccessControlList acl, final String cacheControl) {
this.amazonS3ClientOptions = amazonS3ClientOptions;
this.envVars = envVars;
this.taskListener = taskListener;
this.fileList = fileList;
Expand All @@ -409,7 +408,7 @@ private static class RemoteListUploader implements FilePath.FileCallable<Void> {
@Override
public Void invoke(File localFile, VirtualChannel channel) throws IOException, InterruptedException {
TransferManager mgr = TransferManagerBuilder.standard()
.withS3Client(AWSClientFactory.create(this.amazonS3ClientBuilder, this.envVars))
.withS3Client(AWSClientFactory.create(this.amazonS3ClientOptions.createAmazonS3ClientBuilder(), this.envVars))
.build();
Preconditions.checkArgument(this.path != null && !this.path.isEmpty(), "Path must not be null or empty when uploading file");
final MultipleFileUpload fileUpload;
Expand Down
5 changes: 3 additions & 2 deletions src/test/java/de/taimos/pipeline/aws/AbstractS3StepTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@

package de.taimos.pipeline.aws;

import com.amazonaws.services.s3.AmazonS3ClientBuilder;
import org.junit.Assert;
import org.junit.Test;

import com.amazonaws.services.s3.AmazonS3ClientBuilder;

public class AbstractS3StepTest {
@Test
public void gettersWorkAsExpected() throws Exception {
S3DeleteStep step = new S3DeleteStep( "my-bucket", "my-path" , true, true);
final AmazonS3ClientBuilder amazonS3ClientBuilder = step.createAmazonS3ClientBuilder();
final AmazonS3ClientBuilder amazonS3ClientBuilder = step.createS3ClientOptions().createAmazonS3ClientBuilder();
Assert.assertEquals( true, amazonS3ClientBuilder.isPathStyleAccessEnabled() );
Assert.assertEquals( true, amazonS3ClientBuilder.isPayloadSigningEnabled() );
}
Expand Down

0 comments on commit 01c0341

Please sign in to comment.