Skip to content

Commit

Permalink
SDC-1289. com.streamsets.datacollector.vault.VaultITVersion1Test and …
Browse files Browse the repository at this point in the history
…VaultITVersion2Test are failing in jenkins

This patch reverts SDC-12896 and SDC-10220.

Change-Id: Ibec00fd3e2d3c7c3f58e88dda785009afac3adae
Reviewed-on: https://review.streamsets.net/c/datacollector/+/27606
Tested-by: StreamSets CI <streamsets-ci-spam@streamsets.com>
Reviewed-by: Hugo Prol <hugo@streamsets.com>
  • Loading branch information
dvianabarbosa committed Nov 14, 2019
1 parent 480d908 commit 1dbdf13
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 230 deletions.
3 changes: 0 additions & 3 deletions dist/src/main/etc/credential-stores.properties
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ credentialStore.jks.config.keystore.storePassword=changeIt

# Optional Settings #

# Supported KV Secret Engine version 1 by default. Possible values: 1 or 2.
credentialStore.vault.config.version=1

# The renewal interval must be shorter than the shortest lease issued by Vault including auth tokens.
credentialStore.vault.config.lease.renewal.interval.sec=60
credentialStore.vault.config.lease.expiration.buffer.sec=120
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@ public class VaultCredentialStore implements CredentialStore {
public static final String PATH_KEY_SEPARATOR_DEFAULT = "&";
public static final String SEPARATOR_OPTION = "separator";
public static final String DELAY_OPTION = "delay";
public static final String VERSION_OPTION = "version";

private String pathKeySeparator;
private Vault vault;


@Override
public List<ConfigIssue> init(Context context) {
List<ConfigIssue> issues = new ArrayList<>();
Expand Down Expand Up @@ -83,11 +81,6 @@ public String get() throws StageException {
return vault.read(path, key, delay);
}

public String getWithOpts(int version) throws StageException {
LOG.debug("Getting '{}' delay '{}'", this, delay);
return vault.read(path, key, delay, version);
}

@Override
public String toString() {
return "VaultCredentialValue{" + "path='" + path + '\'' + ", key='" + key + '\'' + '}';
Expand Down Expand Up @@ -118,14 +111,8 @@ public CredentialValue get(String group, String name, String credentialStoreOpti
String delayStr = optionsMap.get(DELAY_OPTION);
long delay = (delayStr == null) ? 0 : Long.parseLong(delayStr);

VaultCredentialValue credential = new VaultCredentialValue(splits[0], splits[1], delay);

if (!optionsMap.containsKey(VERSION_OPTION)) {
credential.get();
} else {
credential.getWithOpts(Integer.parseInt(optionsMap.get(VERSION_OPTION)));
}

CredentialValue credential = new VaultCredentialValue(splits[0], splits[1], delay);
credential.get();
return credential;
} catch (Exception ex) {
throw new StageException(Errors.VAULT_001, name, ex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ public String token() {
/**
* Reads a secret from the local cache if it hasn't expired and returns the value for the specified key.
* If the secret isn't cached or has expired, it requests it from Vault again.
*
* @param path path in Vault to read
* @param key key of the property of the secret represented by the path to return
* @return value of the specified key for the requested secret.
Expand All @@ -316,19 +317,6 @@ public String read(String path, String key) {
return read(path, key, 0L);
}

/**
* Reads a secret from the local cache if it hasn't expired and returns the value for the specified key.
* If the secret isn't cached or has expired, it requests it from Vault again.
*
* @param path path in Vault to read
* @param key key of the property of the secret represented by the path to return
* @param delay delay in milliseconds to wait before returning the value if it wasn't already cached.
* @return value of the specified key for the requested secret.
*/
public String read(String path, String key, long delay) {
return read(path, key, delay, getConfig().getVersion());
}

/**
* Reads a secret from the local cache if it hasn't expired and returns the value for the specified key.
* If the secret isn't cached or has expired, it requests it from Vault again.
Expand All @@ -338,32 +326,12 @@ public String read(String path, String key, long delay) {
* a secret (access keys for example) before they have propagated to all AWS services. For AWS a delay of up to 5
* or 10 seconds may be necessary. If you receive a 403 from AWS services you probably need to increase the delay.
*
* The function handle KV Secret Engine v1 and v2. When v2, the expected endpoint for the secret will be
* /secret/data/<path> instead of /secret/<path>.
*
* @param path path in Vault to read
* @param key key of the property of the secret represented by the path to return
* @param delay delay in milliseconds to wait before returning the value if it wasn't already cached.
* @param version version of KV Secret Engine. Allowed values: 0, 1 or 2. When 0, the function takes the version
* from credential-stores.properties file.
* @return value of the specified key for the requested secret.
*/
public String read(String path, String key, long delay, int version) {

if (version == 2) {
boolean isData = false;
for (String part : mapSplitter.split(path)) {
if (isData) {
path = path.concat("/").concat(part);
} else {
path = part.concat("/data");
isData = true;
}
}
} else if (version != 1) {
throw new VaultRuntimeException(Utils.format("Version not allowed: {}", version));
}

public String read(String path, String key, long delay) {
if (!secrets.containsKey(path)) {
VaultClient vault = new VaultClient(getConfig());
Secret secret;
Expand Down Expand Up @@ -396,9 +364,6 @@ public String read(String path, String key, long delay, int version) {
}

Map<String, Object> data = secrets.get(path).getData();
if (version == 2) {
data = (Map<String, Object>) data.get("data");
}
String value = getSecretValue(data, key).orElseThrow(() -> new VaultRuntimeException("Value not found for key"));
LOG.trace("CredentialStore '{}' Vault, retrieved value for key '{}'", csId, key);
return value;
Expand Down Expand Up @@ -484,7 +449,6 @@ private VaultConfiguration parseVaultConfigs(Configuration configuration) {
}

config = VaultConfigurationBuilder.newVaultConfiguration()
.withVersion(configuration.get("version", 1))
.withAddress(configuration.get("addr", VaultConfigurationBuilder.DEFAULT_ADDRESS))
.withOpenTimeout(configuration.get("open.timeout", 0))
.withProxyOptions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
public class VaultConfiguration {
private final String address;
private final String token;
private final int version;
private final int openTimeout;
private final ProxyOptions proxyOptions;
private final int readTimeout;
Expand All @@ -28,7 +27,6 @@ public class VaultConfiguration {
public VaultConfiguration(
String address,
String token,
int version,
int openTimeout,
ProxyOptions proxyOptions,
int readTimeout,
Expand All @@ -37,7 +35,6 @@ public VaultConfiguration(
) {
this.address = address;
this.token = token;
this.version = version;
this.openTimeout = openTimeout;
this.proxyOptions = proxyOptions;
this.readTimeout = readTimeout;
Expand All @@ -53,10 +50,6 @@ public String getToken() {
return token;
}

public int getVersion() {
return version;
}

public int getOpenTimeout() {
return openTimeout;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ public final class VaultConfigurationBuilder {

private String address = DEFAULT_ADDRESS;
private String token = "";
private int version = 1;
private int openTimeout = 0;
private ProxyOptions proxyOptions = ProxyOptionsBuilder.newProxyOptions().build();
private int readTimeout = 0;
Expand All @@ -38,7 +37,6 @@ public VaultConfigurationBuilder fromVaultConfiguration(VaultConfiguration conf)
return new VaultConfigurationBuilder()
.withAddress(conf.getAddress())
.withToken(conf.getToken())
.withVersion(conf.getVersion())
.withOpenTimeout(conf.getOpenTimeout())
.withProxyOptions(conf.getProxyOptions())
.withReadTimeout(conf.getReadTimeout())
Expand All @@ -55,11 +53,6 @@ public VaultConfigurationBuilder withToken(String token) {
this.token = token;
return this;
}

public VaultConfigurationBuilder withVersion(int version) {
this.version = version;
return this;
}

public VaultConfigurationBuilder withOpenTimeout(int openTimeout) {
this.openTimeout = openTimeout;
Expand Down Expand Up @@ -87,6 +80,6 @@ public VaultConfigurationBuilder withTimeout(int timeout) {
}

public VaultConfiguration build() {
return new VaultConfiguration(address, token, version, openTimeout, proxyOptions, readTimeout, sslOptions, timeout);
return new VaultConfiguration(address, token, openTimeout, proxyOptions, readTimeout, sslOptions, timeout);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@
import static org.junit.Assert.assertTrue;

@RunWith(Parameterized.class)
public class VaultITVersion1Test {
private static final Logger LOG = LoggerFactory.getLogger(VaultITVersion1Test.class);
public class VaultIT {
private static final Logger LOG = LoggerFactory.getLogger(VaultIT.class);

private static final String VAULT_VERSION = "0.9.6"; // Version to test KV Secrets Engine v1.
private static final String VAULT_VERSION = "0.9.6"; // Latest version defaults to APIv2 which we do not support yet
private static final int VAULT_PORT = 8200;
private static final String VAULT_DEV_ROOT_TOKEN_ID = "root-token";
private static final String VAULT_DEV_LISTEN_ADDRESS = "0.0.0.0:" + VAULT_PORT;
Expand Down

This file was deleted.

0 comments on commit 1dbdf13

Please sign in to comment.