Skip to content

Commit

Permalink
[#186151481] added support for range lookups with range header (#12)
Browse files Browse the repository at this point in the history 
* [#186151481] added support for range lookups with range header

* [#186151481] addressed code review comments
  • Loading branch information
@patmagee
patmagee authored Oct 4, 2023
1 parent a43c1d0 commit ef5f09e
Show file tree
Hide file tree
Showing 13 changed files with 415 additions and 108 deletions.
85 changes: 84 additions & 1 deletion e2e-tests/src/main/java/com/dnastack/wes/service/WesE2ETest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.google.auth.oauth2.GoogleCredentials;
import io.restassured.builder.MultiPartSpecBuilder;
import io.restassured.http.ContentType;
import io.restassured.http.Header;
import io.restassured.specification.MultiPartSpecification;
import org.awaitility.core.ConditionFactory;
import org.junit.jupiter.api.*;
Expand Down Expand Up @@ -605,7 +606,6 @@ public void setup() throws Exception {
.jsonPath()
.getString("run_id");
//@formatter:on
final String runPathStatus = format("%s/%s/status", path, workflowJobId);
pollUntilJobCompletes(workflowJobId);
}

Expand Down Expand Up @@ -649,6 +649,43 @@ public void getStdoutForTaskReturnsSuccessfully() {

Assertions.assertEquals("Hello Frank\n",body);

// test range offset
body = given()
.log().uri()
.log().method()
.header("Range","bytes=0-4")
.header(getHeader(getResource(path)))
.get(taskLogs.get("stdout"))
.then()
.statusCode(206)
.extract().asString();

Assertions.assertEquals("Hello",body);

body = given()
.log().uri()
.log().method()
.header("Range","bytes=6-")
.header(getHeader(getResource(path)))
.get(taskLogs.get("stdout"))
.then()
.statusCode(206)
.extract().asString();

Assertions.assertEquals("Frank\n",body);

body = given()
.log().uri()
.log().method()
.header("Range","bytes=1-3")
.header(getHeader(getResource(path)))
.get(taskLogs.get("stdout"))
.then()
.statusCode(206)
.extract().asString();

Assertions.assertEquals("ell",body);

//@formatter:on

//@formatter:off
Expand All @@ -663,6 +700,52 @@ public void getStdoutForTaskReturnsSuccessfully() {

Assertions.assertEquals("Goodbye Frank\n",body);
//@formatter:on

// test range offset
body = given()
.log().uri()
.log().method()
.header("Range","bytes=0-4")
.header(getHeader(getResource(path)))
.get(taskLogs.get("stderr"))
.then()
.statusCode(206)
.extract().asString();

Assertions.assertEquals("Goodb",body);

body = given()
.log().uri()
.log().method()
.header("Range","bytes=8-")
.header(getHeader(getResource(path)))
.get(taskLogs.get("stderr"))
.then()
.statusCode(206)
.extract().asString();

Assertions.assertEquals("Frank\n",body);

body = given()
.log().uri()
.log().method()
.header("Range","bytes=1-3")
.header(getHeader(getResource(path)))
.get(taskLogs.get("stderr"))
.then()
.statusCode(206)
.extract().asString();

Assertions.assertEquals("ood",body);

given()
.log().uri()
.log().method()
.header("Range","bytes=1-3,6-9")
.header(getHeader(getResource(path)))
.get(taskLogs.get("stderr"))
.then()
.statusCode(416);
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion nginx/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ following will provide certificates for hostname `127.0.0.1`.
cd cert

# Generate server cert to be signed
openssl req -new -nodes -x509 -days 365 -keyout server.key -out server.crt -config server.conf
openssl req -new -nodes -x509 -days 365 -keyout server.pem -out server.crt -config server.conf

# Generate client cert to be signed
openssl req -new -nodes -x509 -days 365 -keyout client.key -out client.crt -config client.conf
Expand Down
35 changes: 27 additions & 8 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>2.7.12</version>
<version>2.7.15</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>

Expand All @@ -28,21 +28,23 @@
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
<maven-surefire-plugin.version>3.0.0-M5</maven-surefire-plugin.version>
<spring-cloud.version>2021.0.5</spring-cloud.version>
<spring-cloud.version>2021.0.8</spring-cloud.version>
<logback.version>1.2.10</logback.version>
<logback-extensions.version>1.0.0</logback-extensions.version>
<logback.contrib.version>0.1.5</logback.contrib.version>
<springdoc.version>1.6.13</springdoc.version>
<dnastack-token-validator.version>1.0.8</dnastack-token-validator.version>
<audit-event-logger.version>1.0.9</audit-event-logger.version>
<dnastack-token-validator.version>1.0.9</dnastack-token-validator.version>
<audit-event-logger.version>1.0.15</audit-event-logger.version>
<okhttpclient.version>4.9.3</okhttpclient.version>
<jwt.version>0.11.2</jwt.version>
<feign-form.version>3.8.0</feign-form.version>
<azurestorage.version>12.9.0</azurestorage.version>
<sonar.dependencyCheck.jsonReportPath>target/dependency-check-report.json</sonar.dependencyCheck.jsonReportPath>
<sonar.dependencyCheck.htmlReportPath>target/dependency-check-report.html</sonar.dependencyCheck.htmlReportPath>
<sonar.dependencyCheck.summarize>true</sonar.dependencyCheck.summarize>
<gcloud.version>9.1.0</gcloud.version>
<azure-sdk.version>1.2.8</azure-sdk.version>
<gcloud.storage.version>2.20.1</gcloud.storage.version>
<gcloud.auth.version>1.16.0</gcloud.auth.version>
</properties>

<dependencyManagement>
Expand Down Expand Up @@ -71,10 +73,24 @@
<artifactId>logback-core</artifactId>
<version>${logback.version}</version>
</dependency>
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-sdk-bom</artifactId>
<version>${azure-sdk.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>libraries-bom</artifactId>
<version>${gcloud.version}</version>
<artifactId>google-cloud-storage-bom</artifactId>
<version>${gcloud.storage.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>com.google.auth</groupId>
<artifactId>google-auth-library-bom</artifactId>
<version>${gcloud.auth.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand Down Expand Up @@ -198,12 +214,15 @@
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-storage-blob</artifactId>
<version>${azurestorage.version}</version>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-storage</artifactId>
</dependency>
<dependency>
<groupId>com.google.auth</groupId>
<artifactId>google-auth-library-oauth2-http</artifactId>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/com/dnastack/wes/api/RangeNotSatisfiableException.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.dnastack.wes.api;

public class RangeNotSatisfiableException extends RuntimeException {

public RangeNotSatisfiableException(String s) {
super(s);
}

}
60 changes: 49 additions & 11 deletions src/main/java/com/dnastack/wes/api/WesV1Controller.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@


import com.dnastack.audit.aspect.AuditActionUri;
import com.dnastack.audit.aspect.AuditIgnore;
import com.dnastack.audit.util.AuditIgnore;
import com.dnastack.wes.AppConfig;
import com.dnastack.wes.cromwell.CromwellService;
import com.dnastack.wes.security.AuthenticatedUser;
import com.dnastack.wes.workflow.WorkflowAuthorizerService;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpRange;
import org.springframework.http.MediaType;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.*;
Expand All @@ -18,6 +20,7 @@
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;


Expand Down Expand Up @@ -123,36 +126,71 @@ public RunId cancelRun(@PathVariable("runId") String runId) {
@AuditActionUri("wes:run:stderr")
@PreAuthorize("@accessEvaluator.canAccessResource('/ga4gh/wes/v1/runs/' + #runId, 'wes:runs:read', 'wes')")
@GetMapping(value = "/runs/{runId}/logs/stderr", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE)
public void getStderr(HttpServletResponse response, @PathVariable String runId) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId);
public void getStderr(HttpServletResponse response, @RequestHeader HttpHeaders headers, @PathVariable String runId) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId,getRangeFromHeaders(response,headers));
}

@AuditActionUri("wes:run:stderr")
@PreAuthorize("@accessEvaluator.canAccessResource('/ga4gh/wes/v1/runs/' + #runId, 'wes:runs:read', 'wes')")
@GetMapping(value = "/runs/{runId}/logs/task/{taskId}/stderr", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE)
public void getStderr(HttpServletResponse response, @PathVariable String runId, @PathVariable String taskId) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId, taskId, "stderr");
public void getTaskStderr(
HttpServletResponse response,
@RequestHeader HttpHeaders headers,
@PathVariable String runId,
@PathVariable String taskId
) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId, taskId, "stderr",getRangeFromHeaders(response,headers));
}

@AuditActionUri("wes:run:stdout")
@PreAuthorize("@accessEvaluator.canAccessResource('/ga4gh/wes/v1/runs/' + #runId, 'wes:runs:read', 'wes')")
@GetMapping(value = "/runs/{runId}/logs/task/{taskId}/stdout", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE)
public void getStdout(HttpServletResponse response, @PathVariable String runId, @PathVariable String taskId) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId, taskId, "stdout");
public void getTaskStdout(
HttpServletResponse response,
@RequestHeader HttpHeaders headers,
@PathVariable String runId,
@PathVariable String taskId
) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId, taskId, "stdout",getRangeFromHeaders(response,headers));
}

