diff --git a/buf.work.yaml b/buf.work.yaml index 54b3825b..2039b590 100644 --- a/buf.work.yaml +++ b/buf.work.yaml @@ -3,6 +3,7 @@ directories: - alerting-config-service-api/src/main/proto - config-service-api/src/main/proto - label-application-rule-config-service-api/src/main/proto + - label-application-rule-config-service-impl/src/main/proto - labels-config-service-api/src/main/proto - notification-channel-config-service-api/src/main/proto - notification-rule-config-service-api/src/main/proto diff --git a/config-service/src/main/resources/configs/common/application.conf b/config-service/src/main/resources/configs/common/application.conf index 54f49341..7366b659 100644 --- a/config-service/src/main/resources/configs/common/application.conf +++ b/config-service/src/main/resources/configs/common/application.conf @@ -26,3 +26,8 @@ event.store { schema.registry.url = "http://localhost:8081" } } + +label.application.rule.config.service { + max.dynamic.label.application.rules.per.tenant = 100 + system.label.application.rules = [] +} diff --git a/label-application-rule-config-service-impl/build.gradle.kts b/label-application-rule-config-service-impl/build.gradle.kts index 5947a3b3..18c373c3 100644 --- a/label-application-rule-config-service-impl/build.gradle.kts +++ b/label-application-rule-config-service-impl/build.gradle.kts @@ -1,9 +1,16 @@ plugins { `java-library` + alias(commonLibs.plugins.google.protobuf) jacoco alias(commonLibs.plugins.hypertrace.jacoco) } +protobuf { + protoc { + artifact = "com.google.protobuf:protoc:${commonLibs.versions.protoc.get()}" + } +} + dependencies { api(projects.labelApplicationRuleConfigServiceApi) implementation(projects.configServiceApi) diff --git a/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/DeletedSystemLabelApplicationRuleStore.java b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/DeletedSystemLabelApplicationRuleStore.java new file mode 100644 index 00000000..44cf744e --- /dev/null +++ b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/DeletedSystemLabelApplicationRuleStore.java @@ -0,0 +1,50 @@ +package org.hypertrace.label.application.rule.config.service; + +import com.google.protobuf.Value; +import java.util.Optional; +import lombok.SneakyThrows; +import org.hypertrace.config.objectstore.IdentifiedObjectStore; +import org.hypertrace.config.proto.converter.ConfigProtoConverter; +import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator; +import org.hypertrace.config.service.v1.ConfigServiceGrpc.ConfigServiceBlockingStub; +import org.hypertrace.label.application.rule.config.service.impl.v1.DeletedSystemLabelApplicationRule; + +class DeletedSystemLabelApplicationRuleStore + extends IdentifiedObjectStore { + + private static final String DELETED_SYSTEM_LABEL_APPLICATION_RULE_CONFIG_RESOURCE_NAME = + "deleted-system-label-application-rule-config"; + private static final String LABEL_APPLICATION_RULE_CONFIG_RESOURCE_NAMESPACE = "labels"; + + DeletedSystemLabelApplicationRuleStore( + ConfigServiceBlockingStub stub, ConfigChangeEventGenerator configChangeEventGenerator) { + super( + stub, + LABEL_APPLICATION_RULE_CONFIG_RESOURCE_NAMESPACE, + DELETED_SYSTEM_LABEL_APPLICATION_RULE_CONFIG_RESOURCE_NAME, + configChangeEventGenerator); + } + + @Override + protected Optional buildDataFromValue(Value value) { + try { + DeletedSystemLabelApplicationRule.Builder builder = + DeletedSystemLabelApplicationRule.newBuilder(); + ConfigProtoConverter.mergeFromValue(value, builder); + return Optional.of(builder.build()); + } catch (Exception e) { + return Optional.empty(); + } + } + + @SneakyThrows + @Override + protected Value buildValueFromData(DeletedSystemLabelApplicationRule object) { + return ConfigProtoConverter.convertToValue(object); + } + + @Override + protected String getContextFromData(DeletedSystemLabelApplicationRule object) { + return object.getId(); + } +} diff --git a/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfig.java b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfig.java index 9851e4a2..dcb7e720 100644 --- a/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfig.java +++ b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfig.java @@ -9,6 +9,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import lombok.Getter; import lombok.SneakyThrows; @@ -24,7 +25,7 @@ public class LabelApplicationRuleConfig { @Getter private final int maxDynamicLabelApplicationRulesAllowed; @Getter private final List systemLabelApplicationRules; - @Getter private final Map systemLabelApplicationRulesMap; + private final Map systemLabelApplicationRulesMap; public LabelApplicationRuleConfig(Config config) { Config labelApplicationRuleConfig = @@ -64,4 +65,8 @@ private static LabelApplicationRule buildLabelApplicationRuleFromConfig( JsonFormat.parser().merge(jsonString, builder); return builder.build(); } + + public Optional getSystemLabelApplicationRule(String id) { + return Optional.ofNullable(systemLabelApplicationRulesMap.get(id)); + } } diff --git a/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigServiceImpl.java b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigServiceImpl.java index b944f994..c4a2fcf1 100644 --- a/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigServiceImpl.java +++ b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigServiceImpl.java @@ -2,7 +2,6 @@ import io.grpc.Channel; import io.grpc.Status; -import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import java.util.List; import java.util.Optional; @@ -16,6 +15,7 @@ import org.hypertrace.config.service.v1.ConfigServiceGrpc.ConfigServiceBlockingStub; import org.hypertrace.core.grpcutils.client.RequestContextClientCallCredsProviderFactory; import org.hypertrace.core.grpcutils.context.RequestContext; +import org.hypertrace.label.application.rule.config.service.impl.v1.DeletedSystemLabelApplicationRule; import org.hypertrace.label.application.rule.config.service.v1.CreateLabelApplicationRuleRequest; import org.hypertrace.label.application.rule.config.service.v1.CreateLabelApplicationRuleResponse; import org.hypertrace.label.application.rule.config.service.v1.DeleteLabelApplicationRuleRequest; @@ -30,6 +30,8 @@ public class LabelApplicationRuleConfigServiceImpl extends LabelApplicationRuleConfigServiceGrpc.LabelApplicationRuleConfigServiceImplBase { private final IdentifiedObjectStore labelApplicationRuleStore; + private final IdentifiedObjectStore + deletedSystemLabelApplicationRuleStore; private final LabelApplicationRuleValidator requestValidator; private final LabelApplicationRuleConfig labelApplicationRuleConfig; @@ -45,6 +47,9 @@ public LabelApplicationRuleConfigServiceImpl( RequestContextClientCallCredsProviderFactory.getClientCallCredsProvider().get()); this.labelApplicationRuleStore = new LabelApplicationRuleStore(configServiceBlockingStub, configChangeEventGenerator); + this.deletedSystemLabelApplicationRuleStore = + new DeletedSystemLabelApplicationRuleStore( + configServiceBlockingStub, configChangeEventGenerator); this.requestValidator = new LabelApplicationRuleValidatorImpl(); } @@ -90,9 +95,12 @@ public void getLabelApplicationRules( labelApplicationRules.stream() .map(LabelApplicationRule::getId) .collect(Collectors.toUnmodifiableSet()); + Set deletedSystemLabelApplicationRuleIds = + getDeletedSystemLabelApplicationRuleIds(requestContext); List filteredSystemLabelApplicationRules = this.labelApplicationRuleConfig.getSystemLabelApplicationRules().stream() .filter(rule -> !labelApplicationRuleIds.contains(rule.getId())) + .filter(rule -> !deletedSystemLabelApplicationRuleIds.contains(rule.getId())) .collect(Collectors.toUnmodifiableList()); responseObserver.onNext( GetLabelApplicationRulesResponse.newBuilder() @@ -115,12 +123,7 @@ public void updateLabelApplicationRule( LabelApplicationRule existingRule = this.labelApplicationRuleStore .getData(requestContext, request.getId()) - .or( - () -> - Optional.ofNullable( - this.labelApplicationRuleConfig - .getSystemLabelApplicationRulesMap() - .get(request.getId()))) + .or(() -> getSystemLabelApplicationRule(requestContext, request.getId())) .orElseThrow(Status.NOT_FOUND::asRuntimeException); LabelApplicationRule updateLabelApplicationRule = existingRule.toBuilder().setData(request.getData()).build(); @@ -146,18 +149,20 @@ public void deleteLabelApplicationRule( RequestContext requestContext = RequestContext.CURRENT.get(); this.requestValidator.validateOrThrow(requestContext, request); String labelApplicationRuleId = request.getId(); - if (this.labelApplicationRuleConfig - .getSystemLabelApplicationRulesMap() - .containsKey(labelApplicationRuleId)) { - // Deleting a system label application rule is not allowed - responseObserver.onError(new StatusRuntimeException(Status.INVALID_ARGUMENT)); - return; + boolean customRuleDeleted = + this.labelApplicationRuleStore.deleteObject(requestContext, request.getId()).isPresent(); + Optional systemLabelApplicationRule = + getSystemLabelApplicationRule(requestContext, labelApplicationRuleId); + if (systemLabelApplicationRule.isPresent()) { + deletedSystemLabelApplicationRuleStore.upsertObject( + requestContext, + DeletedSystemLabelApplicationRule.newBuilder().setId(labelApplicationRuleId).build()); } - this.labelApplicationRuleStore - .deleteObject(requestContext, request.getId()) - .orElseThrow(Status.NOT_FOUND::asRuntimeException); - responseObserver.onNext(DeleteLabelApplicationRuleResponse.getDefaultInstance()); - responseObserver.onCompleted(); + if (customRuleDeleted || systemLabelApplicationRule.isPresent()) { + responseObserver.onNext(DeleteLabelApplicationRuleResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + throw Status.NOT_FOUND.asRuntimeException(); } catch (Exception e) { responseObserver.onError(e); } @@ -180,4 +185,20 @@ private void checkRequestForDynamicLabelsLimit( } } } + + private Set getDeletedSystemLabelApplicationRuleIds(RequestContext requestContext) { + return this.deletedSystemLabelApplicationRuleStore.getAllObjects(requestContext).stream() + .map(ConfigObject::getData) + .map(DeletedSystemLabelApplicationRule::getId) + .collect(Collectors.toUnmodifiableSet()); + } + + private Optional getSystemLabelApplicationRule( + RequestContext requestContext, String id) { + return this.labelApplicationRuleConfig + .getSystemLabelApplicationRule(id) + .filter( + unused -> + this.deletedSystemLabelApplicationRuleStore.getData(requestContext, id).isEmpty()); + } } diff --git a/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleStore.java b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleStore.java index 0e44a8f1..d3bf8c02 100644 --- a/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleStore.java +++ b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleStore.java @@ -9,7 +9,7 @@ import org.hypertrace.config.service.v1.ConfigServiceGrpc; import org.hypertrace.label.application.rule.config.service.v1.LabelApplicationRule; -public class LabelApplicationRuleStore extends IdentifiedObjectStore { +class LabelApplicationRuleStore extends IdentifiedObjectStore { private static final String LABEL_APPLICATION_RULE_CONFIG_RESOURCE_NAME = "label-application-rule-config"; private static final String LABEL_APPLICATION_RULE_CONFIG_RESOURCE_NAMESPACE = "labels"; diff --git a/label-application-rule-config-service-impl/src/main/proto/buf.yaml b/label-application-rule-config-service-impl/src/main/proto/buf.yaml new file mode 100644 index 00000000..d2e630f9 --- /dev/null +++ b/label-application-rule-config-service-impl/src/main/proto/buf.yaml @@ -0,0 +1,8 @@ +version: v1 +breaking: + use: + - PACKAGE + - WIRE_JSON +lint: + use: + - DEFAULT diff --git a/label-application-rule-config-service-impl/src/main/proto/org/hypertrace/label/application/rule/config/service/impl/v1/deleted_system_label_application_rule.proto b/label-application-rule-config-service-impl/src/main/proto/org/hypertrace/label/application/rule/config/service/impl/v1/deleted_system_label_application_rule.proto new file mode 100644 index 00000000..f037164a --- /dev/null +++ b/label-application-rule-config-service-impl/src/main/proto/org/hypertrace/label/application/rule/config/service/impl/v1/deleted_system_label_application_rule.proto @@ -0,0 +1,9 @@ +syntax="proto3"; + +package org.hypertrace.label.application.rule.config.service.impl.v1; + +option java_multiple_files = true; + +message DeletedSystemLabelApplicationRule { + string id = 1; +} diff --git a/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigServiceImplTest.java b/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigServiceImplTest.java index 00d4bb3b..d66fd607 100644 --- a/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigServiceImplTest.java +++ b/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigServiceImplTest.java @@ -1,5 +1,6 @@ package org.hypertrace.label.application.rule.config.service; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -14,6 +15,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator; @@ -59,8 +61,8 @@ void setUp() throws InvalidProtocolBufferException { .build(); LabelApplicationRuleConfig config = mock(LabelApplicationRuleConfig.class); when(config.getSystemLabelApplicationRules()).thenReturn(List.of(systemLabelApplicationRule)); - when(config.getSystemLabelApplicationRulesMap()) - .thenReturn(Map.of(systemLabelApplicationRule.getId(), systemLabelApplicationRule)); + when(config.getSystemLabelApplicationRule(systemLabelApplicationRule.getId())) + .thenReturn(Optional.of(systemLabelApplicationRule)); when(config.getMaxDynamicLabelApplicationRulesAllowed()).thenReturn(2); mockGenericConfigService .addService( @@ -204,13 +206,66 @@ void deleteSystemApplicationRuleError() { DeleteLabelApplicationRuleRequest.newBuilder() .setId(systemLabelApplicationRule.getId()) .build(); + assertDoesNotThrow( + () -> labelApplicationRuleConfigServiceBlockingStub.deleteLabelApplicationRule(request)); + // deleting an already deleted system label application rule should throw error Throwable exception = assertThrows( StatusRuntimeException.class, - () -> { - labelApplicationRuleConfigServiceBlockingStub.deleteLabelApplicationRule(request); - }); - assertEquals(Status.INVALID_ARGUMENT, Status.fromThrowable(exception)); + () -> + labelApplicationRuleConfigServiceBlockingStub.deleteLabelApplicationRule(request)); + assertEquals(Status.NOT_FOUND, Status.fromThrowable(exception)); + } + + @Test + void updateDeletedSystemLabelApplicationRule() { + DeleteLabelApplicationRuleRequest request = + DeleteLabelApplicationRuleRequest.newBuilder() + .setId(systemLabelApplicationRule.getId()) + .build(); + assertDoesNotThrow( + () -> labelApplicationRuleConfigServiceBlockingStub.deleteLabelApplicationRule(request)); + // updating an already deleted system label application rule should throw error + LabelApplicationRuleData expectedData = buildSimpleRuleData("auth", "not-valid"); + UpdateLabelApplicationRuleRequest updateRequest = + UpdateLabelApplicationRuleRequest.newBuilder() + .setId(systemLabelApplicationRule.getId()) + .setData(expectedData) + .build(); + Throwable exception = + assertThrows( + StatusRuntimeException.class, + () -> + labelApplicationRuleConfigServiceBlockingStub.updateLabelApplicationRule( + updateRequest)); + assertEquals(Status.NOT_FOUND, Status.fromThrowable(exception)); + } + + @Test + void getLabelApplicationRulesAfterDeletingSystemLabelApplicationRule() { + LabelApplicationRule simpleRule = createSimpleRule("auth", "valid"); + LabelApplicationRule compositeRule = createCompositeRule(); + Set expectedRules = + Set.of(simpleRule, compositeRule, systemLabelApplicationRule); + GetLabelApplicationRulesResponse response = + labelApplicationRuleConfigServiceBlockingStub.getLabelApplicationRules( + GetLabelApplicationRulesRequest.getDefaultInstance()); + assertEquals( + expectedRules, + response.getLabelApplicationRulesList().stream().collect(Collectors.toUnmodifiableSet())); + DeleteLabelApplicationRuleRequest request = + DeleteLabelApplicationRuleRequest.newBuilder() + .setId(systemLabelApplicationRule.getId()) + .build(); + assertDoesNotThrow( + () -> labelApplicationRuleConfigServiceBlockingStub.deleteLabelApplicationRule(request)); + expectedRules = Set.of(simpleRule, compositeRule); + response = + labelApplicationRuleConfigServiceBlockingStub.getLabelApplicationRules( + GetLabelApplicationRulesRequest.getDefaultInstance()); + assertEquals( + expectedRules, + response.getLabelApplicationRulesList().stream().collect(Collectors.toUnmodifiableSet())); } private LabelApplicationRuleData buildCompositeRuleData() { diff --git a/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigTest.java b/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigTest.java index 88d1eba3..f16d32cf 100644 --- a/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigTest.java +++ b/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigTest.java @@ -1,6 +1,7 @@ package org.hypertrace.label.application.rule.config.service; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.util.JsonFormat; @@ -9,6 +10,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import org.hypertrace.label.application.rule.config.service.v1.LabelApplicationRule; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -53,8 +55,12 @@ void test_getSystemLabelApplicationRules() { @Test void test_getSystemLabelApplicationRulesMap() { - assertEquals( - stringLabelApplicationRuleMap, - labelApplicationRuleConfig.getSystemLabelApplicationRulesMap()); + Optional rule = + labelApplicationRuleConfig.getSystemLabelApplicationRule( + systemLabelApplicationRule.getId()); + assertTrue(rule.isPresent()); + assertEquals(systemLabelApplicationRule, rule.get()); + assertTrue( + labelApplicationRuleConfig.getSystemLabelApplicationRule("id-does-not-exist").isEmpty()); } }