Skip to content

Commit

Permalink
fix(gcb): Correctly fetch artifact manifest when it contains a version (
Browse files Browse the repository at this point in the history
#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).
  • Loading branch information
ezimanyi authored and maggieneterval committed Jun 20, 2019
1 parent eddcdc9 commit db05a9b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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

Expand Down

0 comments on commit db05a9b

Please sign in to comment.