Merge pull request opensearch-project#421 from gregschohn/EnableUserA…

…gentTweakViaJolt

Transformer configuration UX changes

TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJMESPathMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJMESPathScriptsTest.java

@@ -32,7 +32,7 @@ public void testTwoScripts() throws Exception {
         var aggregateScriptJoiner = new StringJoiner(",\n", "[", "]");
         for (var script : new String[]{EXCISE_SCRIPT, HOSTNAME_SCRIPT}) {
             aggregateScriptJoiner.add(
-                    "{\"org.opensearch.migrations.transform.JsonJMESPathTransformerProvider\": {" +
+                    "{\"JsonJMESPathTransformerProvider\": {" +
                             " \"script\": \"" + script + "\"}}"
             );
         }

TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJoltMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJoltScriptsTest.java

@@ -21,7 +21,7 @@ private static Map<String,Object> parseAsMap(String contents) throws Exception {
     @Test
     public void testAddGzip() throws Exception {
         final var addGzip =
-                "[{\"org.opensearch.migrations.transform.JsonJoltTransformerProvider\": { \"canned\": \"ADD_GZIP\" }}]";
+                "[{\"JsonJoltTransformerProvider\": { \"canned\": \"ADD_GZIP\" }}]";
         var toNewHostTransformer = new TransformationLoader().getTransformerFactoryLoader("testhostname",
                 addGzip);
         var origDocStr = SampleContents.loadSampleJsonRequestAsString();
@@ -36,8 +36,8 @@ public void testAddGzip() throws Exception {
     @Test
     public void testAddGzipAndCustom() throws Exception {
         final var addGzip = "[" +
-                "{\"org.opensearch.migrations.transform.JsonJoltTransformerProvider\": { \"canned\": \"ADD_GZIP\" }}," +
-                "{ \"org.opensearch.migrations.transform.JsonJoltTransformerProvider\":" +
+                "{\"JsonJoltTransformerProvider\": { \"canned\": \"ADD_GZIP\" }}," +
+                "{ \"JsonJoltTransformerProvider\":" +
                 "  {\"script\": \n" +
                 "    { \"operation\": \"modify-overwrite-beta\", \"spec\": " +
                 "      { \"headers\": {\"newHeader\": \"newValue\"}}}}}" +

TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/java/org/opensearch/migrations/transform/NoopTransformerProvider.java

@@ -0,0 +1,17 @@
+package org.opensearch.migrations.transform;
+
+import java.util.Map;
+
+public class NoopTransformerProvider implements IJsonTransformerProvider {
+    private static class NoopTransformer implements IJsonTransformer {
+        @Override
+        public Map<String, Object> transformJson(Map<String, Object> incomingJson) {
+            return incomingJson;
+        }
+    }
+
+    @Override
+    public IJsonTransformer createTransformer(Object jsonConfig) {
+        return new NoopTransformer();
+    }
+}

TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF/services/org.opensearch.migrations.transform.IJsonTransformerProvider

@@ -0,0 +1 @@
+org.opensearch.migrations.transform.NoopTransformerProvider

TrafficCapture/trafficReplayer/README.md

@@ -127,13 +127,17 @@ transform to add GZIP encoding and another to apply a new header would be config
 
 ```
 [
-{"org.opensearch.migrations.transform.JsonJMESPathTransformerProvider": { "script": 
+{"JsonJMESPathTransformerProvider": { "script": 
     "{\"method\": method,\"URI\": URI,\"headers\":headers,\"payload\":{\"inlinedJsonBody\":{\"mappings\": payload.inlinedJsonBody.mappings.oldType}}}"}},
-{"org.opensearch.migrations.transform.JsonJoltTransformerProvider": { "canned": "ADD_GZIP" }},
-{ "org.opensearch.migrations.transform.JsonJoltTransformerProvider":  {"script": 
+{"JsonJoltTransformerProvider": { "canned": "ADD_GZIP" }},
+{"JsonJoltTransformerProvider":  {"script": 
     { "operation": "modify-overwrite-beta", "spec":       { "headers": {"newHeader": "newValue"}}}}}]
 ```
 
+To run only one transformer without any configuration, the `--transformer-config` argument can simply 
+be set to the classname of the transformer (e.g. 'JsonTransformerForOpenSearch23PlusTargetTransformerProvider', 
+without quotes or any json surrounding it).
+
 Some simple transformations are included to change headers to add compression or to force an HTTP message payload to 
 be chunked.  Another transformer, [JsonTypeMappingTransformer.java](../replayerPlugins/jsonMessageTransformers/openSearch23PlusTargetTransformerProvider/src/main/java/org/opensearch/migrations/transform/JsonTypeMappingTransformer.java),
 is a work-in-progress to excise type mapping references from URIs and message payloads since versions of OpenSource

TrafficCapture/trafficReplayer/build.gradle

@@ -40,6 +40,8 @@ dependencies {
     implementation project(':captureProtobufs')
     implementation project(':coreUtilities')
     implementation project(':replayerPlugins:jsonMessageTransformers:jsonMessageTransformerInterface')
+    runtimeOnly project(':replayerPlugins:jsonMessageTransformers:jsonJMESPathMessageTransformerProvider')
+    runtimeOnly project(':replayerPlugins:jsonMessageTransformers:jsonJoltMessageTransformerProvider')
     runtimeOnly project(':replayerPlugins:jsonMessageTransformers:openSearch23PlusTargetTransformerProvider')
 
     implementation group: 'com.beust', name: 'jcommander', version: '1.82'
@@ -74,6 +76,9 @@ dependencies {
     testImplementation project(':captureOffloader')
     testImplementation testFixtures(project(path: ':captureOffloader'))
     testImplementation testFixtures(project(path: ':testUtilities'))
+    testImplementation project(':replayerPlugins:jsonMessageTransformers:jsonJMESPathMessageTransformerProvider')
+    testImplementation project(':replayerPlugins:jsonMessageTransformers:jsonJoltMessageTransformerProvider')
+    testImplementation project(':replayerPlugins:jsonMessageTransformers:openSearch23PlusTargetTransformerProvider')
 
     testImplementation group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.2.1'
     testImplementation group: 'org.junit.jupiter', name:'junit-jupiter-api', version:'5.x.x'

TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TrafficReplayer.java

@@ -300,8 +300,11 @@ static class Parameters {
         @Parameter(required = false,
                 names = "--transformer-config",
                 arity = 1,
-                description = "Json configuration of message transformers.  Keys are the names of the loaded transformers " +
-                "(shortname or longname) and values are the configuration passed to each of the transformers.")
+                description = "Configuration of message transformers.  Either as a string that identifies the " +
+                        "transformer that should be run (with default settings) or as json to specify options " +
+                        "as well as multiple transformers to run in sequence.  " +
+                        "For json, keys are the (simple) names of the loaded transformers and values are the " +
+                        "configuration passed to each of the transformers.")
         String transformerConfig;
     }
 

TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TransformationLoader.java

@@ -16,13 +16,15 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.ServiceLoader;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 @Slf4j
 public class TransformationLoader {
     public static final String WRONG_JSON_STRUCTURE_MESSAGE = "Must specify the top-level configuration list with a sequence of " +
             "maps that have only one key each, where the key is the name of the transformer to be configured.";
+    public static final Pattern CLASS_NAME_PATTERN = Pattern.compile("^[a-zA-Z_$][a-zA-Z\\d_$]*$");
     private final List<IJsonTransformerProvider> providers;
     ObjectMapper objMapper = new ObjectMapper();
 
@@ -40,23 +42,22 @@ public TransformationLoader() {
     }
 
     List<Map<String, Object>> parseFullConfig(String fullConfig) throws JsonProcessingException {
-        return objMapper.readValue(fullConfig, new TypeReference<>() {});
+        if (CLASS_NAME_PATTERN.matcher(fullConfig).matches()) {
+            return List.of(Collections.singletonMap(fullConfig, null));
+        } else {
+            return objMapper.readValue(fullConfig, new TypeReference<>() {
+            });
+        }
     }
 
     protected Stream<IJsonTransformer> getTransformerFactoryFromServiceLoader(String fullConfig)
             throws JsonProcessingException {
         var configList = fullConfig == null ? List.of() : parseFullConfig(fullConfig);
-        if (providers.size() > 1 && configList.isEmpty()) {
-            throw new IllegalArgumentException("Must provide a configuration when multiple IJsonTransformerProvider " +
-                    "are loaded (" + providers.stream().map(p -> p.getClass().toString())
-                    .collect(Collectors.joining(",")) + ")");
-        } else if (providers.isEmpty()) {
+        if (configList.isEmpty() || providers.isEmpty()) {
+            log.warn("No transformer configuration specified.  No custom transformations will be performed");
             return Stream.of();
-        } else if (!configList.isEmpty()) {
+        }else {
             return configList.stream().map(c -> configureTransformerFromConfig((Map<String, Object>) c));
-        } else {
-            // send in Optional.empty because we would have hit the other case in the previous branch
-            return Stream.of(providers.get(0).createTransformer(Optional.empty()));
         }
     }
 
@@ -69,7 +70,7 @@ private IJsonTransformer configureTransformerFromConfig(Map<String, Object> c) {
         var key = keys.stream().findFirst()
                 .orElseThrow(()->new IllegalArgumentException(WRONG_JSON_STRUCTURE_MESSAGE));
         for (var p : providers) {
-            var className = p.getClass().getName();
+            var className = p.getClass().getSimpleName();
             if (className.equals(key)) {
                 var configuration = c.get(key);
                 log.atInfo().setMessage(()->"Creating a transformer with configuration="+configuration).log();

TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/HeaderTransformerTest.java

@@ -113,7 +113,7 @@ public void testMalformedPayload_andTypeMappingUri_IsPassedThrough() throws Exce
 
         var transformingHandler = new HttpJsonTransformingConsumer(
                 new TransformationLoader().getTransformerFactoryLoader(SILLY_TARGET_CLUSTER_NAME,
-                        "[{\"org.opensearch.migrations.transform.JsonTransformerForOpenSearch23PlusTargetTransformerProvider\":\"\"}]"),
+                        "[{\"JsonTransformerForOpenSearch23PlusTargetTransformerProvider\":\"\"}]"),
                 null, testPacketCapture, "TEST", TestRequestKey.getTestConnectionRequestId(0));
 
         Random r = new Random(2);

TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java

@@ -28,4 +28,28 @@ public void testTransformationLoader() throws Exception {
         Assertions.assertNotEquals(origDocStr, docWithNewHostnameStr);
     }
 
+    @Test
+    public void testThatSimpleNoopTransformerLoads() throws Exception {
+        var noopTransformer = new TransformationLoader()
+                .getTransformerFactoryLoader("localhost", "NoopTransformerProvider");
+        var origDoc = parseAsMap(SampleContents.loadSampleJsonRequestAsString());
+        var output = noopTransformer.transformJson(origDoc);
+        Assertions.assertEquals(mapper.writeValueAsString(origDoc), mapper.writeValueAsString(output));
+    }
+
+    @Test
+    public void testMisconfiguration() throws Exception {
+        Assertions.assertThrows(IllegalArgumentException.class, () -> new TransformationLoader()
+                .getTransformerFactoryLoader("localhost", "Not right"));
+    }
+
+    @Test
+    public void testThatNoConfigMeansNoThrow() throws Exception {
+        var transformer = Assertions.assertDoesNotThrow(()->new TransformationLoader()
+                .getTransformerFactoryLoader("localhost", null));
+        Assertions.assertNotNull(transformer);
+        var origDoc = parseAsMap(SampleContents.loadSampleJsonRequestAsString());
+        Assertions.assertNotNull(transformer.transformJson(origDoc));
+    }
+
 }