From db05a9b00726b32e4d9b8fd082a4865809296522 Mon Sep 17 00:00:00 2001
From: Eric Zimanyi <ezimanyi@google.com>
Date: Thu, 20 Jun 2019 17:30:25 -0400
Subject: [PATCH] fix(gcb): Correctly fetch artifact manifest when it contains
 a version (#468)

The logic to fetch an artifact manifest does not properly account for
the GCS path containing a version (as in gs://bucket/file.json#123).

This isn't causing any active issues as GCB creates a file named
with a UUID and does not include the version in the build output,
but we should properly handle this in case this changes (or in
case we re-use the artifact fetching code elsewhere).
---
 .../gcb/GoogleCloudBuildArtifactFetcher.java  |  4 +++-
 .../igor/gcb/GoogleCloudBuildClient.java      |  9 ++++++--
 ...GoogleCloudBuildArtifactFetcherSpec.groovy | 23 +++++++++++++++----
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/igor-web/src/main/java/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildArtifactFetcher.java b/igor-web/src/main/java/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildArtifactFetcher.java
index 458f22c63..2e245b26a 100644
--- a/igor-web/src/main/java/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildArtifactFetcher.java
+++ b/igor-web/src/main/java/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildArtifactFetcher.java
@@ -115,7 +115,9 @@ private GoogleCloudStorageObject getGoogleCloudStorageManifest(Build build) {
   private List<GoogleCloudBuildArtifact> readGoogleCloudStorageManifest(
       GoogleCloudStorageObject manifest) throws IOException {
     List<GoogleCloudBuildArtifact> results = new ArrayList<>();
-    InputStream is = client.fetchStorageObject(manifest.getBucket(), manifest.getObject());
+    InputStream is =
+        client.fetchStorageObject(
+            manifest.getBucket(), manifest.getObject(), manifest.getVersion());
     try (BufferedReader reader = new BufferedReader(new InputStreamReader(is))) {
       String line;
       while ((line = reader.readLine()) != null) {
diff --git a/igor-web/src/main/java/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildClient.java b/igor-web/src/main/java/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildClient.java
index d76e7f055..ea79728d9 100644
--- a/igor-web/src/main/java/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildClient.java
+++ b/igor-web/src/main/java/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildClient.java
@@ -58,7 +58,12 @@ public Build getBuild(String buildId) {
     return executor.execute(() -> cloudBuild.projects().builds().get(projectId, buildId));
   }
 
-  public InputStream fetchStorageObject(String bucket, String object) throws IOException {
-    return cloudStorage.objects().get(bucket, object).executeMediaAsInputStream();
+  public InputStream fetchStorageObject(String bucket, String object, Long version)
+      throws IOException {
+    Storage.Objects.Get getRequest = cloudStorage.objects().get(bucket, object);
+    if (version != null) {
+      getRequest.setGeneration(version);
+    }
+    return getRequest.executeMediaAsInputStream();
   }
 }
diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildArtifactFetcherSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildArtifactFetcherSpec.groovy
index 42c775624..d20f1240a 100644
--- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildArtifactFetcherSpec.groovy
+++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildArtifactFetcherSpec.groovy
@@ -98,7 +98,22 @@ class GoogleCloudBuildArtifactFetcherSpec extends Specification {
     def artifacts = artifactFetcher.getArtifacts(build)
 
     then:
-    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT) >> new ByteArrayInputStream()
+    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT, null) >> new ByteArrayInputStream()
+
+    artifacts.size() == 0
+  }
+
+  def "correctly fetches a manifest qualified with an object generation"() {
+    given:
+    Results results = new Results()
+    results.setArtifactManifest(MANIFEST_PATH + "#123")
+    Build build = new Build().setResults(results)
+
+    when:
+    def artifacts = artifactFetcher.getArtifacts(build)
+
+    then:
+    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT, 123) >> new ByteArrayInputStream()
 
     artifacts.size() == 0
   }
@@ -112,7 +127,7 @@ class GoogleCloudBuildArtifactFetcherSpec extends Specification {
     def artifacts = artifactFetcher.getArtifacts(build)
 
     then:
-    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT) >> new ByteArrayInputStream(getManifest(gcsObjects).getBytes())
+    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT, null) >> new ByteArrayInputStream(getManifest(gcsObjects).getBytes())
 
     artifacts.size() == 1
     artifacts[0].name == "gs://artifact-bucket/test.out"
@@ -131,7 +146,7 @@ class GoogleCloudBuildArtifactFetcherSpec extends Specification {
     def artifacts = artifactFetcher.getArtifacts(build)
 
     then:
-    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT) >> new ByteArrayInputStream(getManifest(gcsObjects).getBytes())
+    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT, null) >> new ByteArrayInputStream(getManifest(gcsObjects).getBytes())
 
     artifacts.size() == 2
     artifacts[0].name == "gs://artifact-bucket/test.out"
@@ -157,7 +172,7 @@ class GoogleCloudBuildArtifactFetcherSpec extends Specification {
     def artifacts = artifactFetcher.getArtifacts(build)
 
     then:
-    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT) >> new ByteArrayInputStream(getManifest(gcsObjects).getBytes())
+    client.fetchStorageObject(MANIFEST_BUCKET, MANIFEST_OBJECT, null) >> new ByteArrayInputStream(getManifest(gcsObjects).getBytes())
 
     artifacts.size() == 2