Merge pull request #53 from indix/auth-changes

Refactor AWS auth across plugins and upgrade SDK

build.sbt

@@ -1,15 +1,15 @@
 val ant = "org.apache.ant" % "ant" % "1.9.4"
 val apacheCommons = "org.apache.commons" % "commons-lang3" % "3.1"
 val commonsIo = "commons-io" % "commons-io" % "1.3.2"
-val awsS3 = "com.amazonaws" % "aws-java-sdk-s3" % "1.10.26"
+val awsS3 = "com.amazonaws" % "aws-java-sdk-s3" % "1.11.127"
 val nscalaTime = "com.github.nscala-time" %% "nscala-time" % "2.4.0"
 val gson = "com.google.code.gson" % "gson" % "2.2.3"
 val goPluginLibrary = "cd.go.plugin" % "go-plugin-api" % "15.1.0" % Provided
 
-val junit = "junit" % "junit" % "4.10" % Test
-val junitInterface = "com.novocode" % "junit-interface" % "0.11" % Test
 val hamcrest = "org.hamcrest" % "hamcrest-all" % "1.3" % Test
-val mockito = "org.mockito" % "mockito-all" % "1.9.0" % Test
+val junit = "junit" % "junit" % "4.12" % Test
+val junitInterface = "com.novocode" % "junit-interface" % "0.11" % Test
+val mockito = "org.mockito" % "mockito-all" % "1.10.19" % Test
 val scalaTest = "org.scalatest" %% "scalatest" % "2.2.0" % Test
 
 val appVersion = sys.env.get("TRAVIS_TAG") orElse sys.env.get("BUILD_LABEL") getOrElse "1.0.0-SNAPSHOT"
@@ -25,7 +25,7 @@ lazy val commonSettings = Seq(
   scalaVersion := "2.10.4",
   unmanagedBase := file(".") / "lib",
   libraryDependencies ++= Seq(
-    apacheCommons, commonsIo, awsS3, goPluginLibrary, mockito
+    apacheCommons, commonsIo, awsS3, goPluginLibrary, gson
   ),
   variables in EditSource += ("version", appVersion),
   targetDirectory in EditSource <<= baseDirectory(_ / "target" / "transformed"),
@@ -40,7 +40,7 @@ lazy val utils = (project in file("utils")).
     crossPaths := false,
     autoScalaLibrary := false,
     libraryDependencies ++= Seq(
-      junit, junitInterface, hamcrest
+      junit, junitInterface, mockito
     ),
     javacOptions ++= Seq("-source", "1.7", "-target", "1.7")
   )
@@ -53,7 +53,7 @@ lazy val publish = (project in file("publish")).
     crossPaths := false,
     autoScalaLibrary := false,
     libraryDependencies ++= Seq(
-      gson, ant, junit, junitInterface, hamcrest
+      ant, junit, junitInterface, mockito
     ),
     javacOptions ++= Seq("-source", "1.7", "-target", "1.7")
   )
@@ -66,7 +66,7 @@ lazy val material = (project in file("material")).
     crossPaths := false,
     assemblyOption in assembly := (assemblyOption in assembly).value.copy(includeScala = false),
     libraryDependencies ++= Seq(
-      gson, scalaTest
+      scalaTest, mockito
     ),
     javacOptions ++= Seq("-source", "1.7", "-target", "1.7")
   )
@@ -79,7 +79,7 @@ lazy val fetch = (project in file("fetch")).
     crossPaths := false,
     autoScalaLibrary := false,
     libraryDependencies ++= Seq(
-      gson, junit, hamcrest
+      junit, mockito
     ),
     javacOptions ++= Seq("-source", "1.7", "-target", "1.7")
   )

fetch/src/main/java/com/indix/gocd/s3fetch/FetchExecutor.java

