From d6d3c34ae9d24bbe68444fb28391a5f96f5376ab Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Fri, 30 Aug 2024 23:55:28 +0530 Subject: [PATCH 1/5] chore: add support for system label application rules --- ...LabelApplicationRuleConfigServiceImpl.java | 83 +++++++++++++++---- ...lApplicationRuleConfigServiceImplTest.java | 57 +++++++++++-- 2 files changed, 117 insertions(+), 23 deletions(-) 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 321bf4b3..e5da9d7b 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 @@ -1,12 +1,22 @@ package org.hypertrace.label.application.rule.config.service; +import static java.util.function.Function.identity; + +import com.google.protobuf.util.JsonFormat; import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; import io.grpc.Channel; import io.grpc.Status; +import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; +import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; +import lombok.SneakyThrows; import org.hypertrace.config.objectstore.ConfigObject; import org.hypertrace.config.objectstore.IdentifiedObjectStore; import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator; @@ -27,29 +37,42 @@ public class LabelApplicationRuleConfigServiceImpl extends LabelApplicationRuleConfigServiceGrpc.LabelApplicationRuleConfigServiceImplBase { - static final String LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG = + private static final String LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG = "label.application.rule.config.service"; - static final String MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT = + private static final String MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT = "max.dynamic.label.application.rules.per.tenant"; - static final int DEFAULT_MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT = 100; + private static final String SYSTEM_LABEL_APPLICATION_RULES = "system.label.application.rules"; + private static final int DEFAULT_MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT = 100; + private final IdentifiedObjectStore labelApplicationRuleStore; private final LabelApplicationRuleValidator requestValidator; private final int maxDynamicLabelApplicationRulesAllowed; + private final List systemLabelApplicationRules; + private final Map systemLabelApplicationRulesMap; public LabelApplicationRuleConfigServiceImpl( Channel configChannel, Config config, ConfigChangeEventGenerator configChangeEventGenerator) { - int maxDynamicRules = DEFAULT_MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT; - if (config.hasPath(LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG)) { - Config labelApplicationRuleConfig = - config.getConfig(LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG); - if (labelApplicationRuleConfig.hasPath(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT)) { - maxDynamicRules = - labelApplicationRuleConfig.getInt(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT); - } + Config labelApplicationRuleConfig = + config.hasPath(LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG) + ? config.getConfig(LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG) + : ConfigFactory.empty(); + this.maxDynamicLabelApplicationRulesAllowed = + labelApplicationRuleConfig.hasPath(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT) + ? labelApplicationRuleConfig.getInt(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT) + : DEFAULT_MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT; + if (labelApplicationRuleConfig.hasPath(SYSTEM_LABEL_APPLICATION_RULES)) { + final List systemLabelApplicationRulesObjectList = + labelApplicationRuleConfig.getObjectList(SYSTEM_LABEL_APPLICATION_RULES); + this.systemLabelApplicationRules = + buildSystemLabelApplicationRuleList(systemLabelApplicationRulesObjectList); + this.systemLabelApplicationRulesMap = + this.systemLabelApplicationRules.stream() + .collect(Collectors.toUnmodifiableMap(LabelApplicationRule::getId, identity())); + } else { + this.systemLabelApplicationRules = Collections.emptyList(); + this.systemLabelApplicationRulesMap = Collections.emptyMap(); } - this.maxDynamicLabelApplicationRulesAllowed = maxDynamicRules; - - ConfigServiceBlockingStub configServiceBlockingStub = + final ConfigServiceBlockingStub configServiceBlockingStub = ConfigServiceGrpc.newBlockingStub(configChannel) .withCallCredentials( RequestContextClientCallCredsProviderFactory.getClientCallCredsProvider().get()); @@ -96,9 +119,18 @@ public void getLabelApplicationRules( this.labelApplicationRuleStore.getAllObjects(requestContext).stream() .map(ConfigObject::getData) .collect(Collectors.toUnmodifiableList()); + Set labelApplicationRuleIds = + labelApplicationRules.stream() + .map(LabelApplicationRule::getId) + .collect(Collectors.toUnmodifiableSet()); + List filteredSystemLabelApplicationRules = + systemLabelApplicationRules.stream() + .filter(rule -> !labelApplicationRuleIds.contains(rule.getId())) + .collect(Collectors.toUnmodifiableList()); responseObserver.onNext( GetLabelApplicationRulesResponse.newBuilder() .addAllLabelApplicationRules(labelApplicationRules) + .addAllLabelApplicationRules(filteredSystemLabelApplicationRules) .build()); responseObserver.onCompleted(); } catch (Exception e) { @@ -116,6 +148,7 @@ public void updateLabelApplicationRule( LabelApplicationRule existingRule = this.labelApplicationRuleStore .getData(requestContext, request.getId()) + .or(() -> Optional.ofNullable(systemLabelApplicationRulesMap.get(request.getId()))) .orElseThrow(Status.NOT_FOUND::asRuntimeException); LabelApplicationRule updateLabelApplicationRule = existingRule.toBuilder().setData(request.getData()).build(); @@ -140,6 +173,12 @@ public void deleteLabelApplicationRule( try { RequestContext requestContext = RequestContext.CURRENT.get(); this.requestValidator.validateOrThrow(requestContext, request); + String labelApplicationRuleId = request.getId(); + if (systemLabelApplicationRulesMap.containsKey(labelApplicationRuleId)) { + // Deleting a system label application rule is not allowed + responseObserver.onError(new StatusRuntimeException(Status.INVALID_ARGUMENT)); + return; + } this.labelApplicationRuleStore .deleteObject(requestContext, request.getId()) .orElseThrow(Status.NOT_FOUND::asRuntimeException); @@ -166,4 +205,20 @@ private void checkRequestForDynamicLabelsLimit( } } } + + private List buildSystemLabelApplicationRuleList( + List configObjectList) { + return configObjectList.stream() + .map(LabelApplicationRuleConfigServiceImpl::buildLabelApplicationRuleFromConfig) + .collect(Collectors.toUnmodifiableList()); + } + + @SneakyThrows + private static LabelApplicationRule buildLabelApplicationRuleFromConfig( + com.typesafe.config.ConfigObject configObject) { + String jsonString = configObject.render(); + LabelApplicationRule.Builder builder = LabelApplicationRule.newBuilder(); + JsonFormat.parser().merge(jsonString, builder); + return builder.build(); + } } 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 d3e8dac7..a4385a99 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,12 +1,12 @@ package org.hypertrace.label.application.rule.config.service; -import static org.hypertrace.label.application.rule.config.service.LabelApplicationRuleConfigServiceImpl.LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG; -import static org.hypertrace.label.application.rule.config.service.LabelApplicationRuleConfigServiceImpl.MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.util.JsonFormat; import com.typesafe.config.Config; import com.typesafe.config.ConfigFactory; import io.grpc.Channel; @@ -44,20 +44,29 @@ import org.junit.jupiter.api.Test; public class LabelApplicationRuleConfigServiceImplTest { + private static final String SYSTEM_LABEL_APPLICATION_RULE_STR = + "{\"id\":\"system-label-application-rule-1\",\"data\":{\"name\":\"SystemLabelApplicationRule1\",\"matching_condition\":{\"leaf_condition\":{\"key_condition\":{\"operator\":\"OPERATOR_EQUALS\",\"value\":\"test.key\"},\"unary_condition\":{\"operator\":\"OPERATOR_EXISTS\"}}},\"label_action\":{\"entity_types\":[\"API\"],\"operation\":\"OPERATION_MERGE\",\"static_labels\":{\"ids\":[\"static-label-id-1\"]}},\"enabled\":false}}"; MockGenericConfigService mockGenericConfigService; LabelApplicationRuleConfigServiceBlockingStub labelApplicationRuleConfigServiceBlockingStub; + LabelApplicationRule systemLabelApplicationRule; @BeforeEach - void setUp() { + void setUp() throws InvalidProtocolBufferException { mockGenericConfigService = new MockGenericConfigService().mockUpsert().mockGet().mockGetAll().mockDelete(); Channel channel = mockGenericConfigService.channel(); ConfigChangeEventGenerator configChangeEventGenerator = mock(ConfigChangeEventGenerator.class); - Config config = - ConfigFactory.parseMap( - Map.of( - LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG, - Map.of(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT, 2))); + String configStr = + "label.application.rule.config.service {\n" + + "max.dynamic.label.application.rules.per.tenant = 2\n" + + "system.label.application.rules = [\n" + + SYSTEM_LABEL_APPLICATION_RULE_STR + + "\n]\n" + + "}\n"; + Config config = ConfigFactory.parseString(configStr); + LabelApplicationRule.Builder builder = LabelApplicationRule.newBuilder().clear(); + JsonFormat.parser().merge(SYSTEM_LABEL_APPLICATION_RULE_STR, builder); + systemLabelApplicationRule = builder.build(); mockGenericConfigService .addService( new LabelApplicationRuleConfigServiceImpl(channel, config, configChangeEventGenerator)) @@ -109,7 +118,8 @@ void createLabelApplicationRuleWithDynamicLabelApplicationRulesLimitReached() { void getLabelApplicationRules() { LabelApplicationRule simpleRule = createSimpleRule("auth", "valid"); LabelApplicationRule compositeRule = createCompositeRule(); - Set expectedRules = Set.of(simpleRule, compositeRule); + Set expectedRules = + Set.of(simpleRule, compositeRule, systemLabelApplicationRule); GetLabelApplicationRulesResponse response = labelApplicationRuleConfigServiceBlockingStub.getLabelApplicationRules( GetLabelApplicationRulesRequest.getDefaultInstance()); @@ -133,6 +143,20 @@ void updateLabelApplicationRule() { assertEquals(expectedData, response.getLabelApplicationRule().getData()); } + @Test + void updateSystemLabelApplicationRule() { + LabelApplicationRuleData expectedData = buildSimpleRuleData("auth", "not-valid"); + UpdateLabelApplicationRuleRequest request = + UpdateLabelApplicationRuleRequest.newBuilder() + .setId(systemLabelApplicationRule.getId()) + .setData(expectedData) + .build(); + UpdateLabelApplicationRuleResponse response = + labelApplicationRuleConfigServiceBlockingStub.updateLabelApplicationRule(request); + assertEquals(systemLabelApplicationRule.getId(), response.getLabelApplicationRule().getId()); + assertEquals(expectedData, response.getLabelApplicationRule().getData()); + } + @Test void updateLabelApplicationRuleError() { LabelApplicationRule simpleRule = createSimpleRule("auth", "valid"); @@ -179,6 +203,21 @@ void deleteApplicationRuleError() { assertEquals(Status.NOT_FOUND, Status.fromThrowable(exception)); } + @Test + void deleteSystemApplicationRuleError() { + DeleteLabelApplicationRuleRequest request = + DeleteLabelApplicationRuleRequest.newBuilder() + .setId(systemLabelApplicationRule.getId()) + .build(); + Throwable exception = + assertThrows( + StatusRuntimeException.class, + () -> { + labelApplicationRuleConfigServiceBlockingStub.deleteLabelApplicationRule(request); + }); + assertEquals(Status.INVALID_ARGUMENT, Status.fromThrowable(exception)); + } + private LabelApplicationRuleData buildCompositeRuleData() { // This condition implies foo(key) exists AND foo(key) = bar(value) AND // req.http.headers.auth(key) = valid(value) From 740ffd7fbaf099fd14084766dd5e49ecc4219411 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Sat, 31 Aug 2024 00:35:32 +0530 Subject: [PATCH 2/5] address review comment --- .../service/LabelApplicationRuleConfig.java | 67 +++++++++++++++++ ...LabelApplicationRuleConfigServiceImpl.java | 72 ++++--------------- 2 files changed, 82 insertions(+), 57 deletions(-) create mode 100644 label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfig.java 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 new file mode 100644 index 00000000..9851e4a2 --- /dev/null +++ b/label-application-rule-config-service-impl/src/main/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfig.java @@ -0,0 +1,67 @@ +package org.hypertrace.label.application.rule.config.service; + +import static java.util.function.Function.identity; + +import com.google.protobuf.util.JsonFormat; +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigObject; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import lombok.Getter; +import lombok.SneakyThrows; +import org.hypertrace.label.application.rule.config.service.v1.LabelApplicationRule; + +public class LabelApplicationRuleConfig { + private static final String LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG = + "label.application.rule.config.service"; + private static final String MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT = + "max.dynamic.label.application.rules.per.tenant"; + private static final String SYSTEM_LABEL_APPLICATION_RULES = "system.label.application.rules"; + private static final int DEFAULT_MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT = 100; + + @Getter private final int maxDynamicLabelApplicationRulesAllowed; + @Getter private final List systemLabelApplicationRules; + @Getter private final Map systemLabelApplicationRulesMap; + + public LabelApplicationRuleConfig(Config config) { + Config labelApplicationRuleConfig = + config.hasPath(LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG) + ? config.getConfig(LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG) + : ConfigFactory.empty(); + this.maxDynamicLabelApplicationRulesAllowed = + labelApplicationRuleConfig.hasPath(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT) + ? labelApplicationRuleConfig.getInt(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT) + : DEFAULT_MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT; + if (labelApplicationRuleConfig.hasPath(SYSTEM_LABEL_APPLICATION_RULES)) { + final List systemLabelApplicationRulesObjectList = + labelApplicationRuleConfig.getObjectList(SYSTEM_LABEL_APPLICATION_RULES); + this.systemLabelApplicationRules = + buildSystemLabelApplicationRuleList(systemLabelApplicationRulesObjectList); + this.systemLabelApplicationRulesMap = + this.systemLabelApplicationRules.stream() + .collect(Collectors.toUnmodifiableMap(LabelApplicationRule::getId, identity())); + } else { + this.systemLabelApplicationRules = Collections.emptyList(); + this.systemLabelApplicationRulesMap = Collections.emptyMap(); + } + } + + private List buildSystemLabelApplicationRuleList( + List configObjectList) { + return configObjectList.stream() + .map(LabelApplicationRuleConfig::buildLabelApplicationRuleFromConfig) + .collect(Collectors.toUnmodifiableList()); + } + + @SneakyThrows + private static LabelApplicationRule buildLabelApplicationRuleFromConfig( + com.typesafe.config.ConfigObject configObject) { + String jsonString = configObject.render(); + LabelApplicationRule.Builder builder = LabelApplicationRule.newBuilder(); + JsonFormat.parser().merge(jsonString, builder); + return builder.build(); + } +} 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 e5da9d7b..4420ade1 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 @@ -1,22 +1,15 @@ package org.hypertrace.label.application.rule.config.service; -import static java.util.function.Function.identity; - -import com.google.protobuf.util.JsonFormat; import com.typesafe.config.Config; -import com.typesafe.config.ConfigFactory; import io.grpc.Channel; import io.grpc.Status; import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; -import lombok.SneakyThrows; import org.hypertrace.config.objectstore.ConfigObject; import org.hypertrace.config.objectstore.IdentifiedObjectStore; import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator; @@ -37,41 +30,14 @@ public class LabelApplicationRuleConfigServiceImpl extends LabelApplicationRuleConfigServiceGrpc.LabelApplicationRuleConfigServiceImplBase { - private static final String LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG = - "label.application.rule.config.service"; - private static final String MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT = - "max.dynamic.label.application.rules.per.tenant"; - private static final String SYSTEM_LABEL_APPLICATION_RULES = "system.label.application.rules"; - private static final int DEFAULT_MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT = 100; - private final IdentifiedObjectStore labelApplicationRuleStore; private final LabelApplicationRuleValidator requestValidator; - private final int maxDynamicLabelApplicationRulesAllowed; - private final List systemLabelApplicationRules; - private final Map systemLabelApplicationRulesMap; + private final LabelApplicationRuleConfig labelApplicationRuleConfig; public LabelApplicationRuleConfigServiceImpl( Channel configChannel, Config config, ConfigChangeEventGenerator configChangeEventGenerator) { - Config labelApplicationRuleConfig = - config.hasPath(LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG) - ? config.getConfig(LABEL_APPLICATION_RULE_CONFIG_SERVICE_CONFIG) - : ConfigFactory.empty(); - this.maxDynamicLabelApplicationRulesAllowed = - labelApplicationRuleConfig.hasPath(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT) - ? labelApplicationRuleConfig.getInt(MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT) - : DEFAULT_MAX_DYNAMIC_LABEL_APPLICATION_RULES_PER_TENANT; - if (labelApplicationRuleConfig.hasPath(SYSTEM_LABEL_APPLICATION_RULES)) { - final List systemLabelApplicationRulesObjectList = - labelApplicationRuleConfig.getObjectList(SYSTEM_LABEL_APPLICATION_RULES); - this.systemLabelApplicationRules = - buildSystemLabelApplicationRuleList(systemLabelApplicationRulesObjectList); - this.systemLabelApplicationRulesMap = - this.systemLabelApplicationRules.stream() - .collect(Collectors.toUnmodifiableMap(LabelApplicationRule::getId, identity())); - } else { - this.systemLabelApplicationRules = Collections.emptyList(); - this.systemLabelApplicationRulesMap = Collections.emptyMap(); - } + this.labelApplicationRuleConfig = new LabelApplicationRuleConfig(config); + final ConfigServiceBlockingStub configServiceBlockingStub = ConfigServiceGrpc.newBlockingStub(configChannel) .withCallCredentials( @@ -124,7 +90,7 @@ public void getLabelApplicationRules( .map(LabelApplicationRule::getId) .collect(Collectors.toUnmodifiableSet()); List filteredSystemLabelApplicationRules = - systemLabelApplicationRules.stream() + this.labelApplicationRuleConfig.getSystemLabelApplicationRules().stream() .filter(rule -> !labelApplicationRuleIds.contains(rule.getId())) .collect(Collectors.toUnmodifiableList()); responseObserver.onNext( @@ -148,7 +114,12 @@ public void updateLabelApplicationRule( LabelApplicationRule existingRule = this.labelApplicationRuleStore .getData(requestContext, request.getId()) - .or(() -> Optional.ofNullable(systemLabelApplicationRulesMap.get(request.getId()))) + .or( + () -> + Optional.ofNullable( + this.labelApplicationRuleConfig + .getSystemLabelApplicationRulesMap() + .get(request.getId()))) .orElseThrow(Status.NOT_FOUND::asRuntimeException); LabelApplicationRule updateLabelApplicationRule = existingRule.toBuilder().setData(request.getData()).build(); @@ -174,7 +145,9 @@ public void deleteLabelApplicationRule( RequestContext requestContext = RequestContext.CURRENT.get(); this.requestValidator.validateOrThrow(requestContext, request); String labelApplicationRuleId = request.getId(); - if (systemLabelApplicationRulesMap.containsKey(labelApplicationRuleId)) { + if (this.labelApplicationRuleConfig + .getSystemLabelApplicationRulesMap() + .containsKey(labelApplicationRuleId)) { // Deleting a system label application rule is not allowed responseObserver.onError(new StatusRuntimeException(Status.INVALID_ARGUMENT)); return; @@ -200,25 +173,10 @@ private void checkRequestForDynamicLabelsLimit( .filter( action -> action.hasDynamicLabelExpression() || action.hasDynamicLabelKey()) .count(); - if (dynamicLabelApplicationRules >= maxDynamicLabelApplicationRulesAllowed) { + if (dynamicLabelApplicationRules + >= this.labelApplicationRuleConfig.getMaxDynamicLabelApplicationRulesAllowed()) { throw Status.RESOURCE_EXHAUSTED.asRuntimeException(); } } } - - private List buildSystemLabelApplicationRuleList( - List configObjectList) { - return configObjectList.stream() - .map(LabelApplicationRuleConfigServiceImpl::buildLabelApplicationRuleFromConfig) - .collect(Collectors.toUnmodifiableList()); - } - - @SneakyThrows - private static LabelApplicationRule buildLabelApplicationRuleFromConfig( - com.typesafe.config.ConfigObject configObject) { - String jsonString = configObject.render(); - LabelApplicationRule.Builder builder = LabelApplicationRule.newBuilder(); - JsonFormat.parser().merge(jsonString, builder); - return builder.build(); - } } From 5fb03f85a1e45cab4bf674ac79cc2fc602e3e7a7 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Mon, 2 Sep 2024 12:36:11 +0530 Subject: [PATCH 3/5] address review comments --- .../config/service/ConfigServiceFactory.java | 3 +- ...LabelApplicationRuleConfigServiceImpl.java | 7 ++- ...lApplicationRuleConfigServiceImplTest.java | 16 ++--- .../LabelApplicationRuleConfigTest.java | 60 +++++++++++++++++++ 4 files changed, 72 insertions(+), 14 deletions(-) create mode 100644 label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigTest.java diff --git a/config-service-factory/src/main/java/org/hypertrace/config/service/ConfigServiceFactory.java b/config-service-factory/src/main/java/org/hypertrace/config/service/ConfigServiceFactory.java index 2cbb89ec..fb67c254 100644 --- a/config-service-factory/src/main/java/org/hypertrace/config/service/ConfigServiceFactory.java +++ b/config-service-factory/src/main/java/org/hypertrace/config/service/ConfigServiceFactory.java @@ -22,6 +22,7 @@ import org.hypertrace.core.serviceframework.grpc.GrpcPlatformService; import org.hypertrace.core.serviceframework.grpc.GrpcPlatformServiceFactory; import org.hypertrace.core.serviceframework.grpc.GrpcServiceContainerEnvironment; +import org.hypertrace.label.application.rule.config.service.LabelApplicationRuleConfig; import org.hypertrace.label.application.rule.config.service.LabelApplicationRuleConfigServiceImpl; import org.hypertrace.label.config.service.LabelsConfigServiceImpl; import org.hypertrace.notification.config.service.NotificationChannelConfigServiceImpl; @@ -78,7 +79,7 @@ public List buildServices( new SpacesConfigServiceImpl(localChannel), new LabelsConfigServiceImpl(localChannel, config, configChangeEventGenerator), new LabelApplicationRuleConfigServiceImpl( - localChannel, config, configChangeEventGenerator), + localChannel, new LabelApplicationRuleConfig(config), configChangeEventGenerator), new EventConditionConfigServiceImpl(localChannel, configChangeEventGenerator), new NotificationRuleConfigServiceImpl(localChannel, configChangeEventGenerator), new NotificationChannelConfigServiceImpl( 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 4420ade1..b944f994 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 @@ -1,6 +1,5 @@ package org.hypertrace.label.application.rule.config.service; -import com.typesafe.config.Config; import io.grpc.Channel; import io.grpc.Status; import io.grpc.StatusRuntimeException; @@ -35,8 +34,10 @@ public class LabelApplicationRuleConfigServiceImpl private final LabelApplicationRuleConfig labelApplicationRuleConfig; public LabelApplicationRuleConfigServiceImpl( - Channel configChannel, Config config, ConfigChangeEventGenerator configChangeEventGenerator) { - this.labelApplicationRuleConfig = new LabelApplicationRuleConfig(config); + Channel configChannel, + LabelApplicationRuleConfig labelApplicationRuleConfig, + ConfigChangeEventGenerator configChangeEventGenerator) { + this.labelApplicationRuleConfig = labelApplicationRuleConfig; final ConfigServiceBlockingStub configServiceBlockingStub = ConfigServiceGrpc.newBlockingStub(configChannel) 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 a4385a99..ec39421e 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 @@ -4,11 +4,10 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.util.JsonFormat; -import com.typesafe.config.Config; -import com.typesafe.config.ConfigFactory; import io.grpc.Channel; import io.grpc.Status; import io.grpc.StatusRuntimeException; @@ -56,17 +55,14 @@ void setUp() throws InvalidProtocolBufferException { new MockGenericConfigService().mockUpsert().mockGet().mockGetAll().mockDelete(); Channel channel = mockGenericConfigService.channel(); ConfigChangeEventGenerator configChangeEventGenerator = mock(ConfigChangeEventGenerator.class); - String configStr = - "label.application.rule.config.service {\n" - + "max.dynamic.label.application.rules.per.tenant = 2\n" - + "system.label.application.rules = [\n" - + SYSTEM_LABEL_APPLICATION_RULE_STR - + "\n]\n" - + "}\n"; - Config config = ConfigFactory.parseString(configStr); LabelApplicationRule.Builder builder = LabelApplicationRule.newBuilder().clear(); JsonFormat.parser().merge(SYSTEM_LABEL_APPLICATION_RULE_STR, builder); systemLabelApplicationRule = builder.build(); + LabelApplicationRuleConfig config = mock(LabelApplicationRuleConfig.class); + when(config.getSystemLabelApplicationRules()).thenReturn(List.of(systemLabelApplicationRule)); + when(config.getSystemLabelApplicationRulesMap()) + .thenReturn(Map.of(systemLabelApplicationRule.getId(), systemLabelApplicationRule)); + when(config.getMaxDynamicLabelApplicationRulesAllowed()).thenReturn(2); mockGenericConfigService .addService( new LabelApplicationRuleConfigServiceImpl(channel, config, configChangeEventGenerator)) 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 new file mode 100644 index 00000000..88d1eba3 --- /dev/null +++ b/label-application-rule-config-service-impl/src/test/java/org/hypertrace/label/application/rule/config/service/LabelApplicationRuleConfigTest.java @@ -0,0 +1,60 @@ +package org.hypertrace.label.application.rule.config.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.util.JsonFormat; +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.hypertrace.label.application.rule.config.service.v1.LabelApplicationRule; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class LabelApplicationRuleConfigTest { + private static final String SYSTEM_LABEL_APPLICATION_RULE_STR = + "{\"id\":\"system-label-application-rule-1\",\"data\":{\"name\":\"SystemLabelApplicationRule1\",\"matching_condition\":{\"leaf_condition\":{\"key_condition\":{\"operator\":\"OPERATOR_EQUALS\",\"value\":\"test.key\"},\"unary_condition\":{\"operator\":\"OPERATOR_EXISTS\"}}},\"label_action\":{\"entity_types\":[\"API\"],\"operation\":\"OPERATION_MERGE\",\"static_labels\":{\"ids\":[\"static-label-id-1\"]}},\"enabled\":false}}"; + LabelApplicationRule systemLabelApplicationRule; + Map stringLabelApplicationRuleMap; + LabelApplicationRuleConfig labelApplicationRuleConfig; + + @BeforeEach + void setUp() throws InvalidProtocolBufferException { + String configStr = + "label.application.rule.config.service {\n" + + "max.dynamic.label.application.rules.per.tenant = 5\n" + + "system.label.application.rules = [\n" + + SYSTEM_LABEL_APPLICATION_RULE_STR + + "\n]\n" + + "}\n"; + Config config = ConfigFactory.parseString(configStr); + LabelApplicationRule.Builder builder = LabelApplicationRule.newBuilder().clear(); + JsonFormat.parser().merge(SYSTEM_LABEL_APPLICATION_RULE_STR, builder); + systemLabelApplicationRule = builder.build(); + stringLabelApplicationRuleMap = new HashMap<>(); + stringLabelApplicationRuleMap.put( + systemLabelApplicationRule.getId(), systemLabelApplicationRule); + labelApplicationRuleConfig = new LabelApplicationRuleConfig(config); + } + + @Test + void test_getMaxDynamicLabelApplicationRulesAllowed() { + assertEquals(5, labelApplicationRuleConfig.getMaxDynamicLabelApplicationRulesAllowed()); + } + + @Test + void test_getSystemLabelApplicationRules() { + assertEquals( + List.of(systemLabelApplicationRule), + labelApplicationRuleConfig.getSystemLabelApplicationRules()); + } + + @Test + void test_getSystemLabelApplicationRulesMap() { + assertEquals( + stringLabelApplicationRuleMap, + labelApplicationRuleConfig.getSystemLabelApplicationRulesMap()); + } +} From 953a4c409639afd08a8f895225224627e9467e90 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Mon, 2 Sep 2024 13:02:27 +0530 Subject: [PATCH 4/5] address review comments --- .../LabelApplicationRuleConfigServiceImplTest.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) 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 ec39421e..00d4bb3b 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 @@ -7,7 +7,6 @@ import static org.mockito.Mockito.when; import com.google.protobuf.InvalidProtocolBufferException; -import com.google.protobuf.util.JsonFormat; import io.grpc.Channel; import io.grpc.Status; import io.grpc.StatusRuntimeException; @@ -43,8 +42,6 @@ import org.junit.jupiter.api.Test; public class LabelApplicationRuleConfigServiceImplTest { - private static final String SYSTEM_LABEL_APPLICATION_RULE_STR = - "{\"id\":\"system-label-application-rule-1\",\"data\":{\"name\":\"SystemLabelApplicationRule1\",\"matching_condition\":{\"leaf_condition\":{\"key_condition\":{\"operator\":\"OPERATOR_EQUALS\",\"value\":\"test.key\"},\"unary_condition\":{\"operator\":\"OPERATOR_EXISTS\"}}},\"label_action\":{\"entity_types\":[\"API\"],\"operation\":\"OPERATION_MERGE\",\"static_labels\":{\"ids\":[\"static-label-id-1\"]}},\"enabled\":false}}"; MockGenericConfigService mockGenericConfigService; LabelApplicationRuleConfigServiceBlockingStub labelApplicationRuleConfigServiceBlockingStub; LabelApplicationRule systemLabelApplicationRule; @@ -55,9 +52,11 @@ void setUp() throws InvalidProtocolBufferException { new MockGenericConfigService().mockUpsert().mockGet().mockGetAll().mockDelete(); Channel channel = mockGenericConfigService.channel(); ConfigChangeEventGenerator configChangeEventGenerator = mock(ConfigChangeEventGenerator.class); - LabelApplicationRule.Builder builder = LabelApplicationRule.newBuilder().clear(); - JsonFormat.parser().merge(SYSTEM_LABEL_APPLICATION_RULE_STR, builder); - systemLabelApplicationRule = builder.build(); + systemLabelApplicationRule = + LabelApplicationRule.newBuilder() + .setId("system-label-rule-id-1") + .setData(LabelApplicationRuleData.newBuilder().setName("SystemLabelApplicationRule1")) + .build(); LabelApplicationRuleConfig config = mock(LabelApplicationRuleConfig.class); when(config.getSystemLabelApplicationRules()).thenReturn(List.of(systemLabelApplicationRule)); when(config.getSystemLabelApplicationRulesMap()) From 959ac0d2db408cbdb15b33131f21ddd5eac503e6 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Mon, 2 Sep 2024 13:30:48 +0530 Subject: [PATCH 5/5] update catalog version --- settings.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings.gradle.kts b/settings.gradle.kts index 8a8fb666..817d229e 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -16,7 +16,7 @@ plugins { } configure { - catalogVersion.set("0.3.23") + catalogVersion.set("0.3.28") } enableFeaturePreview("TYPESAFE_PROJECT_ACCESSORS")