chore | delete configs permanently rather than soft deletes

config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceGrpcImpl.java

@@ -1,6 +1,5 @@
 package org.hypertrace.config.service;
 
-import static org.hypertrace.config.service.ConfigServiceUtils.emptyValue;
 import static org.hypertrace.config.service.ConfigServiceUtils.filterNull;
 import static org.hypertrace.config.service.ConfigServiceUtils.merge;
 
@@ -11,6 +10,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
 import org.hypertrace.config.service.store.ConfigStore;
@@ -125,16 +125,15 @@ public void deleteConfig(
     try {
       ConfigResourceContext configResourceContext = getConfigResourceContext(request);
       ContextSpecificConfig configToDelete = configStore.getConfig(configResourceContext);
-
       // if configToDelete is null/empty (i.e. config value doesn't exist or is already deleted),
       // then throw NOT_FOUND exception
-      if (ConfigServiceUtils.isNull(configToDelete.getConfig())) {
+      if (configToDelete == null || ConfigServiceUtils.isNull(configToDelete.getConfig())) {
         responseObserver.onError(Status.NOT_FOUND.asException());
         return;
       }
 
-      // write an empty config for the specified config resource. This maintains the versioning.
-      configStore.writeConfig(configResourceContext, getUserId(), emptyValue());
+      // delete the config for the specified config resource.
+      configStore.deleteConfigs(Set.of(configResourceContext));
       responseObserver.onNext(
           DeleteConfigResponse.newBuilder().setDeletedConfig(configToDelete).build());
       responseObserver.onCompleted();
@@ -155,39 +154,23 @@ public void deleteConfigs(
                 .asException());
         return;
       }
-      Map<ConfigResourceContext, Value> valuesByContext =
-          request.getConfigsList().stream()
-              .map(
-                  requestedDelete ->
-                      Map.entry(this.getConfigResourceContext(requestedDelete), emptyValue()))
-              .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue));
-
-      List<UpsertedConfig> deletedConfigs =
-          configStore.writeAllConfigs(valuesByContext, getUserId());
 
+      Set<ConfigResourceContext> configResourceContexts =
+          request.getConfigsList().stream()
+              .map(this::getConfigResourceContext)
+              .collect(Collectors.toUnmodifiableSet());
+      List<ContextSpecificConfig> configs = configStore.getConfigs(configResourceContexts);
+      // delete the configs for the specified config resources.
+      configStore.deleteConfigs(configResourceContexts);
       responseObserver.onNext(
-          DeleteConfigsResponse.newBuilder()
-              .addAllDeletedConfigs(
-                  deletedConfigs.stream()
-                      .map(this::buildDeletedContextSpecificConfig)
-                      .collect(Collectors.toUnmodifiableList()))
-              .build());
+          DeleteConfigsResponse.newBuilder().addAllDeletedConfigs(configs).build());
       responseObserver.onCompleted();
     } catch (Exception e) {
       log.error("Delete configs failed for request: {}", request, e);
       responseObserver.onError(e);
     }
   }
 