@@ -27,10 +27,10 @@ public TaskExecutionResult execute(Config config, final Context context) {
         if (env.isAbsent(GO_ARTIFACTS_S3_BUCKET)) return envNotFound(GO_ARTIFACTS_S3_BUCKET);
 
         try {
+            String artifactPathOnS3 = getArtifactsLocationTemplate(config, env);
             final String bucket = env.get(GO_ARTIFACTS_S3_BUCKET);
             final S3ArtifactStore store = getS3ArtifactStore(env, bucket);
 
-            String artifactPathOnS3 = getArtifactsLocationTemplate(config, env);
             context.printMessage(String.format("Getting artifacts from %s", store.pathString(artifactPathOnS3)));
             String destination = String.format("%s/%s", context.getWorkingDir(), config.getDestination());
             setupDestinationDirectory(destination);
@@ -43,8 +43,8 @@ public TaskExecutionResult execute(Config config, final Context context) {
         }
     }
 
-    public S3ArtifactStore getS3ArtifactStore(GoEnvironment env, String bucket) {
-        return new S3ArtifactStore(s3Client(env), bucket);
+    protected S3ArtifactStore getS3ArtifactStore(GoEnvironment env, String bucket) {
+        return new S3ArtifactStore(env, bucket);
     }
 
     private void setupDestinationDirectory(String destination) {
@@ -58,18 +58,7 @@ private void setupDestinationDirectory(String destination) {
         }
     }
 
-    public AmazonS3Client s3Client(GoEnvironment env) {
-        AmazonS3Client client;
-        if (env.hasAWSUseIamRole()) {
-            client = new AmazonS3Client(new InstanceProfileCredentialsProvider());
-        } else {
-            client = new AmazonS3Client(new BasicAWSCredentials(env.get(AWS_ACCESS_KEY_ID), env.get(AWS_SECRET_ACCESS_KEY)));
-        }
-        return client;
-    }
-
-
-    public String getArtifactsLocationTemplate(Config config, GoEnvironment env) {
+    private String getArtifactsLocationTemplate(Config config, GoEnvironment env) {
         String repoName = config.getRepo();
         String packageName = config.getPkg();
         logger.debug(String.format("S3 fetch config uses repoName=%s and packageName=%s", repoName, packageName));

fetch/src/test/java/com/indix/gocd/s3fetch/FetchExecutorTest.java

@@ -10,24 +10,19 @@
 import com.indix.gocd.utils.mocks.MockContext;
 import com.indix.gocd.utils.store.S3ArtifactStore;
 import com.indix.gocd.utils.utils.Maps;
-import com.thoughtworks.go.plugin.api.response.execution.ExecutionResult;
-import com.thoughtworks.go.plugin.api.task.TaskConfig;
-import com.thoughtworks.go.plugin.api.task.TaskExecutionContext;
 import org.junit.Before;
 import org.junit.Test;
 
-import java.util.HashMap;
 import java.util.Map;
 
 import static com.indix.gocd.utils.Constants.*;
-import static org.hamcrest.Matchers.is;
+import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.*;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.*;
 
 
 public class FetchExecutorTest {
-    private final String destination = "artifacts";
     private final String bucket = "gocd";
     Maps.MapBuilder<String, String> mockEnvironmentVariables;
     private FetchExecutor fetchExecutor;
@@ -72,6 +67,10 @@ public void shouldBeFailureIfFetchConfigNotValid() {
                 .with(Constants.PACKAGE, Maps.builder().with("value", "TESTPUBLISHS3ARTIFACTS").build())
                 .with(Constants.DESTINATION, Maps.builder().with("value", "artifacts").build())
                 .build());
+        AmazonS3Client mockClient = mockClient();
+        S3ArtifactStore store = new S3ArtifactStore(mockClient, bucket);
+        doReturn(store).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), eq(bucket));
+
         TaskExecutionResult result = fetchExecutor.execute(config, mockContext(mockVariables));
 
         assertFalse(result.isSuccessful());
@@ -82,8 +81,9 @@ public void shouldBeFailureIfFetchConfigNotValid() {
     public void shouldBeFailureIfUnableToFetchArtifacts() {
         Map<String, String> mockVariables = mockEnvironmentVariables.build();
         AmazonS3Client mockClient = mockClient();
+        S3ArtifactStore store = new S3ArtifactStore(mockClient, bucket);
         doThrow(new AmazonClientException("Exception message")).when(mockClient).listObjects(any(ListObjectsRequest.class));
-        doReturn(mockClient).when(fetchExecutor).s3Client(any(GoEnvironment.class));
+        doReturn(store).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), eq(bucket));
 
         TaskExecutionResult result = fetchExecutor.execute(config, mockContext(mockVariables));
 
@@ -95,7 +95,8 @@ public void shouldBeFailureIfUnableToFetchArtifacts() {
     public void shouldBeSuccessResultOnSuccessfulFetch() {
         Map<String, String> mockVariables = mockEnvironmentVariables.build();
         AmazonS3Client mockClient = mockClient();
-        doReturn(mockClient).when(fetchExecutor).s3Client(any(GoEnvironment.class));
+        S3ArtifactStore store = new S3ArtifactStore(mockClient, bucket);
+        doReturn(store).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), eq(bucket));
         S3ArtifactStore mockStore = mockStore();
 
         doReturn(mockStore).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), any(String.class));
@@ -116,11 +117,9 @@ public void shouldBeAbleToHandleTaskConfigEntriesWithDashesInTheName() {
                 .with("GO_PACKAGE_REPO_WITH_DASH_PACKAGE_WITH_DASH_STAGE_NAME", "defaultStage")
                 .with("GO_PACKAGE_REPO_WITH_DASH_PACKAGE_WITH_DASH_JOB_NAME", "defaultJob")
                 .build();
-        AmazonS3Client mockClient = mockClient();
-        doReturn(mockClient).when(fetchExecutor).s3Client(any(GoEnvironment.class));
-        S3ArtifactStore mockStore = mockStore();
+        S3ArtifactStore store = mockStore();
+        doReturn(store).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), eq(bucket));
 
