cryostatio / cryostatio/cryostat

[Story] Replace LabelSelectorMatcher with MatchExpression

Open
#430 1 comment 1 reaction 0 assignees View on GitHub
feat
Dominant language
Java
Stars
57
Forks
17
Avg merge
15h 39m
Merged PRs (30d)
42

Description

```diff
diff --git a/src/main/java/io/cryostat/graphql/RootNode.java b/src/main/java/io/cryostat/graphql/RootNode.java
index 4af6eef..c2facb6 100644
--- a/src/main/java/io/cryostat/graphql/RootNode.java
+++ b/src/main/java/io/cryostat/graphql/RootNode.java
@@ -21,6 +21,8 @@ import java.util.Set;
import java.util.function.Predicate;

import io.cryostat.discovery.DiscoveryNode;
+import io.cryostat.expressions.MatchExpression;
+import io.cryostat.expressions.MatchExpressionEvaluator;
import io.cryostat.graphql.matchers.LabelSelectorMatcher;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -30,6 +32,7 @@ import org.eclipse.microprofile.graphql.Description;
import org.eclipse.microprofile.graphql.GraphQLApi;
import org.eclipse.microprofile.graphql.Query;
import org.eclipse.microprofile.graphql.Source;
+import org.projectnessie.cel.tools.ScriptException;

@GraphQLApi
public class RootNode {
@@ -77,6 +80,7 @@ public class RootNode {
public @Nullable List nodeTypes;
public @Nullable List labels;
public @Nullable List annotations;
+ public @Nullable List matchExpressions;

@Override
public boolean test(DiscoveryNode t) {
@@ -114,6 +118,24 @@ public class RootNode {
.annotations
.merged())));

+ MatchExpressionEvaluator evaluator = new MatchExpressionEvaluator(); // WRONG
+ Predicate matchesExpression =
+ n ->
+ matchExpressions == null
+ || (n.target != null
+ && matchExpressions.stream()
+ .map(MatchExpression::new)
+ .allMatch(
+ expr -> {
+ try {
+ return evaluator.applies(
+ expr, n.target);
+ } catch (ScriptException e) {
+ // FIXME log this
+ return false;
+ }
+ }));
+
return List.of(
matchesId,
matchesIds,
@@ -122,7 +144,8 @@ public class RootNode {
matchesNames,
matchesNodeTypes,
matchesLabels,
- matchesAnnotations)
+ matchesAnnotations,
+ matchesExpression)
.stream()
.reduce(x -> true, Predicate::and)
.test(t);
diff --git a/src/test/java/itest/GraphQLTest.java b/src/test/java/itest/GraphQLTest.java
index 3d28654..15d1b69 100644
--- a/src/test/java/itest/GraphQLTest.java
+++ b/src/test/java/itest/GraphQLTest.java
@@ -307,8 +307,8 @@ class GraphQLTest extends StandardSelfTest {
JsonObject query2 = new JsonObject();
query2.put(
"query",
- "mutation { createRecording( nodes:{annotations: ["
- + "\"REALM = Custom Targets\""
+ "mutation { createRecording( nodes:{matchExpressions: ["
+ + "\"target.annotations[\"REALM\"] == \"Custom Targets\"\""
+ "]}, recording: { name: \"test\", template:"
+ " \"Profiling\", templateType: \"TARGET\", duration: 30, continuous:"
+ " false, archiveOnStop: true, toDisk: true }) { name state duration"
```

Rough prototype of the idea. The `LabelSelectorMatcher` has a very limited syntax and is only used for matching on maps of labels and annotations. This could easily be a MatchExpression instead and use CEL syntax that is also already used for Automated Rules and Stored Credentials, instead of its own k8s-inspired syntax that only appears in GraphQL query filters.

Contributor guide

Open the contributing guide

Research direction

Start with the GraphQL filtering logic in src/main/java/io/cryostat/graphql/RootNode.java, then inspect LabelSelectorMatcher, MatchExpression, and MatchExpressionEvaluator to understand the existing APIs. Review the related cases in src/test/java/itest/GraphQLTest.java and run the GraphQL tests. Done means GraphQL node filters use the shared CEL-based match-expression behavior with coverage for the shown query.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, java
Domain
api, backend-api-design
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.