Skip to content

Commit

Permalink
Introduce disable direct download feature in CLI. (#1275)
Browse files Browse the repository at this point in the history
Introduce disable direct download feature in CLI.
  • Loading branch information
yoyama committed Dec 10, 2019
1 parent 508abad commit 04bc47a
Show file tree
Hide file tree
Showing 5 changed files with 300 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public abstract class ClientCommand
private static final Logger logger = LoggerFactory.getLogger(ClientCommand.class);

private static final String DEFAULT_ENDPOINT = "http://127.0.0.1:65432";
private static final String DEFAULT_DISABLE_DIRECT_DOWNLOAD = "false";

@Inject Injector injector;

Expand Down Expand Up @@ -170,10 +171,12 @@ private boolean isBatchModeForVersionCheck()
}

@VisibleForTesting
static DigdagClient buildClient(String endpoint, Map<String, String> env, Properties props, boolean disableCertValidation, Map<String, String> httpHeaders, Iterable<DigdagClientConfigurator> clientConfigurators)
static DigdagClient buildClient(String endpoint, Map<String, String> env, Properties props, boolean disableCertValidation,
Map<String, String> httpHeaders, Iterable<DigdagClientConfigurator> clientConfigurators)
throws SystemExitException
{
String[] fragments = endpoint.split(":", 2);
boolean disableDirectDownload = Boolean.parseBoolean(props.getProperty("client.http.disable_direct_download", DEFAULT_DISABLE_DIRECT_DOWNLOAD));

boolean useSsl = false;
if (fragments.length == 2 && fragments[1].startsWith("//")) {
Expand Down Expand Up @@ -219,6 +222,7 @@ static DigdagClient buildClient(String endpoint, Map<String, String> env, Proper
.port(port)
.ssl(useSsl)
.disableCertValidation(disableCertValidation)
.disableDirectDownload(disableDirectDownload)
.headers(headers);

Optional<ProxyConfig> proxyConfig = Proxies.proxyConfigFromEnv(scheme, env);
Expand Down
39 changes: 32 additions & 7 deletions digdag-client/src/main/java/io/digdag/client/DigdagClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public static class Builder
private final Map<String, String> baseHeaders = new HashMap<>();
private Function<Map<String, String>, Map<String, String>> headerBuilder = null;
private boolean disableCertValidation;
private boolean disableDirectDownload = false;

public Builder host(String host)
{
Expand Down Expand Up @@ -165,6 +166,12 @@ public Builder disableCertValidation(boolean value)
return this;
}

public Builder disableDirectDownload(boolean value)
{
this.disableDirectDownload = value;
return this;
}

public DigdagClient build()
{
return new DigdagClient(this);
Expand Down Expand Up @@ -197,6 +204,7 @@ public static Builder builder()

private final Client client;
private final ConfigFactory cf;
private final boolean disableDirectDownload;

private DigdagClient(Builder builder)
{
Expand Down Expand Up @@ -261,6 +269,9 @@ private DigdagClient(Builder builder)
this.client = clientBuilder.build();

this.cf = new ConfigFactory(mapper);

this.disableDirectDownload = builder.disableDirectDownload;

}

@Override
Expand Down Expand Up @@ -473,7 +484,7 @@ public InputStream getProjectArchive(Id projId, String revision)
WebTarget webTarget = target("/api/projects/{id}/archive")
.resolveTemplate("id", projId)
.queryParam("revision", revision);

webTarget = addDisableDirectDownloadParam(webTarget);
return withFollowingRedirect(webTarget,
(wt, lastResponse) -> {
Invocation.Builder request = wt.request();
Expand Down Expand Up @@ -628,18 +639,22 @@ public RestTaskCollection getTasks(Id attemptId)

public RestLogFileHandleCollection getLogFileHandlesOfAttempt(Id attemptId)
{
return doGet(RestLogFileHandleCollection.class,
target("/api/logs/{id}/files")
.resolveTemplate("id", attemptId));
WebTarget webTarget = target("/api/logs/{id}/files")
.resolveTemplate("id", attemptId);
webTarget = addDisableDirectDownloadParam(webTarget);

return doGet(RestLogFileHandleCollection.class, webTarget);
}

public RestLogFileHandleCollection getLogFileHandlesOfTask(Id attemptId, String taskName)
{
try {
return doGet(RestLogFileHandleCollection.class,
target("/api/logs/{id}/files")
WebTarget webTarget = target("/api/logs/{id}/files")
.resolveTemplate("id", attemptId)
.queryParam("task", URLEncoder.encode(taskName, "UTF-8")));
.queryParam("task", URLEncoder.encode(taskName, "UTF-8"));
webTarget = addDisableDirectDownloadParam(webTarget);

return doGet(RestLogFileHandleCollection.class, webTarget);
} catch (UnsupportedEncodingException ex) {
throw Throwables.propagate(ex);
}
Expand Down Expand Up @@ -671,6 +686,16 @@ public InputStream getLogFile(Id attemptId, String fileName)
.readEntity(InputStream.class);
}

private WebTarget addDisableDirectDownloadParam(WebTarget target)
{
// direct_download is default true
// So only set direct_download=false when disableDirectDownload == true
if (disableDirectDownload) {
target = target.queryParam("direct_download", false);
}
return target;
}

private Response invokeWithRetry(Invocation request)
{
Retryer<Response> retryer = RetryerBuilder.<Response>newBuilder()
Expand Down
3 changes: 3 additions & 0 deletions digdag-docs/src/command_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,13 @@ Client-mode common options:

Example: ``-c digdag-server/client.properties``



You can include following parameters in ~/.config/digdag/config file:

* client.http.endpoint = http://HOST:PORT or https://HOST:PORT
* client.http.headers.KEY = VALUE (set custom HTTP header)
* client.http.disable_direct_download=true (disable direct download in `log` and `download`. effect to server v0.10.0(not yet released) or later.)


start
Expand Down
140 changes: 140 additions & 0 deletions digdag-tests/src/test/java/acceptance/CliDownloadIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
package acceptance;

import io.netty.handler.codec.http.FullHttpRequest;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.littleshoot.proxy.HttpProxyServer;
import utils.CommandStatus;
import utils.TemporaryDigdagServer;
import utils.TestUtils;

import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static utils.TestUtils.copyResource;
import static utils.TestUtils.main;

public class CliDownloadIT
{
@Rule
public TemporaryFolder folder = new TemporaryFolder();

public TemporaryDigdagServer server;
private HttpProxyServer proxyServer;

private Map<String, String> env;
private Path config;
private Path projectDir;

private List<FullHttpRequest> requests = Collections.synchronizedList(new ArrayList<>());

@Before
public void setUp()
throws Exception
{
env = new HashMap<>();
proxyServer = TestUtils.startRequestTrackingProxy(requests);
server = TemporaryDigdagServer.builder()
.withRandomSecretEncryptionKey()
.build();
server.start();
projectDir = folder.getRoot().toPath().resolve("foobar");
String proxyUrl = "http://" + proxyServer.getListenAddress().getHostString() + ":" + proxyServer.getListenAddress().getPort();
env.put("http_proxy", proxyUrl);

config = folder.newFile().toPath();
}

@Test
public void disableDirectDownload()
throws Exception
{
// Create new project
CommandStatus initStatus = main("init",
"-c", config.toString(),
projectDir.toString());
assertThat(initStatus.code(), is(0));

copyResource("acceptance/basic.dig", projectDir.resolve("basic.dig"));

{
CommandStatus pushStatus = main(
"push",
"--project", projectDir.toString(),
"test_proj",
"-c", config.toString(),
"-e", server.endpoint()
);
assertThat(pushStatus.errUtf8(), pushStatus.code(), is(0));
}

// No configuration "client.http.disable_direct_download=true"
{
requests.clear();
CommandStatus status = main(env,
"download",
"test_proj",
"-c", config.toString(),
"-e", server.endpoint(),
"-o", folder.getRoot().toPath().resolve("test1").toString()
);
assertThat(status.errUtf8(), status.code(), is(0));
boolean match = false;
for (FullHttpRequest req : requests) {
if (req.uri().matches(".*/api/projects/.*/archive.*")) {
match = true;
assertThat("direct_download= must not be set.",
!req.uri().matches(".*direct_download=.*"));
}
}
assertThat("No record", match);
}

// Set configuration "client.http.disable_direct_download=true"
{
requests.clear();
Files.write(config, Arrays.asList("client.http.disable_direct_download=true"));
CommandStatus status = main(env,
"download",
"test_proj",
"-c", config.toString(),
"-e", server.endpoint(),
"-o", folder.getRoot().toPath().resolve("test3").toString()
);
assertThat(status.errUtf8(), status.code(), is(0));
boolean match = false;
for (FullHttpRequest req : requests) {
if (req.uri().matches(".*/api/projects/.*/archive.*")) {
match = true;
assertThat("direct_download= must be set.",req.uri().matches(".*direct_download=.*"));
}
}
assertThat("No record", match);
}
}

@After
public void tearDown()
throws Exception
{
if (proxyServer != null) {
proxyServer.stop();
proxyServer = null;
}
if (server != null) {
server.close();
server = null;
}
}
}
120 changes: 120 additions & 0 deletions digdag-tests/src/test/java/acceptance/CliLogIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package acceptance;

import io.digdag.client.DigdagClient;
import io.netty.handler.codec.http.FullHttpRequest;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.littleshoot.proxy.HttpProxyServer;
import utils.CommandStatus;
import utils.TemporaryDigdagServer;
import utils.TestUtils;

import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static utils.TestUtils.copyResource;
import static utils.TestUtils.main;

public class CliLogIT
{
@Rule
public TemporaryFolder folder = new TemporaryFolder();

public TemporaryDigdagServer server;
private HttpProxyServer proxyServer;

private Map<String, String> env;
private Path config;
private Path projectDir;

private List<FullHttpRequest> requests = Collections.synchronizedList(new ArrayList<>());

@Before
public void setUp()
throws Exception
{
env = new HashMap<>();
proxyServer = TestUtils.startRequestTrackingProxy(requests);
server = TemporaryDigdagServer.builder()
.withRandomSecretEncryptionKey()
.build();
server.start();
projectDir = folder.getRoot().toPath().resolve("foobar");
String proxyUrl = "http://" + proxyServer.getListenAddress().getHostString() + ":" + proxyServer.getListenAddress().getPort();
env.put("http_proxy", proxyUrl);

config = folder.newFile().toPath();

}

@Test
public void disableDirectDownload()
throws Exception
{
// No configuration "client.http.disable_direct_download=true"
{
requests.clear();
CommandStatus status = main(env,
"log",
"1",
"-c", config.toString(),
"-e", server.endpoint()
);
boolean match = false;
for (FullHttpRequest req :requests) {
if (req.uri().matches(".*/api/logs/1/.*")) {
assertThat("direct_download= must not be set.",
!req.uri().matches(".*direct_download=.*"));
match = true;
}
}
assertThat("No record", match);
}

// Set configuration "client.http.disable_direct_download=true"
{
requests.clear();
Files.write(config, Arrays.asList("client.http.disable_direct_download=true"));
CommandStatus status = main(env,
"log",
"3",
"-c", config.toString(),
"-e", server.endpoint()
);
boolean match = false;
for (FullHttpRequest req :requests) {
if (req.uri().matches(".*/api/logs/3/.*")) {
assertThat("direct_download=false must be set with client.http.disable_direct_download=true",
req.uri().matches(".*direct_download=false.*"));
match = true;
}
}
assertThat("No record", match);
}
}

@After
public void tearDown()
throws Exception
{
if (proxyServer != null) {
proxyServer.stop();
proxyServer = null;
}
if (server != null) {
server.close();
server = null;
}
}
}

0 comments on commit 04bc47a

Please sign in to comment.