Skip to content
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.

Sign up for GitHub

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

feat(vault): implements custom secret folder config #4385

Merged
merged 4 commits into from
Sep 19, 2024
Merged

feat(vault): implements custom secret folder config #4385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1de44d3
feat(vault): implements custom secret folder config
saschaisele-zf Jul 31, 2024
9b53e68
feat(vault): review suggestions
SaschaIsele Sep 9, 2024
22f5da4
feat(vault): more efficient null handling
SaschaIsele Sep 16, 2024
931e7a4
feat(vault): avoid duplication
SaschaIsele Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ...ault-hashicorp/src/main/java/org/eclipse/edc/vault/hashicorp/HashicorpVaultExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ public class HashicorpVaultExtension implements ServiceExtension {
@Setting(value = "The URL path of the vault's /secret endpoint", defaultValue = VAULT_API_SECRET_PATH_DEFAULT)
public static final String VAULT_API_SECRET_PATH = "edc.vault.hashicorp.api.secret.path";

@Setting(value = "The path of the folder that the secret is stored in, relative to VAULT_FOLDER_PATH")
public static final String VAULT_FOLDER_PATH = "edc.vault.hashicorp.folder";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe name it subfolder?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is whether the path is relative or absolute, not if it is a subfolder

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it depends on how you define relative/absolute in this case, right? 😄

For example the "full" URL would be: https://the-vault.com/v1/secrets/data/some/deep/deep/deep/folder/my-secret
This url splits up in the following parts:

  • https://the-vault.com/v1/secrets (already covered by VAULT_URL and VAULT_API_SECRET_PATH)
  • data (or metadata)
  • some/deep/deep/deep/folder (will be covered by VAULT_FOLDER_PATH
  • my-secret

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An absolute path always starts with a scheme, so I don't think there is any ambiguity here. From the example, the path is always relative so it should explicitly state that, "The path of the folder relative to VAULT_FOLDER_PATH..."

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SaschaIsele can you please align the changes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed the description to reflect the relative nature of the path


@Inject
private EdcHttpClient httpClient;

Expand Down Expand Up @@ -144,6 +147,7 @@ private HashicorpVaultSettings getSettings(ServiceExtensionContext context) {
var ttl = context.getSetting(VAULT_TOKEN_TTL, VAULT_TOKEN_TTL_DEFAULT);
var renewBuffer = context.getSetting(VAULT_TOKEN_RENEW_BUFFER, VAULT_TOKEN_RENEW_BUFFER_DEFAULT);
var secretPath = context.getSetting(VAULT_API_SECRET_PATH, VAULT_API_SECRET_PATH_DEFAULT);
var folderPath = context.getSetting(VAULT_FOLDER_PATH, null);

return HashicorpVaultSettings.Builder.newInstance()
.url(url)
Expand All @@ -155,6 +159,7 @@ private HashicorpVaultSettings getSettings(ServiceExtensionContext context) {
.ttl(ttl)
.renewBuffer(renewBuffer)
.secretPath(secretPath)
.folderPath(folderPath)
.build();
}
}
11 changes: 9 additions & 2 deletions ...-hashicorp/src/main/java/org/eclipse/edc/vault/hashicorp/client/HashicorpVaultClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,18 @@ private HttpUrl getSecretUrl(String key, String entryType) {
key = key.replace("%2F", "/");

var vaultApiPath = settings.secretPath();
var folderPath = settings.getFolderPath();

return settings.url()
var builder = settings.url()
.newBuilder()
.addPathSegments(PathUtil.trimLeadingOrEndingSlash(vaultApiPath))
.addPathSegment(entryType)
.addPathSegment(entryType);

if (folderPath != null) {
builder.addPathSegments(PathUtil.trimLeadingOrEndingSlash(folderPath));
}

return builder
.addPathSegments(key)
.build();
}
Expand Down
11 changes: 11 additions & 0 deletions ...ashicorp/src/main/java/org/eclipse/edc/vault/hashicorp/client/HashicorpVaultSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public class HashicorpVaultSettings {
private long renewBuffer;
private String secretPath;

private String folderPath;

private HashicorpVaultSettings() {
}

Expand Down Expand Up @@ -72,6 +74,10 @@ public String secretPath() {
return secretPath;
}

public String getFolderPath() {
return folderPath;
}

public static class Builder {
private final HashicorpVaultSettings values;

Expand Down Expand Up @@ -129,6 +135,11 @@ public Builder secretPath(String secretPath) {
return this;
}

public Builder folderPath(String folderPath) {
values.folderPath = folderPath;
return this;
}

public HashicorpVaultSettings build() {
requireNonNull(values.url, "Vault url must be valid");
requireNonNull(values.healthCheckPath, "Vault health check path must not be null");
Expand Down
42 changes: 42 additions & 0 deletions ...hicorp/src/test/java/org/eclipse/edc/vault/hashicorp/client/HashicorpVaultClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
class HashicorpVaultClientTest {

private static final String VAULT_URL = "https://mock.url";
private static final String SECRET_FOLDER = "/foo";
private static final String HEALTH_PATH = "sys/health";
private static final String VAULT_TOKEN = UUID.randomUUID().toString();
private static final long VAULT_TOKEN_TTL = 5L;
Expand All @@ -87,6 +88,17 @@ class HashicorpVaultClientTest {
.secretPath(CUSTOM_SECRET_PATH)
.build();

private static final HashicorpVaultSettings HASHICORP_VAULT_CLIENT_CONFIG_VALUES_WITH_FOLDER = HashicorpVaultSettings.Builder.newInstance()
.url(VAULT_URL)
.healthCheckPath(HEALTH_PATH)
.healthStandbyOk(false)
.token(VAULT_TOKEN)
.ttl(VAULT_TOKEN_TTL)
.renewBuffer(RENEW_BUFFER)
.secretPath(CUSTOM_SECRET_PATH)
.folderPath(SECRET_FOLDER)
.build();

private final EdcHttpClient httpClient = mock();
private final Monitor monitor = mock();
private final HashicorpVaultClient vaultClient = new HashicorpVaultClient(
Expand All @@ -95,6 +107,12 @@ class HashicorpVaultClientTest {
monitor,
HASHICORP_VAULT_CLIENT_CONFIG_VALUES);

private final HashicorpVaultClient vaultClientWithFolder = new HashicorpVaultClient(
httpClient,
OBJECT_MAPPER,
monitor,
HASHICORP_VAULT_CLIENT_CONFIG_VALUES_WITH_FOLDER);

@Nested
class HealthCheck {
@Test
Expand Down Expand Up @@ -393,6 +411,30 @@ void getSecret_whenApiReturns200_shouldSucceed() throws IOException {
request.url().encodedPathSegments().contains(KEY)));
}

@Test
void getSecret_with_folder_whenApiReturns200_shouldSucceed() throws IOException {
var ow = new ObjectMapper().writer();
var data = GetEntryResponsePayloadGetVaultEntryData.Builder.newInstance().data(new HashMap<>(0)).build();
var body = GetEntryResponsePayload.Builder.newInstance().data(data).build();
var bodyString = ow.writeValueAsString(body);
var response = new Response.Builder()
.code(200)
.message("any")
.body(ResponseBody.create(bodyString, MediaType.get("application/json")))
.protocol(Protocol.HTTP_1_1)
.request(new Request.Builder().url("http://any").build())
.build();

when(httpClient.execute(any(Request.class))).thenReturn(response);

var result = vaultClientWithFolder.getSecretValue(KEY);

assertNotNull(result);
verify(httpClient).execute(argThat(request -> request.method().equalsIgnoreCase("GET") &&
request.url().encodedPath().contains(CUSTOM_SECRET_PATH + "/data" + SECRET_FOLDER) &&
request.url().encodedPathSegments().contains(KEY)));
}

@Test
void setSecret_whenApiReturns200_shouldSucceed() throws IOException {
var ow = new ObjectMapper().writer();
Expand Down
1 change: 1 addition & 0 deletions ...corp/src/test/java/org/eclipse/edc/vault/hashicorp/client/HashicorpVaultSettingsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void createSettings_withDefaultValues_shouldSucceed() {
assertThat(settings.ttl()).isEqualTo(VAULT_TOKEN_TTL_DEFAULT);
assertThat(settings.renewBuffer()).isEqualTo(VAULT_TOKEN_RENEW_BUFFER_DEFAULT);
assertThat(settings.secretPath()).isEqualTo(SECRET_PATH);
assertThat(settings.getFolderPath()).isNull();
}

@Test
Expand Down
Loading