-  private ContextSpecificConfig buildDeletedContextSpecificConfig(UpsertedConfig deletedConfig) {
-    return ContextSpecificConfig.newBuilder()
-        .setContext(deletedConfig.getContext())
-        .setCreationTimestamp(deletedConfig.getCreationTimestamp())
-        .setUpdateTimestamp(deletedConfig.getUpdateTimestamp())
-        .setConfig(deletedConfig.getPrevConfig())
-        .build();
-  }
-
   @Override
   public void upsertAllConfigs(
       UpsertAllConfigsRequest request, StreamObserver<UpsertAllConfigsResponse> responseObserver) {

config-service-impl/src/main/java/org/hypertrace/config/service/store/ConfigStore.java

@@ -5,6 +5,7 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.hypertrace.config.service.ConfigResource;
 import org.hypertrace.config.service.ConfigResourceContext;
 import org.hypertrace.config.service.v1.ContextSpecificConfig;
@@ -42,6 +43,15 @@ UpsertedConfig writeConfig(
    */
   ContextSpecificConfig getConfig(ConfigResourceContext configResourceContext) throws IOException;
 
+  /**
+   * Get the configs with the latest version for the specified resources.
+   *
+   * @param configResourceContexts
+   * @return
+   */
+  List<ContextSpecificConfig> getConfigs(Set<ConfigResourceContext> configResourceContexts)
+      throws IOException;
+
   /**
    * Get all the configs with the latest version(along with the context to which it applies) for the
    * specified parameters, sorted in the descending order of their creation time.
@@ -63,6 +73,13 @@ UpsertedConfig writeConfig(
   List<UpsertedConfig> writeAllConfigs(
       Map<ConfigResourceContext, Value> resourceContextValueMap, String userId) throws IOException;
 
+  /**
+   * delete the config values associated with the specified config resources.
+   *
+   * @param configResourceContexts
+   */
+  void deleteConfigs(Set<ConfigResourceContext> configResourceContexts) throws IOException;
+
   /**
    * Health check for the backend store
    *

config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java

@@ -158,6 +158,14 @@
     return this.writeConfigs(resourceContextValueMap, userId);
   }
 
+  @Override
+  public void deleteConfigs(Set<ConfigResourceContext> configResourceContexts) {
+    if (configResourceContexts.isEmpty()) {
+      return;
+    }
+    collection.delete(getConfigResourceContextsFilter(configResourceContexts));
+  }
+
   @Override
   public ContextSpecificConfig getConfig(ConfigResourceContext configResourceContext)
       throws IOException {
@@ -166,6 +174,20 @@
         .orElseGet(() -> emptyConfig(configResourceContext.getContext()));
   }
 
+  @Override
+  public List<ContextSpecificConfig> getConfigs(Set<ConfigResourceContext> configResourceContexts)
+      throws IOException {
+    Map<ConfigResourceContext, Optional<ConfigDocument>> configDocs =
+        getLatestVersionConfigDocs(configResourceContexts);
+    return configDocs.values().stream()
+        .filter(Optional::isPresent)
+        .map(Optional::get)
+        .map(this::convertToContextSpecificConfig)
+        .filter(Optional::isPresent)
+        .map(Optional::get)
+        .collect(Collectors.toUnmodifiableList());
+  }
+
   @Override
   public List<ContextSpecificConfig> getAllConfigs(ConfigResource configResource)
       throws IOException {
@@ -216,20 +238,8 @@
     if (configResourceContexts.isEmpty()) {
       return Collections.emptyMap();
     }
-    // build filter
-    List<Filter> childFilters =
-        configResourceContexts.stream()
-            .map(this::getConfigResourceFieldContextFilter)
-            .collect(Collectors.toUnmodifiableList());
-    Filter configResourceFieldContextFilter = new Filter();
-    configResourceFieldContextFilter.setOp(Filter.Op.OR);
-    configResourceFieldContextFilter.setChildFilters(childFilters.toArray(Filter[]::new));
-    Filter tenantIdFilter =
-        Filter.eq(
-            TENANT_ID_FIELD_NAME,
-            configResourceContexts.iterator().next().getConfigResource().getTenantId());
-    Filter filter = tenantIdFilter.and(configResourceFieldContextFilter);
 
+    Filter filter = getConfigResourceContextsFilter(configResourceContexts);
     // build query
     Query query = new Query();
     query.setFilter(filter);
@@ -254,6 +264,22 @@
     return latestVersionConfigDocs;
   }
 
+  private Filter getConfigResourceContextsFilter(
+      Set<ConfigResourceContext> configResourceContexts) {
+    List<Filter> childFilters =
+        configResourceContexts.stream()
+            .map(this::getConfigResourceFieldContextFilter)
+            .collect(Collectors.toUnmodifiableList());
+    Filter configResourceFieldContextFilter = new Filter();
+    configResourceFieldContextFilter.setOp(Filter.Op.OR);
+    configResourceFieldContextFilter.setChildFilters(childFilters.toArray(Filter[]::new));
+    Filter tenantIdFilter =
+        Filter.eq(
+            TENANT_ID_FIELD_NAME,
+            configResourceContexts.iterator().next().getConfigResource().getTenantId());
+    return tenantIdFilter.and(configResourceFieldContextFilter);
+  }
+
   private Filter getConfigResourceContextFilter(ConfigResourceContext configResourceContext) {
     return this.getConfigResourceFilter(configResourceContext.getConfigResource())
         .and(Filter.eq(CONTEXT_FIELD_NAME, configResourceContext.getContext()));

config-service-impl/src/test/java/org/hypertrace/config/service/ConfigServiceGrpcImplTest.java

@@ -29,6 +29,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.hypertrace.config.service.store.ConfigStore;
 import org.hypertrace.config.service.v1.ContextSpecificConfig;
 import org.hypertrace.config.service.v1.DeleteConfigRequest;
@@ -193,8 +194,7 @@ void deleteConfig() throws IOException {
         () -> configServiceGrpc.deleteConfig(getDeleteConfigRequest(), responseObserver);
     RequestContext.forTenantId(TENANT_ID).run(runnable);
 
-    verify(configStore, times(1))
-        .writeConfig(eq(configResourceWithContext), eq(""), eq(emptyValue()));
+    verify(configStore, times(1)).deleteConfigs(eq(Set.of(configResourceWithContext)));
     verify(responseObserver, times(1))
         .onNext(eq(DeleteConfigResponse.newBuilder().setDeletedConfig(deletedConfig).build()));
     verify(responseObserver, times(1)).onCompleted();
@@ -222,15 +222,17 @@ void deleteConfigs() throws IOException {
             getConfigResourceContext(context2),
             emptyValue());
 
-    when(configStore.writeAllConfigs(writeAllConfigsRequest, ""))
+    when(configStore.getConfigs(writeAllConfigsRequest.keySet()))
         .thenReturn(
             List.of(
-                buildUpsertedConfig(context1, emptyValue(), config1, 10L, 20L),
-                buildUpsertedConfig(context2, emptyValue(), config2, 10L, 20L)));
+                buildContextSpecificConfig(context1, config1, 10L, 20L),
+                buildContextSpecificConfig(context2, config2, 10L, 20L)));
     Runnable runnable =
         () -> configServiceGrpc.deleteConfigs(deleteConfigsRequest, responseObserver);
     RequestContext.forTenantId(TENANT_ID).run(runnable);
-    verify(configStore, times(1)).writeAllConfigs(eq(writeAllConfigsRequest), eq(""));
+    verify(configStore, times(1))
+        .deleteConfigs(
+            eq(Set.of(getConfigResourceContext(context1), getConfigResourceContext(context2))));
     verify(responseObserver, times(1))
         .onNext(
             DeleteConfigsResponse.newBuilder()
@@ -265,8 +267,7 @@ void deleteDefaultContextConfig() throws IOException {
                 getDefaultContextDeleteConfigRequest(), responseObserver);
     RequestContext.forTenantId(TENANT_ID).run(runnable);
 
-    verify(configStore, times(1))
-        .writeConfig(eq(configResourceWithoutContext), eq(""), eq(emptyValue()));
+    verify(configStore, times(1)).deleteConfigs(eq(Set.of(configResourceWithoutContext)));
     verify(responseObserver, times(1))
         .onNext(eq(DeleteConfigResponse.newBuilder().setDeletedConfig(deletedConfig).build()));
     verify(responseObserver, times(1)).onCompleted();

config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java

@@ -107,6 +107,26 @@ void writeConfig() throws IOException {
         document);
   }
 
+  @Test
+  void getConfigs() throws IOException {
+    CloseableIteratorImpl iterator =
+        new CloseableIteratorImpl(
+            Collections.singletonList(
+                getConfigDocument("context", CONFIG_VERSION, config1, TIMESTAMP1, TIMESTAMP2)));
+    when(collection.search(any(Query.class))).thenReturn(iterator);
+
+    ContextSpecificConfig expectedConfig =
+        ContextSpecificConfig.newBuilder()
+            .setConfig(config1)
+            .setContext("context")
+            .setCreationTimestamp(TIMESTAMP1)
+            .setUpdateTimestamp(TIMESTAMP2)
+            .build();
+    List<ContextSpecificConfig> actualConfigs =
+        configStore.getConfigs(Set.of(configResourceContext));
+    assertEquals(List.of(expectedConfig), actualConfigs);
+  }
+
   @Test
   void getConfig() throws IOException {
     CloseableIteratorImpl iterator =

config-service/src/integrationTest/java/org/hypertrace/config/service/ConfigServiceIntegrationTest.java

@@ -286,10 +286,6 @@ void testDeleteConfigs() {
             .getDeletedConfigsList();
 
     assertEquals(2, deletedContextSpecificConfigs.size());
-    assertEquals(CONTEXT_1, deletedContextSpecificConfigs.get(0).getContext());
-    assertEquals(config1, deletedContextSpecificConfigs.get(0).getConfig());
-    assertEquals(CONTEXT_2, deletedContextSpecificConfigs.get(1).getContext());
-    assertEquals(config2, deletedContextSpecificConfigs.get(1).getConfig());
 
     // There should be no configs for RESOURCE_NAME_1 and RESOURCE_NAMESPACE_1 for TENANT_1
     List<ContextSpecificConfig> contextSpecificConfigList =

gradle/libs.versions.toml

@@ -2,9 +2,9 @@
 protoc = "3.23.2"
 grpc = "1.57.2"
 gson = "2.9.0"
-hypertrace-grpcutils = "0.12.2"
-hypertrace-framework = "0.1.58"
-kafka-streams-framework = "0.3.2"
+hypertrace-grpcutils = "0.12.5"
+hypertrace-framework = "0.1.61"
+kafka-streams-framework = "0.3.9"
 lombok = "1.18.28"
 jackson = "2.15.2"
 

owasp-suppressions.xml

@@ -10,4 +10,16 @@
         <cpe>cpe:/a:utils_project:utils</cpe>
         <cpe>cpe:/a:processing:processing</cpe>
     </suppress>
+    <suppress until="2023-11-30Z">
+        <notes><![CDATA[
+  This vulnerability is disputed, with the argument that SSL configuration is the responsibility of the client rather
+  than the transport. The change in default is under consideration for the next major Netty release, revisit then.
+  Regardless, our client (which is what brings in this dependency) enables the concerned feature, hostname verification
+  Ref:
+  https://github.com/grpc/grpc-java/issues/10033
+  https://github.com/netty/netty/issues/8537#issuecomment-1527896917
+   ]]></notes>
+        <packageUrl regex="true">^pkg:maven/io\.netty/netty.*@.*$</packageUrl>
+        <vulnerabilityName>CVE-2023-4586</vulnerabilityName>
+    </suppress>
 </suppressions>