-        doReturn(mockStore).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), any(String.class));
         config = new Config(Maps.builder()
                 .with(Constants.REPO, Maps.builder().with("value", "repo-with-dash").build())
                 .with(Constants.PACKAGE, Maps.builder().with("value", "package-with-dash").build())
@@ -130,7 +129,7 @@ public void shouldBeAbleToHandleTaskConfigEntriesWithDashesInTheName() {
 
         assertTrue(result.isSuccessful());
         assertThat(result.message(), is("Fetched all artifacts"));
-        verify(mockStore, times(1)).getPrefix("TestPublish/defaultStage/defaultJob/20.1", "here/artifacts");
+        verify(store, times(1)).getPrefix("TestPublish/defaultStage/defaultJob/20.1", "here/artifacts");
     }
 
     @Test
@@ -142,11 +141,9 @@ public void shouldBeAbleToHandleTaskConfigEntriesWithPeriodsInTheName() {
                 .with("GO_PACKAGE_REPO_WITH_PERIOD_PACKAGE_WITH_PERIOD_STAGE_NAME", "defaultStage")
                 .with("GO_PACKAGE_REPO_WITH_PERIOD_PACKAGE_WITH_PERIOD_JOB_NAME", "defaultJob")
                 .build();
-        AmazonS3Client mockClient = mockClient();
-        doReturn(mockClient).when(fetchExecutor).s3Client(any(GoEnvironment.class));
-        S3ArtifactStore mockStore = mockStore();
+        S3ArtifactStore store = mockStore();
+        doReturn(store).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), eq(bucket));
 
-        doReturn(mockStore).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), any(String.class));
         config = new Config(Maps.builder()
                 .with(Constants.REPO, Maps.builder().with("value", "repo-with.period").build())
                 .with(Constants.PACKAGE, Maps.builder().with("value", "package-with.period").build())
@@ -156,7 +153,7 @@ public void shouldBeAbleToHandleTaskConfigEntriesWithPeriodsInTheName() {
 
         assertTrue(result.isSuccessful());
         assertThat(result.message(), is("Fetched all artifacts"));
-        verify(mockStore, times(1)).getPrefix("TestPublish/defaultStage/defaultJob/20.1", "here/artifacts");
+        verify(store, times(1)).getPrefix("TestPublish/defaultStage/defaultJob/20.1", "here/artifacts");
     }
 
     @Test
@@ -168,11 +165,9 @@ public void shouldBeAbleToHandleTaskConfigEntriesWithSpecialCharacters() {
                 .with("GO_PACKAGE_REPO_WITH________________________________PACKAGE_WITH________________________________STAGE_NAME", "defaultStage")
                 .with("GO_PACKAGE_REPO_WITH________________________________PACKAGE_WITH________________________________JOB_NAME", "defaultJob")
                 .build();
-        AmazonS3Client mockClient = mockClient();
-        doReturn(mockClient).when(fetchExecutor).s3Client(any(GoEnvironment.class));
-        S3ArtifactStore mockStore = mockStore();
+        S3ArtifactStore store = mockStore();
+        doReturn(store).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), eq(bucket));
 
