Added new operator to handle IP not exists in CIDR case (#169)

* Added new operator to handle ip not exists in CIDR case * Updated enum names * Added validators and ip-utils for validation * Added just IP validation util * Removed unwanted libs * Addressed review comments * Addressed review comments * Addressed review comments * Addressed review comments
hypertrace · Jun 15, 2023 · a61ccc8 · a61ccc8
1 parent 385a24e
commit a61ccc8
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 4 deletions.
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -36,6 +36,7 @@ lombok = { module = "org.projectlombok:lombok", version.ref = "lombok" }
 jackson-databind = { module = "com.fasterxml.jackson.core:jackson-databind", version.ref = "jackson" }
 rxjava3 = { module = "io.reactivex.rxjava3:rxjava", version = "3.0.11" }
 google-re2j = { module = "com.google.re2j:re2j", version = "1.6" }
+seancfoley-ipaddress = { module = "com.github.seancfoley:ipaddress", version = "5.3.4" }
 
 grpc-protobuf = { module = "io.grpc:grpc-protobuf", version.ref = "grpc" }
 grpc-stub = { module = "io.grpc:grpc-stub", version.ref = "grpc" }

diff --git a/...roto/org/hypertrace/label/application/rule/config/service/v1/label_application_rule.proto b/...roto/org/hypertrace/label/application/rule/config/service/v1/label_application_rule.proto
@@ -45,13 +45,21 @@ message LabelApplicationRuleData {
 
   message StringCondition {
     Operator operator = 1;
-    string value = 2;
+    oneof kind {
+      string value = 2;
+      StringList values = 3;
+    }
+
+    message StringList {
+      repeated string values = 1;
+    }
 
     enum Operator {
       OPERATOR_UNSPECIFIED = 0;
       OPERATOR_EQUALS = 1; // operator to check if the key exists, and value is equal to the provided value
       OPERATOR_MATCHES_REGEX = 2; // operator to check if the key exists, and value matches the provided regex value
-      OPERATOR_CIDR = 3; // operator to check if the key exists, and IP value lies in the provided CIDR
+      OPERATOR_MATCHES_IPS = 3; // operator to check if the key exists, and IP value matches provided IP(s), CIDR(s)
+      OPERATOR_NOT_MATCHES_IPS = 4; // operator to check if the key exists, and IP doesn't match provided IP(s), CIDR(s)
     }
   }
 

diff --git a/...g/hypertrace/label/application/rule/config/service/LabelApplicationRuleValidatorImpl.java b/...g/hypertrace/label/application/rule/config/service/LabelApplicationRuleValidatorImpl.java
@@ -1,8 +1,11 @@
 package org.hypertrace.label.application.rule.config.service;
 
+import static org.hypertrace.config.validation.GrpcValidatorUtils.isValidIpAddressOrSubnet;
 import static org.hypertrace.config.validation.GrpcValidatorUtils.printMessage;
 import static org.hypertrace.config.validation.GrpcValidatorUtils.validateNonDefaultPresenceOrThrow;
 import static org.hypertrace.config.validation.GrpcValidatorUtils.validateRequestContextOrThrow;
+import static org.hypertrace.label.application.rule.config.service.v1.LabelApplicationRuleData.StringCondition.Operator.OPERATOR_MATCHES_IPS;
+import static org.hypertrace.label.application.rule.config.service.v1.LabelApplicationRuleData.StringCondition.Operator.OPERATOR_NOT_MATCHES_IPS;
 
 import com.google.protobuf.Message;
 import io.grpc.Status;
@@ -25,6 +28,7 @@
 import org.hypertrace.label.application.rule.config.service.v1.UpdateLabelApplicationRuleRequest;
 
 public class LabelApplicationRuleValidatorImpl implements LabelApplicationRuleValidator {
+
   @Override
   public void validateOrThrow(
       RequestContext requestContext,
@@ -120,7 +124,31 @@ private void validateJsonCondition(JsonCondition jsonCondition) {
   }
 
   private void validateStringCondition(StringCondition stringCondition) {
-    validateNonDefaultPresenceOrThrow(stringCondition, stringCondition.OPERATOR_FIELD_NUMBER);
+    validateNonDefaultPresenceOrThrow(stringCondition, StringCondition.OPERATOR_FIELD_NUMBER);
+    if (stringCondition.getOperator().equals(OPERATOR_MATCHES_IPS)
+        || stringCondition.getOperator().equals(OPERATOR_NOT_MATCHES_IPS)) {
+      switch (stringCondition.getKindCase()) {
+        case VALUE:
+          final String ip = stringCondition.getValue();
+          if (!isValidIpAddressOrSubnet(ip)) {
+            throwInvalidArgumentException(
+                "StringCondition in LabelApplicationRule should have a valid IP address or CIDR");
+          }
+          break;
+        case VALUES:
+          if (stringCondition.getValues().getValuesList().stream()
+              .anyMatch(s -> !isValidIpAddressOrSubnet(s))) {
+            throwInvalidArgumentException(
+                "StringCondition in LabelApplicationRule should have a valid list of IP addresses and CIDRs");
+          }
+          break;
+        default:
+          throwInvalidArgumentException(
+              String.format(
+                  "Unexpected Case in %s:%n %s",
+                  getName(stringCondition), printMessage(stringCondition)));
+      }
+    }
   }
 
   private void validateUnaryCondition(UnaryCondition unaryCondition) {

diff --git a/...g/hypertrace/label/application/rule/config/service/LabelApplicationRuleValidatorTest.java b/...g/hypertrace/label/application/rule/config/service/LabelApplicationRuleValidatorTest.java
@@ -24,8 +24,12 @@ public class LabelApplicationRuleValidatorTest {
   private final LabelApplicationRuleValidator labelApplicationRuleValidator;
   private final StringCondition errorKeyCondition;
   private final StringCondition correctKeyCondition;
+  private final StringCondition correctKeyConditionWithMatchesIPs;
+  private final StringCondition incorrectKeyConditionWithMatchesIPs;
   private final StringCondition correctAuthKeyCondition;
   private final StringCondition correctStringValueCondition;
+  private final StringCondition correctStringValueConditionWithMatchesIPs;
+  private final StringCondition incorrectStringValueConditionWithMatchesIPs;
   private final UnaryCondition errorUnaryValueCondition;
   private final UnaryCondition correctUnaryValueCondition;
   private final JsonCondition correctJsonValueCondition;
@@ -43,6 +47,16 @@ public LabelApplicationRuleValidatorTest() {
             .setOperator(StringCondition.Operator.OPERATOR_EQUALS)
             .setValue("foo")
             .build();
+    correctKeyConditionWithMatchesIPs =
+        StringCondition.newBuilder()
+            .setOperator(StringCondition.Operator.OPERATOR_MATCHES_IPS)
+            .setValue("1.2.3.4")
+            .build();
+    incorrectKeyConditionWithMatchesIPs =
+        StringCondition.newBuilder()
+            .setOperator(StringCondition.Operator.OPERATOR_MATCHES_IPS)
+            .setValue("4.5.6.7/s")
+            .build();
     errorUnaryValueCondition =
         UnaryCondition.newBuilder()
             .setOperator(UnaryCondition.Operator.OPERATOR_UNSPECIFIED)
@@ -54,6 +68,20 @@ public LabelApplicationRuleValidatorTest() {
             .setOperator(StringCondition.Operator.OPERATOR_EQUALS)
             .setValue("bar")
             .build();
+    correctStringValueConditionWithMatchesIPs =
+        StringCondition.newBuilder()
+            .setOperator(StringCondition.Operator.OPERATOR_MATCHES_IPS)
+            .setValues(
+                StringCondition.StringList.newBuilder()
+                    .addAllValues(List.of("1.2.3.4", "4.5.6.7/8")))
+            .build();
+    incorrectStringValueConditionWithMatchesIPs =
+        StringCondition.newBuilder()
+            .setOperator(StringCondition.Operator.OPERATOR_MATCHES_IPS)
+            .setValues(
+                StringCondition.StringList.newBuilder()
+                    .addAllValues(List.of("1.2.3.4/5", "1.2 3.4")))
+            .build();
     correctAuthKeyCondition =
         StringCondition.newBuilder()
             .setOperator(StringCondition.Operator.OPERATOR_EQUALS)
@@ -146,6 +174,43 @@ void validateOrThrowCreateRuleLeafCondition() {
     labelApplicationRuleValidator.validateOrThrow(REQUEST_CONTEXT, request);
   }
 
+  @Test
+  void validateOrThrowCreateRuleLeafConditionWithMatchesIPs() {
+    // This will check the condition that foo(key) = bar(value)
+    LeafCondition errorLeafCondition =
+        LeafCondition.newBuilder()
+            .setKeyCondition(correctKeyConditionWithMatchesIPs)
+            .setStringCondition(correctStringValueConditionWithMatchesIPs)
+            .build();
+    Condition matchingCondition =
+        Condition.newBuilder().setLeafCondition(errorLeafCondition).build();
+    CreateLabelApplicationRuleRequest request =
+        buildCreateCreateLabelApplicationRuleRequest(
+            "Correct Leaf Rule", matchingCondition, Optional.empty());
+    labelApplicationRuleValidator.validateOrThrow(REQUEST_CONTEXT, request);
+  }
+
+  @Test
+  void validateOrThrowCreateRuleInvalidLeafConditionWithMatchesIPs() {
+    // This will check the condition that foo(key) = bar(value)
+    LeafCondition errorLeafCondition =
+        LeafCondition.newBuilder()
+            .setKeyCondition(incorrectKeyConditionWithMatchesIPs)
+            .setStringCondition(incorrectStringValueConditionWithMatchesIPs)
+            .build();
+    Condition matchingCondition =
+        Condition.newBuilder().setLeafCondition(errorLeafCondition).build();
+    CreateLabelApplicationRuleRequest request =
+        buildCreateCreateLabelApplicationRuleRequest(
+            "Correct Leaf Rule", matchingCondition, Optional.empty());
+
+    assertThrows(
+        StatusRuntimeException.class,
+        () -> {
+          labelApplicationRuleValidator.validateOrThrow(REQUEST_CONTEXT, request);
+        });
+  }
+
   @Test
   void validateOrThrowCreateRuleCompositeCondition() {
     // This condition implies foo(key) exists AND foo(key) = bar(value) AND

diff --git a/settings.gradle.kts b/settings.gradle.kts
@@ -52,4 +52,3 @@ include(":span-processing-utils")
 
 include(":partitioner-config-service-api")
 include(":partitioner-config-service-impl")
-
diff --git a/validation-utils/build.gradle.kts b/validation-utils/build.gradle.kts
@@ -6,4 +6,5 @@ dependencies {
   api(libs.hypertrace.grpcutils.context)
   api(libs.grpc.api)
   implementation(libs.protobuf.javautil)
+  implementation(libs.seancfoley.ipaddress)
 }
diff --git a/validation-utils/src/main/java/org/hypertrace/config/validation/GrpcValidatorUtils.java b/validation-utils/src/main/java/org/hypertrace/config/validation/GrpcValidatorUtils.java
@@ -3,11 +3,23 @@
 import com.google.protobuf.Descriptors.FieldDescriptor;
 import com.google.protobuf.Message;
 import com.google.protobuf.util.JsonFormat;
+import inet.ipaddr.IPAddressString;
+import inet.ipaddr.IPAddressStringParameters;
 import io.grpc.Status;
 import org.hypertrace.core.grpcutils.context.RequestContext;
 
 public class GrpcValidatorUtils {
   private static final JsonFormat.Printer JSON_PRINTER = JsonFormat.printer();
+  private static final IPAddressStringParameters ADDRESS_VALIDATION_PARAMS =
+      new IPAddressStringParameters.Builder()
+          // Allows ipv4 joined segments like 1.2.3, 1.2, or just 1 For the case of just 1 segment
+          .allow_inet_aton(false)
+          // Allows an address to be specified as a single value, eg ffffffff, without the standard
+          // use of segments like 1.2.3.4 or 1:2:4:3:5:6:7:8
+          .allowSingleSegment(false)
+          // Allows zero-length IPAddressStrings like ""
+          .allowEmpty(false)
+          .toParams();
 
   private GrpcValidatorUtils() {}
 
@@ -36,6 +48,10 @@ public static <T extends Message> void validateNonDefaultPresenceOrThrow(
     }
   }
 
+  public static boolean isValidIpAddressOrSubnet(final String input) {
+    return new IPAddressString(input, ADDRESS_VALIDATION_PARAMS).getAddress() != null;
+  }
+
   private static <T extends Message> void validateNonDefaultPresenceRepeatedOrThrow(
       T source, FieldDescriptor descriptor) {
     if (source.getRepeatedFieldCount(descriptor) == 0) {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -52,4 +52,3 @@ include(":span-processing-utils")

		include(":partitioner-config-service-api")
		include(":partitioner-config-service-impl")