-
Notifications
You must be signed in to change notification settings - Fork 4.9k
secret-persistence: Hashicorp Vault Secret Store #13616
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
4b229e6
4c496f3
613231e
9d600b6
0986a33
29fe3ff
23925c5
a7cf4bd
8a44740
20c4bc6
a326740
846742f
bcdf863
c41e60a
0a7610d
1f34e0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| /* | ||
| * Copyright (c) 2022 Airbyte, Inc., all rights reserved. | ||
| */ | ||
|
|
||
| package io.airbyte.config.persistence.split_secrets; | ||
|
|
||
| import com.bettercloud.vault.Vault; | ||
| import com.bettercloud.vault.VaultConfig; | ||
| import com.bettercloud.vault.VaultException; | ||
| import io.airbyte.commons.lang.Exceptions; | ||
| import java.util.HashMap; | ||
| import java.util.Optional; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import lombok.val; | ||
|
|
||
| @Slf4j | ||
| final public class VaultSecretPersistence implements SecretPersistence { | ||
|
|
||
| private final String SECRET_KEY = "value"; | ||
| private final Vault vault; | ||
| private final String pathPrefix; | ||
|
|
||
| public VaultSecretPersistence(final String address, final String prefix) { | ||
| this.vault = Exceptions.toRuntime(() -> getVaultClient(address)); | ||
| this.pathPrefix = prefix; | ||
| } | ||
|
|
||
| /** | ||
| * Constructor for testing | ||
| */ | ||
| public VaultSecretPersistence(final String address, final String prefix, final String token) { | ||
| this.vault = Exceptions.toRuntime(() -> getVaultClient(address, token)); | ||
| this.pathPrefix = prefix; | ||
| } | ||
|
|
||
| @Override | ||
| public Optional<String> read(final SecretCoordinate coordinate) { | ||
| try { | ||
| val response = vault.logical().read(pathPrefix + coordinate.getFullCoordinate()); | ||
| val restResponse = response.getRestResponse(); | ||
| val responseCode = restResponse.getStatus(); | ||
| if (responseCode != 200) { | ||
| log.error("Vault failed on read. Response code: " + responseCode); | ||
| return Optional.empty(); | ||
| } | ||
| val data = response.getData(); | ||
| return Optional.of(data.get(SECRET_KEY)); | ||
| } catch (final VaultException e) { | ||
| return Optional.empty(); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void write(final SecretCoordinate coordinate, final String payload) { | ||
| try { | ||
| val newSecret = new HashMap<String, Object>(); | ||
| newSecret.put(SECRET_KEY, payload); | ||
| vault.logical().write(pathPrefix + coordinate.getFullCoordinate(), newSecret); | ||
| } catch (final VaultException e) { | ||
| log.error("Vault failed on write", e); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * This creates a vault client using a vault agent which uses AWS IAM for auth using engine version 2. | ||
| */ | ||
| private static Vault getVaultClient(final String address) throws VaultException { | ||
| val config = new VaultConfig() | ||
| .address(address) | ||
| .engineVersion(2) | ||
| .build(); | ||
| return new Vault(config); | ||
| } | ||
|
|
||
| /** | ||
| * Vault client for testing | ||
| */ | ||
| private static Vault getVaultClient(final String address, final String token) throws VaultException { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be made available for the non-test mode? It looks like it will only required to add one env variable.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should. Would this actually be 2 env vars? One to indicate the auth type and the other to pass in a token?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think this should be it. |
||
| val config = new VaultConfig() | ||
| .address(address) | ||
| .token(token) | ||
| .engineVersion(2) | ||
| .build(); | ||
| return new Vault(config); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| package io.airbyte.config.persistence.split_secrets; | ||
|
|
||
| import lombok.val; | ||
| import org.apache.commons.lang3.RandomUtils; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.testcontainers.vault.VaultContainer; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class VaultSecretPersistenceTest { | ||
| private VaultSecretPersistence persistence; | ||
| private String baseCoordinate; | ||
|
|
||
| private VaultContainer vaultContainer; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| vaultContainer = new VaultContainer("vault").withVaultToken("vault-dev-token-id"); | ||
| vaultContainer.start(); | ||
|
|
||
| val vaultAddress = "http://" + vaultContainer.getHost() + ":" + vaultContainer.getFirstMappedPort(); | ||
|
|
||
| persistence = new VaultSecretPersistence(vaultAddress, "secret/testing", "vault-dev-token-id"); | ||
| baseCoordinate = "VaultSecretPersistenceIntegrationTest_coordinate_" + RandomUtils.nextInt() % 20000; | ||
| } | ||
|
|
||
| @AfterEach | ||
| void tearDown() { | ||
| vaultContainer.stop(); | ||
| } | ||
|
|
||
| @Test | ||
| void testReadWriteUpdate() { | ||
| val coordinate1 = new SecretCoordinate(baseCoordinate, 1); | ||
|
|
||
| // try reading non-existent value | ||
| val firstRead = persistence.read(coordinate1); | ||
| assertThat(firstRead.isEmpty()).isTrue(); | ||
|
|
||
| // write | ||
| val firstPayload = "abc"; | ||
| persistence.write(coordinate1, firstPayload); | ||
| val secondRead = persistence.read(coordinate1); | ||
| assertThat(secondRead.isPresent()).isTrue(); | ||
| assertEquals(firstPayload, secondRead.get()); | ||
|
|
||
| // update | ||
| val secondPayload = "def"; | ||
| val coordinate2 = new SecretCoordinate(baseCoordinate, 2); | ||
| persistence.write(coordinate2, secondPayload); | ||
| val thirdRead = persistence.read(coordinate2); | ||
| assertThat(thirdRead.isPresent()).isTrue(); | ||
| assertEquals(secondPayload, thirdRead.get()); | ||
| } | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,11 @@ The following variables are relevant to both Docker and Kubernetes. | |
| #### Secrets | ||
| 1. `SECRET_STORE_GCP_PROJECT_ID` - Defines the GCP Project to store secrets in. Alpha support. | ||
| 2. `SECRET_STORE_GCP_CREDENTIALS` - Define the JSON credentials used to read/write Airbyte Configuration to Google Secret Manager. These credentials must have Secret Manager Read/Write access. Alpha support. | ||
| 3. `SECRET_PERSISTENCE` - Defines the Secret Persistence type. Defaults to NONE. Set to GOOGLE_SECRET_MANAGER to use Google Secret Manager. Set to TESTING_CONFIG_DB_TABLE to use the database as a test. Alpha support. Undefined behavior will result if this is turned on and then off. | ||
| 3. `VAULT_ADDRESS` - Define the vault address to read/write Airbyte Configuration to Hashicorp Vault. Alpha Support. | ||
| 4. `VAULT_PREFIX` - Define the vault path prefix. Empty by default. Alpha Support. | ||
| 5. `VAULT_TOKEN` - The token used for vault authentication. Alpha Support. | ||
| 6. `VAULT_AUTH_METHOD` - How vault will preform authentication. Currently, only supports Token auth. Defaults to token. Alpha Support. | ||
| 7. `SECRET_PERSISTENCE` - Defines the Secret Persistence type. Defaults to NONE. Set to GOOGLE_SECRET_MANAGER to use Google Secret Manager. Set to TESTING_CONFIG_DB_TABLE to use the database as a test. Set to VAULT to use Hashicorp Vault. Alpha support. Undefined behavior will result if this is turned on and then off. | ||
|
||
|
|
||
| #### Database | ||
| 1. `DATABASE_USER` - Define the Jobs Database user. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove that method. If the vault mode is wanted, they have to specify a token in their env. This will make it explicit for someone reading the code that the identification is token based.
getEnsureEnvin the Configs has a precondition for that.