Skip to content

Commit

Permalink
fix(core): existing ns based on KV and not only flows
Browse files Browse the repository at this point in the history
Lookup if there are existing KV to know if a ns exist from the kv function and not only if flows exist in the KV.
  • Loading branch information
loicmathieu committed Dec 20, 2024
1 parent 8e5a84b commit 7b68b1b
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 15 deletions.
32 changes: 20 additions & 12 deletions core/src/main/java/io/kestra/core/services/KVStoreService.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import jakarta.inject.Inject;
import jakarta.inject.Singleton;

import java.io.IOException;

@Singleton
public class KVStoreService {

Expand All @@ -29,19 +31,7 @@ public class KVStoreService {
* @return The {@link KVStore}.
*/
public KVStore get(String tenant, String namespace, @Nullable String fromNamespace) {

// Only check namespace existence if not a descendant
boolean checkIfNamespaceExists = fromNamespace == null || isNotParentNamespace(namespace, fromNamespace);

if (checkIfNamespaceExists && !namespaceService.isNamespaceExists(tenant, namespace)) {
throw new KVStoreException(String.format(
"Cannot access the KV store. The namespace '%s' does not exist.",
namespace
));
}

boolean isNotSameNamespace = fromNamespace != null && !namespace.equals(fromNamespace);

if (isNotSameNamespace && isNotParentNamespace(namespace, fromNamespace)) {
try {
flowService.checkAllowedNamespace(tenant, namespace, tenant, fromNamespace);
Expand All @@ -52,6 +42,24 @@ public KVStore get(String tenant, String namespace, @Nullable String fromNamespa
}
}

// Only check namespace existence if not a descendant
boolean checkIfNamespaceExists = fromNamespace == null || isNotParentNamespace(namespace, fromNamespace);
if (checkIfNamespaceExists && !namespaceService.isNamespaceExists(tenant, namespace)) {
// if it didn't exist, we still check if there are KV as you can add KV without creating a namespace in DB or having flows in it
KVStore kvStore = new InternalKVStore(tenant, namespace, storageInterface);
try {
if (kvStore.list().isEmpty()) {
throw new KVStoreException(String.format(
"Cannot access the KV store. The namespace '%s' does not exist.",
namespace
));
}
} catch (IOException e) {
throw new KVStoreException(e);
}
return kvStore;
}

return new InternalKVStore(tenant, namespace, storageInterface);
}

Expand Down
14 changes: 12 additions & 2 deletions core/src/test/java/io/kestra/core/services/KVStoreServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

import io.kestra.core.junit.annotations.KestraTest;
import io.kestra.core.repositories.FlowRepositoryInterface;
import io.kestra.core.storages.kv.KVStoreException;
import io.kestra.core.storages.StorageInterface;
import io.kestra.core.storages.kv.*;
import io.micronaut.test.annotation.MockBean;
import jakarta.inject.Inject;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.time.Duration;
import java.util.Optional;

@KestraTest
Expand All @@ -19,7 +22,7 @@ class KVStoreServiceTest {
KVStoreService storeService;

@Inject
FlowRepositoryInterface flowRepository;
StorageInterface storageInterface;

@Test
void shouldGetKVStoreForExistingNamespaceGivenFromNull() {
Expand All @@ -37,6 +40,13 @@ void shouldGetKVStoreForAnyNamespaceWhenAccessingFromChildNamespace() {
Assertions.assertNotNull(storeService.get(null, "io.kestra", TEST_EXISTING_NAMESPACE));
}

@Test
void shouldGetKVStoreFromNonExistingNamespaceWithAKV() throws IOException {
KVStore kvStore = new InternalKVStore(null, "system", storageInterface);
kvStore.put("key", new KVValueAndMetadata(new KVMetadata(Duration.ofHours(1)), "value"));
Assertions.assertNotNull(storeService.get(null, "system", null));
}

@MockBean(NamespaceService.class)
public static class MockNamespaceService extends NamespaceService {

Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/io/kestra/plugin/core/kv/SetTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void shouldFailGivenNonExistingNamespace() {
.type(Set.class.getName())
.key("{{ inputs.key }}")
.value("{{ inputs.value }}")
.namespace("???")
.namespace("not-found")
.build();

// When - Then
Expand Down

0 comments on commit 7b68b1b

Please sign in to comment.