-        doReturn(mockStore).when(fetchExecutor).getS3ArtifactStore(any(GoEnvironment.class), any(String.class));
         config = new Config(Maps.builder()
                 .with(Constants.REPO, Maps.builder().with("value", "repo-with`~!@#$%^&*()-+=[{]}\\|;:'\",<.>/?").build())
                 .with(Constants.PACKAGE, Maps.builder().with("value", "package-with`~!@#$%^&*()-+=[{]}\\|;:'\",<.>/?").build())
@@ -182,7 +177,7 @@ public void shouldBeAbleToHandleTaskConfigEntriesWithSpecialCharacters() {
 
         assertTrue(result.isSuccessful());
         assertThat(result.message(), is("Fetched all artifacts"));
-        verify(mockStore, times(1)).getPrefix("TestPublish/defaultStage/defaultJob/20.1", "here/artifacts");
+        verify(store, times(1)).getPrefix("TestPublish/defaultStage/defaultJob/20.1", "here/artifacts");
     }
 
     private Context mockContext(final Map<String, String> environmentMap) {

material/src/main/java/com/indix/gocd/s3material/plugin/S3PackageMaterialPoller.java

@@ -75,7 +75,7 @@ private GoPluginApiResponse handleLatestRevisionSince(GoPluginApiRequest goPlugi
         String s3Bucket = repositoryKeyValuePairs.get(S3_BUCKET);
         S3ArtifactStore artifactStore = s3ArtifactStore(s3Bucket);
         try {
-            RevisionStatus revision = artifactStore.getLatest(s3Client(), artifact(packageKeyValuePairs));
+            RevisionStatus revision = artifactStore.getLatest(artifact(packageKeyValuePairs));
             if(new Revision(revision.revision.getRevision()).compareTo(new Revision(previousRevision)) > 0) {
                 return createResponse(DefaultGoPluginApiResponse.SUCCESS_RESPONSE_CODE, revision.toMap());
             }
@@ -93,7 +93,7 @@ private GoPluginApiResponse handleGetLatestRevision(GoPluginApiRequest goPluginA
         String s3Bucket = repositoryKeyValuePairs.get(S3_BUCKET);
         S3ArtifactStore artifactStore = s3ArtifactStore(s3Bucket);
         try {
-            RevisionStatus revision = artifactStore.getLatest(s3Client(), artifact(packageKeyValuePairs));
+            RevisionStatus revision = artifactStore.getLatest(artifact(packageKeyValuePairs));
             return createResponse(DefaultGoPluginApiResponse.SUCCESS_RESPONSE_CODE, revision.toMap());
         } catch (Exception e) {
             logger.error(e.getMessage(), e);
@@ -222,17 +222,7 @@ private Map<String, String> keyValuePairs(GoPluginApiRequest goPluginApiRequest,
     }
 
     public S3ArtifactStore s3ArtifactStore(String s3Bucket) {
-        return new S3ArtifactStore(s3Client(), s3Bucket);
-    }
-
-    private static AmazonS3Client s3Client() {
-        // The s3 client has a nice way to pick up the creds.
-        // It first checks the env to see if it contains the required key related variables/values
-        // If not, it checks the java system properties to see if it's set there(ideally via -D args)
-        // If not, it falls back to check ~/.aws/credentials file
-        // If not, finally, very insecure way, it tries to fetch from the internal metadata service that each
-        // instance comes with(if its exposed).
-        return new AmazonS3Client();
+        return new S3ArtifactStore(s3Bucket);
     }
 
     private Artifact artifact(Map<String, String> packageConfig) {

material/src/test/scala/com/indix/gocd/s3material/plugin/S3PackageMaterialPollerSpec.scala

@@ -41,7 +41,7 @@ class S3PackageMaterialPollerSpec extends FlatSpec with MockitoSugar with org.sc
 
   it should "return null result if no new revision since previous revision" in {
     val status = new RevisionStatus(new Revision("1.1"), new Date(), "", "", "")
-    doReturn(status).when(mockS3ArtifactStore).getLatest(Matchers.any[AmazonS3Client], Matchers.any[Artifact])
+    doReturn(status).when(mockS3ArtifactStore).getLatest(Matchers.any[Artifact])
     val result = sut.handle(getRequest(S3PackageMaterialPoller.REQUEST_LATEST_REVISION_SINCE, """
                   |{
                   |    "repository-configuration": {
@@ -73,7 +73,7 @@ class S3PackageMaterialPollerSpec extends FlatSpec with MockitoSugar with org.sc
 
   it should "get more latest revision since previous revision" in {
     val status = new RevisionStatus(new Revision("1.2"), new Date(), "", "", "")
-    doReturn(status).when(mockS3ArtifactStore).getLatest(Matchers.any[AmazonS3Client], Matchers.any[Artifact])
+    doReturn(status).when(mockS3ArtifactStore).getLatest(Matchers.any[Artifact])
     val result = sut.handle(getRequest(S3PackageMaterialPoller.REQUEST_LATEST_REVISION_SINCE, """
                   |{
                   |    "repository-configuration": {
@@ -105,7 +105,7 @@ class S3PackageMaterialPollerSpec extends FlatSpec with MockitoSugar with org.sc
 
   it should "get latest revision" in {
     val status = new RevisionStatus(new Revision("1.1"), new Date(), "", "", "")
-    doReturn(status).when(mockS3ArtifactStore).getLatest(Matchers.any[AmazonS3Client], Matchers.any[Artifact])
+    doReturn(status).when(mockS3ArtifactStore).getLatest(Matchers.any[Artifact])
     val result = sut.handle(getRequest(S3PackageMaterialPoller.REQUEST_LATEST_REVISION, """
                   |{
                   |    "repository-configuration": {

publish/src/main/java/com/indix/gocd/s3publish/Config.java

@@ -1,17 +1,16 @@
 package com.indix.gocd.s3publish;
 
-import com.amazonaws.util.json.JSONArray;
-import com.amazonaws.util.json.JSONException;
-import com.amazonaws.util.json.JSONObject;
-import com.indix.gocd.utils.utils.Tuple2;
+import com.google.gson.GsonBuilder;
+import com.google.gson.JsonSyntaxException;
+import com.google.gson.reflect.TypeToken;
 
+import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
 import static com.indix.gocd.utils.Constants.DESTINATION_PREFIX;
 import static com.indix.gocd.utils.Constants.SOURCEDESTINATIONS;
-import static org.apache.commons.lang3.StringUtils.trim;
 
 public class Config {
 
@@ -24,17 +23,9 @@ public Config(Map config) {
         destinationPrefix  = getValue(config, DESTINATION_PREFIX);
     }
 
-    public List<Tuple2<String, String>> sourceDestinations() throws JSONException {
-        JSONArray sourceDestinations = new JSONArray(sourceDestinationsJson);
-        List<Tuple2<String, String>> result = new ArrayList<>();
-        for(int i =0; i < sourceDestinations.length(); i++) {
-            JSONObject sourceDestination = (JSONObject)sourceDestinations.get(i);
-            String source = trim(sourceDestination.getString("source"));
-            String destination = trim(sourceDestination.getString("destination"));
-            result.add(new Tuple2<>(source, destination));
-        }
-
-        return result;
+    public List<SourceDestination> sourceDestinations() throws JsonSyntaxException {
+        Type type = new TypeToken<ArrayList<SourceDestination>>() {}.getType();
+        return new GsonBuilder().create().fromJson(sourceDestinationsJson, type);
     }
 
     private String getValue(Map config, String property) {