From 62044b4605b8aa0b5401cbc6a4f53827d628cf1c Mon Sep 17 00:00:00 2001
From: Scott Leberknight <174812+sleberknight@users.noreply.github.com>
Date: Wed, 11 Nov 2020 17:14:42 -0500
Subject: [PATCH] Add SftpConfig
* Add SftpConfig and test
* Add package-private AppConfiguration, used by the test
* Add YAML and JSON external configuration files, used by the test
Fixes #420
---
.../java/org/kiwiproject/jsch/SftpConfig.java | 159 ++++++++++
.../kiwiproject/jsch/AppConfiguration.java | 17 ++
.../org/kiwiproject/jsch/SftpConfigTest.java | 289 ++++++++++++++++++
.../SftpConfigTest/config-all-properties.json | 12 +
.../SftpConfigTest/config-all-properties.yml | 12 +
.../config-minimal-properties.json | 8 +
.../config-minimal-properties.yml | 8 +
7 files changed, 505 insertions(+)
create mode 100644 src/main/java/org/kiwiproject/jsch/SftpConfig.java
create mode 100644 src/test/java/org/kiwiproject/jsch/AppConfiguration.java
create mode 100644 src/test/java/org/kiwiproject/jsch/SftpConfigTest.java
create mode 100644 src/test/resources/SftpConfigTest/config-all-properties.json
create mode 100644 src/test/resources/SftpConfigTest/config-all-properties.yml
create mode 100644 src/test/resources/SftpConfigTest/config-minimal-properties.json
create mode 100644 src/test/resources/SftpConfigTest/config-minimal-properties.yml
diff --git a/src/main/java/org/kiwiproject/jsch/SftpConfig.java b/src/main/java/org/kiwiproject/jsch/SftpConfig.java
new file mode 100644
index 00000000..5fc0f805
--- /dev/null
+++ b/src/main/java/org/kiwiproject/jsch/SftpConfig.java
@@ -0,0 +1,159 @@
+package org.kiwiproject.jsch;
+
+import static java.util.Objects.isNull;
+import static org.apache.commons.lang3.StringUtils.isBlank;
+
+import io.dropwizard.util.Duration;
+import io.dropwizard.validation.MinDuration;
+import io.dropwizard.validation.PortRange;
+import lombok.Builder;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.Setter;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import java.beans.ConstructorProperties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * This (Dropwizard) config class allows for common configuration for SFTP access to remote hosts.
+ *
+ * One note on the {@code password} and {@code privateKeyFilePath} fields: the SFTP connector will use either the
+ * password or the private key but not both. If both are specified, the private key will take precedence. This allows
+ * for the greatest flexibility when connecting to different SFTP locations.
+ *
+ * @implNote This uses Dropwizard's {@link Duration} and several Dropwizard validation annotations in addition to the
+ * standard Java Beans Validation annotations, so you will need to ensure the the appropriate Dropwizard JAR files are
+ * on the class/module path.
+ */
+@Getter
+@Setter
+@NoArgsConstructor
+@Builder
+public class SftpConfig {
+
+ private static final int DEFAULT_PORT = 22;
+ private static final String DEFAULT_PREFERRED_AUTHENTICATIONS = "publickey,password";
+ private static final Duration DEFAULT_TIMEOUT = Duration.seconds(5);
+
+ /**
+ * The SFTP port. Default is 22.
+ */
+ @PortRange
+ private int port = DEFAULT_PORT;
+
+ /**
+ * The remote host to connect to via SFTP.
+ */
+ @NotBlank
+ private String host;
+
+ /**
+ * The remote user to connect with via SFTP.
+ */
+ @NotBlank
+ private String user;
+
+ /**
+ * The password, only used if password authentication is used.
+ */
+ private String password;
+
+ /**
+ * The path to the private key file, only used if public key authentication is used.
+ */
+ private String privateKeyFilePath;
+
+ /**
+ * The comma-separated list of preferred authentication mechanisms. Equivalent to the
+ * {@code -o PreferredAuthentications=[gssapi-with-mic|hostbased|publickey|keyboard-interactive|password]} option.
+ *
+ * The default is {@code publickey,password} which differs from the default order in the SFTP command. See
+ * the {@code ssh_info} man page for details.
+ */
+ @NotBlank
+ private String preferredAuthentications = DEFAULT_PREFERRED_AUTHENTICATIONS;
+
+ /**
+ * The root directory of the remote SFTP location, provided as a convenience to store the remote path in the same
+ * place as the other SFTP properties.
+ *
+ * It is not required nor currently used by {@link SftpConnector} or {@link SftpTransfers}.
+ */
+ private String remoteBasePath;
+
+ /**
+ * The local directory to write out any errors if SFTP fails.
+ */
+ @NotBlank
+ private String errorPath;
+
+ /**
+ * The path to the known hosts file.
+ */
+ @NotBlank
+ private String knownHostsFile;
+
+ /**
+ * Provides option to disable strict host key checking, equivalent to the
+ * {@code -o StrictHostKeyChecking=[yes|no]} option.
+ *
+ * The default is false. See the {@code ssh_info} man page for more details.
+ */
+ private boolean disableStrictHostChecking;
+
+ /**
+ * SFTP connection timeout. Default is 5 seconds.
+ *
+ * @implNote The minimum duration annotation of 50 milliseconds is somewhat arbitrary, and is really just intended
+ * to ensure a positive value.
+ */
+ @NotNull
+ @MinDuration(value = 50, unit = TimeUnit.MILLISECONDS)
+ private Duration timeout = DEFAULT_TIMEOUT;
+
+ /**
+ * All-args constructor.
+ *
+ * @implNote This is intentionally not using Lombok because using {@link lombok.AllArgsConstructor} together
+ * with @{@link Builder} results in an all-args constructor that does not respect {@link lombok.Builder.Default}.
+ * As a result we need to handle the defaults ourselves. This is intended to be used during deserialization
+ * from an external configuration file (e.g. a Dropwizard YAML configuration file, or from JSON). Prefer the
+ * builder when constructing programmatically.
+ */
+ // Suppress Sonar's "Methods should not have too many parameters". We need this for the builder and for
+ // deserialization from YAML or JSON configurations.
+ @SuppressWarnings({"java:S107"})
+ @ConstructorProperties({
+ "port", "host", "user", "password",
+ "privateKeyFilePath", "preferredAuthentications",
+ "remoteBasePath", "errorPath", "knownHostsFile",
+ "disableStrictHostChecking", "timeout"
+ })
+ public SftpConfig(int port,
+ String host,
+ String user,
+ String password,
+ String privateKeyFilePath,
+ String preferredAuthentications,
+ String remoteBasePath,
+ String errorPath,
+ String knownHostsFile,
+ boolean disableStrictHostChecking,
+ Duration timeout) {
+
+ this.port = (port == 0) ? DEFAULT_PORT : port;
+ this.host = host;
+ this.user = user;
+ this.password = password;
+ this.privateKeyFilePath = privateKeyFilePath;
+ this.preferredAuthentications =
+ isBlank(preferredAuthentications) ? DEFAULT_PREFERRED_AUTHENTICATIONS : preferredAuthentications;
+ this.remoteBasePath = remoteBasePath;
+ this.errorPath = errorPath;
+ this.knownHostsFile = knownHostsFile;
+ this.disableStrictHostChecking = disableStrictHostChecking;
+ this.timeout = isNull(timeout) ? DEFAULT_TIMEOUT : timeout;
+ }
+}
diff --git a/src/test/java/org/kiwiproject/jsch/AppConfiguration.java b/src/test/java/org/kiwiproject/jsch/AppConfiguration.java
new file mode 100644
index 00000000..dda97073
--- /dev/null
+++ b/src/test/java/org/kiwiproject/jsch/AppConfiguration.java
@@ -0,0 +1,17 @@
+package org.kiwiproject.jsch;
+
+import io.dropwizard.Configuration;
+import lombok.Getter;
+import lombok.Setter;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotNull;
+
+@Getter
+@Setter
+class AppConfiguration extends Configuration {
+
+ @NotNull
+ @Valid
+ private SftpConfig sftpConfig;
+}
diff --git a/src/test/java/org/kiwiproject/jsch/SftpConfigTest.java b/src/test/java/org/kiwiproject/jsch/SftpConfigTest.java
new file mode 100644
index 00000000..e553cda6
--- /dev/null
+++ b/src/test/java/org/kiwiproject/jsch/SftpConfigTest.java
@@ -0,0 +1,289 @@
+package org.kiwiproject.jsch;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.kiwiproject.validation.ValidationTestHelper.DEFAULT_VALIDATOR;
+import static org.kiwiproject.validation.ValidationTestHelper.assertPropertyViolations;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import io.dropwizard.configuration.ResourceConfigurationSourceProvider;
+import io.dropwizard.configuration.YamlConfigurationFactory;
+import io.dropwizard.jackson.Jackson;
+import io.dropwizard.testing.FixtureHelpers;
+import io.dropwizard.util.Duration;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.kiwiproject.validation.ValidationTestHelper;
+import org.opentest4j.AssertionFailedError;
+
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.stream.Stream;
+
+@DisplayName("SftpConfig")
+class SftpConfigTest {
+
+ private static final ObjectMapper DW_OBJECT_MAPPER = Jackson.newObjectMapper();
+
+ @Nested
+ class DefaultValues {
+
+ private static final String SFTP_CONFIG_FACTORY = "org.kiwiproject.jsch.SftpConfigTest#defaultSftpConfigs";
+
+ @ParameterizedTest
+ @MethodSource(SFTP_CONFIG_FACTORY)
+ void shouldHaveDefaultPort(SftpConfig config) {
+ assertThat(config.getPort()).isEqualTo(22);
+ }
+
+ @ParameterizedTest
+ @MethodSource(SFTP_CONFIG_FACTORY)
+ void shouldHaveDefaultPreferredAuthentications(SftpConfig config) {
+ assertThat(config.getPreferredAuthentications()).isEqualTo("publickey,password");
+ }
+
+ @ParameterizedTest
+ @MethodSource(SFTP_CONFIG_FACTORY)
+ void shouldHaveDefaultTimeout(SftpConfig config) {
+ assertThat(config.getTimeout()).isEqualTo(Duration.seconds(5));
+ }
+
+ @ParameterizedTest
+ @MethodSource(SFTP_CONFIG_FACTORY)
+ void shouldHaveNullDefaults(SftpConfig config) {
+ assertThat(config.getHost()).isNull();
+ assertThat(config.getUser()).isNull();
+ assertThat(config.getPassword()).isNull();
+ assertThat(config.getPrivateKeyFilePath()).isNull();
+ assertThat(config.getRemoteBasePath()).isNull();
+ assertThat(config.getErrorPath()).isNull();
+ assertThat(config.getKnownHostsFile()).isNull();
+ }
+ }
+
+ /**
+ * Factory for {@link SftpConfig} instances using the default constructor, all-args constructor, and builder.
+ */
+ @SuppressWarnings("unused") // IntelliJ says this is unused when referenced via a constant. It is wrong.
+ static Stream defaultSftpConfigs() {
+ return Stream.of(
+ new SftpConfig(),
+ newConfigUsingAllArgsConstructor(),
+ SftpConfig.builder().build()
+ );
+ }
+
+ private static SftpConfig newConfigUsingAllArgsConstructor() {
+ return new SftpConfig(0, null, null, null, null, null, null, null, null, false, null);
+ }
+
+ @Nested
+ class Deserializing {
+
+ @Nested
+ class FromYaml {
+
+ private static final String DW_PROPERTY_PREFIX = "dw";
+
+ @Test
+ void shouldAllowOverridingDefaultValues() {
+ var sftpConfig = deserializeAndExtractSftpConfig("config-all-properties.yml");
+ assertExplicitlySpecifiedProperties(sftpConfig);
+ assertDefaultValuesOverridden(sftpConfig);
+ }
+
+ @Test
+ void shouldRespectDefaultValues() {
+ var sftpConfig = deserializeAndExtractSftpConfig("config-minimal-properties.yml");
+ assertExplicitlySpecifiedProperties(sftpConfig);
+ assertDefaultValuesUsed(sftpConfig);
+ }
+
+ private SftpConfig deserializeAndExtractSftpConfig(String configFileName) {
+ var factory = new YamlConfigurationFactory<>(
+ AppConfiguration.class,
+ DEFAULT_VALIDATOR,
+ DW_OBJECT_MAPPER,
+ DW_PROPERTY_PREFIX
+ );
+ var path = Paths.get("SftpConfigTest", configFileName).toString();
+
+ try {
+ var config = factory.build(new ResourceConfigurationSourceProvider(), path);
+ return config.getSftpConfig();
+ } catch (Exception e) {
+ throw new AssertionFailedError("Error de-serializing config at path: " + path, e);
+ }
+ }
+ }
+
+ @Nested
+ class FromJson {
+
+ @Test
+ void shouldAllowOverridingDefaultValues() throws IOException {
+ var json = FixtureHelpers.fixture("SftpConfigTest/config-all-properties.json");
+ var sftpConfig = DW_OBJECT_MAPPER.readValue(json, SftpConfig.class);
+ assertExplicitlySpecifiedProperties(sftpConfig);
+ assertDefaultValuesOverridden(sftpConfig);
+ }
+
+ @Test
+ void shouldRespectDefaultValues() throws IOException {
+ var json = FixtureHelpers.fixture("SftpConfigTest/config-minimal-properties.json");
+ var sftpConfig = DW_OBJECT_MAPPER.readValue(json, SftpConfig.class);
+ assertExplicitlySpecifiedProperties(sftpConfig);
+ assertDefaultValuesUsed(sftpConfig);
+ }
+ }
+
+ private void assertExplicitlySpecifiedProperties(SftpConfig sftpConfig) {
+ assertThat(sftpConfig.getHost()).isEqualTo("localhost");
+ assertThat(sftpConfig.getUser()).isEqualTo("bob");
+ assertThat(sftpConfig.getPrivateKeyFilePath()).isEqualTo("/home/bob/.ssh/id_rsa");
+ assertThat(sftpConfig.getKnownHostsFile()).isEqualTo("/home/bob/.ssh/known_hosts");
+ assertThat(sftpConfig.getRemoteBasePath()).isEqualTo("/data_shared/development/my-service/remote-base-path");
+ assertThat(sftpConfig.getErrorPath()).isEqualTo("/data_shared/development/my-service/transfer-errors");
+ }
+
+ private void assertDefaultValuesOverridden(SftpConfig sftpConfig) {
+ assertThat(sftpConfig.getPort()).isEqualTo(42);
+ assertThat(sftpConfig.getPreferredAuthentications()).isEqualTo("publickey");
+ assertThat(sftpConfig.isDisableStrictHostChecking()).isTrue();
+ assertThat(sftpConfig.getTimeout()).isEqualTo(Duration.seconds(10));
+ }
+
+ private void assertDefaultValuesUsed(SftpConfig sftpConfig) {
+ assertThat(sftpConfig.getPort()).isEqualTo(22);
+ assertThat(sftpConfig.getPassword()).isNull();
+ assertThat(sftpConfig.getPreferredAuthentications()).isEqualTo("publickey,password");
+ assertThat(sftpConfig.isDisableStrictHostChecking()).isFalse();
+ assertThat(sftpConfig.getTimeout()).isEqualTo(Duration.seconds(5));
+ }
+ }
+
+ @Nested
+ class Validation {
+
+ private static final String BLANK_STRINGS_FACTORY = "org.kiwiproject.jsch.SftpConfigTest#someBlankStrings";
+
+ private SftpConfig config;
+
+ @BeforeEach
+ void setUp() {
+ config = new SftpConfig();
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {-1, 65_536})
+ void shouldRequireValidPort(int port) {
+ config.setPort(port);
+ assertOnePropertyViolation(config, "port");
+ }
+
+ @ParameterizedTest
+ @MethodSource(BLANK_STRINGS_FACTORY)
+ void shouldRequireHost(String value) {
+ config.setHost(value);
+ assertOnePropertyViolation(config, "host");
+ }
+
+ @ParameterizedTest
+ @MethodSource(BLANK_STRINGS_FACTORY)
+ void shouldRequirePreferredAuthentications(String value) {
+ config.setPreferredAuthentications(value);
+ assertOnePropertyViolation(config, "preferredAuthentications");
+ }
+
+ @Test
+ void shouldNotRequirePassword() {
+ assertNoPropertyViolations(config, "password");
+ }
+
+ @Test
+ void shouldNotRequirePrivateKeyFilePath() {
+ assertNoPropertyViolations(config, "privateKeyFilePath");
+ }
+
+ @ParameterizedTest
+ @MethodSource(BLANK_STRINGS_FACTORY)
+ void shouldNotRequireRemoteBasePath(String value) {
+ config.setRemoteBasePath(value);
+ assertNoPropertyViolations(config, "remoteBasePath");
+ }
+
+ @ParameterizedTest
+ @MethodSource(BLANK_STRINGS_FACTORY)
+ void shouldRequireErrorPath(String value) {
+ config.setErrorPath(value);
+ assertOnePropertyViolation(config, "errorPath");
+ }
+
+ @ParameterizedTest
+ @MethodSource(BLANK_STRINGS_FACTORY)
+ void shouldRequireKnownHostsFile(String value) {
+ config.setKnownHostsFile(value);
+ assertOnePropertyViolation(config, "knownHostsFile");
+ }
+
+ @Test
+ void shouldRequireTimeout() {
+ config.setTimeout(null);
+ assertOnePropertyViolation(config, "timeout");
+ }
+
+ @ParameterizedTest
+ @CsvSource({
+ "0, 1",
+ "1, 1",
+ "48, 1",
+ "49, 1",
+ "50, 0",
+ "500, 0",
+ "5000, 0",
+ "50000, 0",
+ })
+ void shouldRequireMinimumTimeout(long millis, int numExpectedViolations) {
+ config.setTimeout(Duration.milliseconds(millis));
+ assertPropertyViolations(DEFAULT_VALIDATOR, config, "timeout", numExpectedViolations);
+ }
+ }
+
+ private static Stream someBlankStrings() {
+ return Stream.of(
+ // Basics
+ null,
+ "",
+ " ",
+ " ",
+
+ // ASCII characters
+ "\t", // HORIZONTAL TABULATION (U+0009
+ "\n", // LINE FEED (U+000A
+ "\u000B", // VERTICAL TABULATION
+ "\f", // FORM FEED (U+000C
+ "\r", // CARRIAGE RETURN (U+000D
+ "\u001C", // FILE SEPARATOR
+ "\u001D", // GROUP SEPARATOR
+ "\u001E",
+ "\u001F",
+
+ // Multiple character tests
+ // ASCII chars
+ "\t \n \u000B \f \r \u001C \u001D \u001E \u001F"
+ );
+ }
+
+ private static void assertOnePropertyViolation(SftpConfig config, String propertyName) {
+ ValidationTestHelper.assertOnePropertyViolation(DEFAULT_VALIDATOR, config, propertyName);
+ }
+
+ private static void assertNoPropertyViolations(SftpConfig config, String propertyName) {
+ ValidationTestHelper.assertNoPropertyViolations(DEFAULT_VALIDATOR, config, propertyName);
+ }
+}
diff --git a/src/test/resources/SftpConfigTest/config-all-properties.json b/src/test/resources/SftpConfigTest/config-all-properties.json
new file mode 100644
index 00000000..7aa4bc2e
--- /dev/null
+++ b/src/test/resources/SftpConfigTest/config-all-properties.json
@@ -0,0 +1,12 @@
+{
+ "port": 42,
+ "host": "localhost",
+ "user": "bob",
+ "preferredAuthentications": "publickey",
+ "privateKeyFilePath": "/home/bob/.ssh/id_rsa",
+ "knownHostsFile": "/home/bob/.ssh/known_hosts",
+ "remoteBasePath": "/data_shared/development/my-service/remote-base-path",
+ "errorPath": "/data_shared/development/my-service/transfer-errors",
+ "disableStrictHostChecking": true,
+ "timeout": "10 seconds"
+}
diff --git a/src/test/resources/SftpConfigTest/config-all-properties.yml b/src/test/resources/SftpConfigTest/config-all-properties.yml
new file mode 100644
index 00000000..6e6dd0da
--- /dev/null
+++ b/src/test/resources/SftpConfigTest/config-all-properties.yml
@@ -0,0 +1,12 @@
+---
+sftpConfig:
+ port: 42
+ host: localhost
+ user: bob
+ preferredAuthentications: publickey
+ privateKeyFilePath: /home/bob/.ssh/id_rsa
+ knownHostsFile: /home/bob/.ssh/known_hosts
+ remoteBasePath: /data_shared/development/my-service/remote-base-path
+ errorPath: /data_shared/development/my-service/transfer-errors
+ disableStrictHostChecking: true
+ timeout: 10 seconds
diff --git a/src/test/resources/SftpConfigTest/config-minimal-properties.json b/src/test/resources/SftpConfigTest/config-minimal-properties.json
new file mode 100644
index 00000000..f4e3dc7c
--- /dev/null
+++ b/src/test/resources/SftpConfigTest/config-minimal-properties.json
@@ -0,0 +1,8 @@
+{
+ "host": "localhost",
+ "user": "bob",
+ "privateKeyFilePath": "/home/bob/.ssh/id_rsa",
+ "knownHostsFile": "/home/bob/.ssh/known_hosts",
+ "remoteBasePath": "/data_shared/development/my-service/remote-base-path",
+ "errorPath": "/data_shared/development/my-service/transfer-errors"
+}
diff --git a/src/test/resources/SftpConfigTest/config-minimal-properties.yml b/src/test/resources/SftpConfigTest/config-minimal-properties.yml
new file mode 100644
index 00000000..57b2bb7a
--- /dev/null
+++ b/src/test/resources/SftpConfigTest/config-minimal-properties.yml
@@ -0,0 +1,8 @@
+---
+sftpConfig:
+ host: localhost
+ user: bob
+ privateKeyFilePath: /home/bob/.ssh/id_rsa
+ knownHostsFile: /home/bob/.ssh/known_hosts
+ remoteBasePath: /data_shared/development/my-service/remote-base-path
+ errorPath: /data_shared/development/my-service/transfer-errors