@AuditActionUri("wes:run:stderr")
@PreAuthorize("@accessEvaluator.canAccessResource('/ga4gh/wes/v1/runs/' + #runId, 'wes:runs:read', 'wes')")
@GetMapping(value = "/runs/{runId}/logs/task/{taskName}/{index}/stderr", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE)
public void getStderr(HttpServletResponse response, @PathVariable String runId, @PathVariable String taskName, @PathVariable int index) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId, taskName, index, "stderr");
public void getTaskStderr(
HttpServletResponse response,
@RequestHeader HttpHeaders headers,
@PathVariable String runId,
@PathVariable String taskName,
@PathVariable int index
) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId, taskName, index, "stderr",getRangeFromHeaders(response,headers));
}

@AuditActionUri("wes:run:stdout")
@PreAuthorize("@accessEvaluator.canAccessResource('/ga4gh/wes/v1/runs/' + #runId, 'wes:runs:read', 'wes')")
@GetMapping(value = "/runs/{runId}/logs/task/{taskName}/{index}/stdout", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE)
public void getStdout(HttpServletResponse response, @PathVariable String runId, @PathVariable String taskName, @PathVariable int index) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId, taskName, index, "stdout");
public void getTaskStdout(
HttpServletResponse response,
@RequestHeader HttpHeaders headers,
@PathVariable String runId,
@PathVariable String taskName,
@PathVariable int index
) throws IOException {
adapter.getLogBytes(response.getOutputStream(), runId, taskName, index, "stdout", getRangeFromHeaders(response, headers));
}

