From 036e85bde6da91fa1bf1a2059bff2d21832e81fc Mon Sep 17 00:00:00 2001 From: Sriram Date: Fri, 19 Jan 2018 17:35:25 +0530 Subject: [PATCH] Handle corner case in ArtifactStore getLatest, which cascades to MaterialPoller In our setup, we've noticed that objectMetadata request after doing a getLatest fails with 404. One possible code path that leads to this is getLatest returning Revisions.base(), which is problematic, since, we consider it to be a valid revision and try to do a metadata fetch. Added a fix to handle it with logging enabled. --- .../plugin/S3PackageMaterialPoller.java | 5 +++ .../plugin/S3PackageMaterialPollerSpec.scala | 34 ++++++++++++++++++- .../indix/gocd/s3publish/PublishExecutor.java | 2 ++ .../indix/gocd/models/BadRevisionStatus.java | 33 ++++++++++++++++++ .../java/com/indix/gocd/models/Revision.java | 2 ++ .../com/indix/gocd/models/RevisionStatus.java | 1 + .../gocd/utils/store/S3ArtifactStore.java | 8 ++--- .../gocd/utils/store/S3ArtifactStoreTest.java | 16 +++++++++ 8 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 utils/src/main/java/com/indix/gocd/models/BadRevisionStatus.java diff --git a/material/src/main/java/com/indix/gocd/s3material/plugin/S3PackageMaterialPoller.java b/material/src/main/java/com/indix/gocd/s3material/plugin/S3PackageMaterialPoller.java index 19bc6e7..782a595 100644 --- a/material/src/main/java/com/indix/gocd/s3material/plugin/S3PackageMaterialPoller.java +++ b/material/src/main/java/com/indix/gocd/s3material/plugin/S3PackageMaterialPoller.java @@ -76,6 +76,11 @@ private GoPluginApiResponse handleLatestRevisionSince(GoPluginApiRequest goPlugi S3ArtifactStore artifactStore = s3ArtifactStore(s3Bucket); try { RevisionStatus revision = artifactStore.getLatest(artifact(packageKeyValuePairs)); + if (!revision.isValid) { + logger.warn("Received invalid revision from artifact store with this message: "+revision.toString()); + return createResponse(DefaultGoPluginApiResponse.INTERNAL_ERROR, revision.toMap()); + } + if(new Revision(revision.revision.getRevision()).compareTo(new Revision(previousRevision)) > 0) { return createResponse(DefaultGoPluginApiResponse.SUCCESS_RESPONSE_CODE, revision.toMap()); } diff --git a/material/src/test/scala/com/indix/gocd/s3material/plugin/S3PackageMaterialPollerSpec.scala b/material/src/test/scala/com/indix/gocd/s3material/plugin/S3PackageMaterialPollerSpec.scala index 26a79e7..eafd8c6 100644 --- a/material/src/test/scala/com/indix/gocd/s3material/plugin/S3PackageMaterialPollerSpec.scala +++ b/material/src/test/scala/com/indix/gocd/s3material/plugin/S3PackageMaterialPollerSpec.scala @@ -6,7 +6,7 @@ import java.util.Date import com.amazonaws.services.s3.AmazonS3Client import com.google.gson.GsonBuilder -import com.indix.gocd.models.{Artifact, Revision, RevisionStatus} +import com.indix.gocd.models.{Artifact, BadRevisionStatus, Revision, RevisionStatus} import com.indix.gocd.utils.store.S3ArtifactStore import com.thoughtworks.go.plugin.api.request.GoPluginApiRequest import org.mockito.Matchers @@ -71,6 +71,38 @@ class S3PackageMaterialPollerSpec extends FlatSpec with MockitoSugar with org.sc assert(resultMap == null) } + it should "handle BadRevisionStatus response from artifact store appropriately" in { + val status = new BadRevisionStatus(new Artifact("Pipeline", "Stage", "Job", new Revision("1.1")), "No valid revision found for this artifact") + doReturn(status).when(mockS3ArtifactStore).getLatest(Matchers.any[Artifact]) + val result = sut.handle(getRequest(S3PackageMaterialPoller.REQUEST_LATEST_REVISION_SINCE, """ + |{ + | "repository-configuration": { + | "S3_BUCKET": { + | "value": "S3 Bucket" + | } + | }, + | "package-configuration": { + | "PIPELINE_NAME": { + | "value": "Pipeline" + | }, + | "STAGE_NAME": { + | "value": "Stage" + | }, + | "JOB_NAME": { + | "value": "Job" + | } + | }, + | "previous-revision": { + | "revision": "1.1", + | "timestamp": "" + | } + |} + """.stripMargin)) + val resultMap = new GsonBuilder().create.fromJson[AnyRef](result.responseBody(), classOf[Any]) + assert(result.responseCode() == 200) + assert(resultMap == null) + } + 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[Artifact]) diff --git a/publish/src/main/java/com/indix/gocd/s3publish/PublishExecutor.java b/publish/src/main/java/com/indix/gocd/s3publish/PublishExecutor.java index e603428..0e7407b 100644 --- a/publish/src/main/java/com/indix/gocd/s3publish/PublishExecutor.java +++ b/publish/src/main/java/com/indix/gocd/s3publish/PublishExecutor.java @@ -55,6 +55,8 @@ public TaskExecutionResult execute(Config config, final Context context) { } } + // A configured destination prefix is used to deploy files rather than publish artifacts + // We want to set metadata only when publishing artifacts if(!hasConfigDestinationPrefix(config)) { setMetadata(env, bucket, destinationPrefix, store); } diff --git a/utils/src/main/java/com/indix/gocd/models/BadRevisionStatus.java b/utils/src/main/java/com/indix/gocd/models/BadRevisionStatus.java new file mode 100644 index 0000000..3a16787 --- /dev/null +++ b/utils/src/main/java/com/indix/gocd/models/BadRevisionStatus.java @@ -0,0 +1,33 @@ +package com.indix.gocd.models; + +import java.util.HashMap; +import java.util.Map; + +public class BadRevisionStatus extends RevisionStatus { + public Artifact artifact = null; + public Boolean isValid = false; + public String msg = null; + public BadRevisionStatus(Artifact artifact, String msg) { + super(artifact.revision, null, null, null, null); + this.artifact = artifact; + this.msg = msg; + this.isValid = false; + } + + @Override + public String toString() { + return "BadRevisionStatus{" + + "artifact=" + artifact.prefixWithRevision() + + ", msg=" + msg + + '}'; + } + + @Override + public Map toMap() { + final HashMap result = new HashMap(); + result.put("artifact", artifact.prefixWithRevision()); + result.put("msg", this.msg); + + return result; + } +} diff --git a/utils/src/main/java/com/indix/gocd/models/Revision.java b/utils/src/main/java/com/indix/gocd/models/Revision.java index 9b2829c..e74ea75 100644 --- a/utils/src/main/java/com/indix/gocd/models/Revision.java +++ b/utils/src/main/java/com/indix/gocd/models/Revision.java @@ -1,5 +1,7 @@ package com.indix.gocd.models; +import java.util.Arrays; + public class Revision implements Comparable { private String revision; private String[] parts; diff --git a/utils/src/main/java/com/indix/gocd/models/RevisionStatus.java b/utils/src/main/java/com/indix/gocd/models/RevisionStatus.java index 4fd38a7..5036865 100644 --- a/utils/src/main/java/com/indix/gocd/models/RevisionStatus.java +++ b/utils/src/main/java/com/indix/gocd/models/RevisionStatus.java @@ -13,6 +13,7 @@ public class RevisionStatus { public String tracebackUrl; public String user; public String revisionLabel; + public Boolean isValid = true; private static final String DATE_PATTERN = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"; public RevisionStatus(Revision revision, Date lastModified, String tracebackUrl, String user) { diff --git a/utils/src/main/java/com/indix/gocd/utils/store/S3ArtifactStore.java b/utils/src/main/java/com/indix/gocd/utils/store/S3ArtifactStore.java index ac8bbd7..35b35a8 100644 --- a/utils/src/main/java/com/indix/gocd/utils/store/S3ArtifactStore.java +++ b/utils/src/main/java/com/indix/gocd/utils/store/S3ArtifactStore.java @@ -7,10 +7,7 @@ import com.amazonaws.services.s3.AmazonS3Client; import com.amazonaws.services.s3.AmazonS3ClientBuilder; import com.amazonaws.services.s3.model.*; -import com.indix.gocd.models.Artifact; -import com.indix.gocd.models.ResponseMetadataConstants; -import com.indix.gocd.models.Revision; -import com.indix.gocd.models.RevisionStatus; +import com.indix.gocd.models.*; import com.indix.gocd.utils.GoEnvironment; import com.indix.gocd.utils.utils.Function; import com.indix.gocd.utils.utils.Functions; @@ -183,6 +180,9 @@ public RevisionStatus getLatest(Artifact artifact) { ObjectListing listing = client.listObjects(listObjectsRequest); if (listing != null) { Revision recent = latestOf(listing); + if (recent.compareTo(Revision.base()) == 0) { + return new BadRevisionStatus(artifact, "No artifacts found which are later than this"); + } Artifact artifactWithRevision = artifact.withRevision(recent); GetObjectMetadataRequest objectMetadataRequest = new GetObjectMetadataRequest(bucket, artifactWithRevision.prefixWithRevision()); ObjectMetadata metadata = client.getObjectMetadata(objectMetadataRequest); diff --git a/utils/src/test/java/com/indix/gocd/utils/store/S3ArtifactStoreTest.java b/utils/src/test/java/com/indix/gocd/utils/store/S3ArtifactStoreTest.java index c85e5ca..d4833e3 100644 --- a/utils/src/test/java/com/indix/gocd/utils/store/S3ArtifactStoreTest.java +++ b/utils/src/test/java/com/indix/gocd/utils/store/S3ArtifactStoreTest.java @@ -4,6 +4,9 @@ import com.amazonaws.services.s3.model.ListObjectsRequest; import com.amazonaws.services.s3.model.ObjectListing; import com.amazonaws.services.s3.model.PutObjectRequest; +import com.indix.gocd.models.Artifact; +import com.indix.gocd.models.BadRevisionStatus; +import com.indix.gocd.models.RevisionStatus; import com.indix.gocd.utils.GoEnvironment; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -20,6 +23,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.*; @@ -114,6 +118,18 @@ public void shouldReturnNullWhenObjectListingIsSize0() { assertNull(prefix); } + @Test + public void shouldReturnBadRevisionStatusOnGetLatest() { + ObjectListing listing = new ObjectListing(); + doReturn(listing).when(mockClient).listObjects(any(ListObjectsRequest.class)); + S3ArtifactStore store = new S3ArtifactStore(mockClient, "foo-bar"); + + RevisionStatus status = store.getLatest(new Artifact("pipeline", "stage", "job")); + assertNotNull(status); + assertEquals(status instanceof BadRevisionStatus, true); + } + + @Test public void shouldReturnTheLatestStageCounter() { ObjectListing listing = new ObjectListing();