private HttpRange getRangeFromHeaders(HttpServletResponse response, HttpHeaders headers){
List<HttpRange> ranges = headers.getRange();
if (ranges.isEmpty()){
return null;
} else if (ranges.size() > 1) {
// only return the first range parsed
throw new RangeNotSatisfiableException("Streaming of multiple ranges is not supported");
} else {
response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT);
return ranges.get(0);
}
}

}
20 changes: 14 additions & 6 deletions src/main/java/com/dnastack/wes/cromwell/CromwellService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import feign.FeignException;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpRange;
import org.springframework.stereotype.Service;
import org.springframework.web.multipart.MultipartFile;

Expand Down Expand Up @@ -217,9 +218,10 @@ public RunId cancel(String runId) {
return RunId.builder().runId(runId).build();
}

public void getLogBytes(OutputStream outputStream, String runId, String taskId, String logKey) throws IOException {
public void getLogBytes(OutputStream outputStream, String runId, String taskId, String logKey, HttpRange range) throws IOException {
String logPath = getLogPath(runId, taskId, logKey);
storageClient.readBytes(outputStream, logPath, null, null);

storageClient.readBytes(outputStream, logPath, range);
}

private String getLogPath(String runId, String taskId, String logKey) throws IOException {
Expand All @@ -236,9 +238,9 @@ private String getLogPath(String runId, String taskId, String logKey) throws IOE
}
}

public void getLogBytes(OutputStream outputStream, String runId, String taskName, int index, String logKey) throws IOException {
public void getLogBytes(OutputStream outputStream, String runId, String taskName, int index, String logKey, HttpRange httpRange) throws IOException {
String logPath = getLogPath(runId, taskName, index, logKey);
storageClient.readBytes(outputStream, logPath, null, null);
storageClient.readBytes(outputStream, logPath, httpRange);
}

//legacy
Expand All @@ -258,10 +260,16 @@ private String getLogPath(String runId, String taskName, int index, String logKe
}
}

public void getLogBytes(OutputStream outputStream, String runId) throws IOException {
public void getLogBytes(OutputStream outputStream, String runId, HttpRange range) throws IOException {
CromwellMetadataResponse response = client.getMetadata(runId);
if (response.getFailures() != null) {
outputStream.write(mapper.writeValueAsBytes(response.getFailures()));

byte[] bytes = mapper.writeValueAsBytes(response.getFailures());
if (range != null) {
outputStream.write(Arrays.copyOfRange(bytes, (int) range.getRangeStart(bytes.length), (int) range.getRangeEnd(bytes.length) + 1));
} else {
outputStream.write(bytes);
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/main/java/com/dnastack/wes/shared/GlobalControllerExceptionHandler.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.dnastack.wes.shared;

import com.dnastack.wes.api.ErrorResponse;
import com.dnastack.wes.api.RangeNotSatisfiableException;
import com.dnastack.wes.workflow.UnauthorizedWorkflowException;
import feign.FeignException;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -39,4 +40,10 @@ public ResponseEntity<ErrorResponse> handle(FeignException ex) {
.body(ErrorResponse.builder().msg(ex.getMessage()).errorCode(ex.status()).build());
}

@ExceptionHandler(RangeNotSatisfiableException.class)
public ResponseEntity<ErrorResponse> handle(RangeNotSatisfiableException ex) {
return ResponseEntity.status(416)
.body(ErrorResponse.builder().msg(ex.getMessage()).errorCode(416).build());
}

}
Loading

0 comments on commit ef5f09e

Please sign in to comment.