This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 2d6e8cdc406a58e7fd8fcb8c85ba5ef87ffa8f1e Author: Alex Heneveld <[email protected]> AuthorDate: Wed May 24 11:35:59 2023 +0100 more filtering of output - follow up to PR https://github.com/apache/brooklyn-server/pull/1389 Some tweaks: * tidy name of methods and params - call it filterOutputFields instead of suppressOutput (to me, suppress hints at security) * expose the capability on the generic resolver (used for some sensors, below) * when doing the filter, don't bother if it's short/trivial, and if it's not a string include the type for info * change implementation of filter to run even if suppress secrets is false * (in UI, remove one redundant activities call) And some additional restrictions: * exclude detail from tasks when doing a recursive/children lookup * filter output fields in tasks when detail is excluded * filter output fields from children and recursive task requests by default * filter output fields for selected sensors (name starts with "internal") when doing a batch read of sensors (this affects the workflow cache which contains all output) Impact for a simple test is: * 3 med size workflows - call to list - reduced from 126kb to 2kb * recursive task call for the above - from 25k to 5k * sensor batch read - from 120k to 2k * details of 1 workflow - 42kb to access -> unchanged --- .../resources/AbstractBrooklynRestResource.java | 40 ++++++++++++++-------- .../brooklyn/rest/resources/ActivityResource.java | 2 +- .../brooklyn/rest/resources/EntityResource.java | 2 +- .../brooklyn/rest/resources/SensorResource.java | 2 +- .../brooklyn/rest/transform/TaskTransformer.java | 26 ++++++++++---- 5 files changed, 49 insertions(+), 23 deletions(-) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java index 0a435bf681..a7fcb2d965 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java @@ -124,6 +124,7 @@ public abstract class AbstractBrooklynRestResource { private @Nullable Boolean skipResolution; private @Nullable Boolean suppressBecauseSecret; private @Nullable Boolean suppressSecrets; + private boolean filterOutputFields = false; public static RestValueResolver resolving(ManagementContext mgmt, Object v) { return new RestValueResolver(mgmt, v); } @@ -221,6 +222,7 @@ public abstract class AbstractBrooklynRestResource { public RestValueResolver timeout(Duration timeout) { this.timeout = timeout; return this; } public RestValueResolver immediately(boolean immediately) { this.immediately = immediately; return this; } public RestValueResolver renderAs(Object rendererHintSource) { this.rendererHintSource = rendererHintSource; return this; } + public RestValueResolver filterOutputFields(boolean filterOutputFields) { this.filterOutputFields = filterOutputFields; return this; } public Object resolve() { Object valueResult = @@ -235,8 +237,8 @@ public abstract class AbstractBrooklynRestResource { valueResult = suppressAsMinimalizedJson(mapper(), valueResult); } - return getValueForDisplayAfterSecretsCheck(mgmt, mapper(), valueResult, preferJson, isJerseyReturnValue, - Boolean.TRUE.equals(suppressSecrets) && !Boolean.TRUE.equals(suppressBecauseSecret), false); + return getValueForDisplaySanitized(mgmt, mapper(), valueResult, preferJson, isJerseyReturnValue, + Boolean.TRUE.equals(suppressSecrets) && !Boolean.TRUE.equals(suppressBecauseSecret), filterOutputFields); } private static Object UNRESOLVED = "UNRESOLVED".toCharArray(); @@ -258,18 +260,21 @@ public abstract class AbstractBrooklynRestResource { preferJson!=null ? preferJson : this.preferJson, isJerseyReturnValue!=null ? isJerseyReturnValue : this.isJerseyReturnValue, suppressNestedSecrets!=null ? suppressNestedSecrets : this.suppressSecrets, false); } - public Object getValueForDisplay(Object value, Boolean preferJson, Boolean isJerseyReturnValue, Boolean suppressNestedSecrets, boolean suppressOutput) { + public Object getValueForDisplay(Object value, Boolean preferJson, Boolean isJerseyReturnValue, Boolean suppressNestedSecrets, boolean filterOutputFields) { return getValueForDisplay(mgmt, mapper(), value, preferJson!=null ? preferJson : this.preferJson, isJerseyReturnValue!=null ? isJerseyReturnValue : this.isJerseyReturnValue, - suppressNestedSecrets!=null ? suppressNestedSecrets : this.suppressSecrets, suppressOutput); + suppressNestedSecrets!=null ? suppressNestedSecrets : this.suppressSecrets, filterOutputFields); } - public static Object getValueForDisplay(ManagementContext mgmt, ObjectMapper mapper, Object value, boolean preferJson, boolean isJerseyReturnValue, Boolean suppressNestedSecrets, Boolean suppressOutput) { + public static Object getValueForDisplay(ManagementContext mgmt, ObjectMapper mapper, Object value, boolean preferJson, boolean isJerseyReturnValue, Boolean suppressNestedSecrets, Boolean filterOutputFields) { suppressNestedSecrets = checkAndGetSecretsSuppressed(mgmt, suppressNestedSecrets, false); - return getValueForDisplayAfterSecretsCheck(mgmt, mapper, value, preferJson, isJerseyReturnValue, suppressNestedSecrets, suppressOutput); + return getValueForDisplaySanitized(mgmt, mapper, value, preferJson, isJerseyReturnValue, suppressNestedSecrets, filterOutputFields); } - static Object getValueForDisplayAfterSecretsCheck(ManagementContext mgmt, ObjectMapper mapper, Object value, boolean preferJson, boolean isJerseyReturnValue, Boolean suppressNestedSecrets, Boolean suppressOutput) { + static Object getValueForDisplaySanitized(ManagementContext mgmt, ObjectMapper mapper, Object value, boolean preferJson, boolean isJerseyReturnValue, Boolean suppressNestedSecrets, Boolean filterOutputFields) { + if (suppressNestedSecrets==null) suppressNestedSecrets = false; + if (filterOutputFields==null) filterOutputFields = false; + if (preferJson) { if (value==null) return null; Object result = value; @@ -288,22 +293,29 @@ public abstract class AbstractBrooklynRestResource { } } - if (suppressNestedSecrets) { + if (suppressNestedSecrets || filterOutputFields) { if (result==null || Boxing.isPrimitiveOrBoxedObject(result)) { // no action needed } else if (result instanceof CharSequence) { - result = Sanitizer.sanitizeMultilineString(result.toString()); + if (suppressNestedSecrets) { + result = Sanitizer.sanitizeMultilineString(result.toString()); + } } else { // go ahead and convert to json and suppress deep try { String resultS = mapper.writeValueAsString(result); result = BeanWithTypeUtils.newSimpleMapper().readValue(resultS, Object.class); - if (suppressOutput){ - result = TaskTransformer.suppressOutputs(result); + + if (filterOutputFields) { + result = TaskTransformer.filterOutputFields(result); } - //the below treats all numbers as doubles - //new Gson().fromJson(resultS, Object.class); - return Sanitizer.suppressNestedSecretsJson(result, true); + + if (suppressNestedSecrets) { + //the below treats all numbers as doubles + //new Gson().fromJson(resultS, Object.class); + result = Sanitizer.suppressNestedSecretsJson(result, true); + } + } catch (JsonProcessingException e) { throw Exceptions.propagateAnnotated("Cannot serialize REST result", e); } diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java index 39bb1552ef..081048cddd 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java @@ -90,7 +90,7 @@ public class ActivityResource extends AbstractBrooklynRestResource implements Ac Set<Task<?>> thisLayer = nextLayer; nextLayer = MutableSet.of(); for (final Task<?> childTask : thisLayer) { - TaskSummary wasThere = result.put(childTask.getId(), TaskTransformer.fromTask(ui.getBaseUriBuilder(), resolving(null), suppressSecrets).apply(childTask)); + TaskSummary wasThere = result.put(childTask.getId(), TaskTransformer.fromTask(ui.getBaseUriBuilder(), resolving(null), suppressSecrets, false).apply(childTask)); if (wasThere==null) { if (--limit == 0) { break outer; diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java index 3f8b5b6ad2..d2268068cb 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java @@ -159,7 +159,7 @@ public class EntityResource extends AbstractBrooklynRestResource implements Enti limit, recurse, entity, ui, resolving(null), suppressSecrets, TaskTransformer.TaskSummaryMode.NONE); } - /** API does not guarantee order, but this is a the one we use (when there are lots of tasks): + /** API does not guarantee order, but this is the one we use (when there are lots of tasks): * prefer top-level tasks and to recent tasks, * balanced such that the following are equal: * <li>something manually submitted here, submitted two hours ago diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/SensorResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/SensorResource.java index d8dc1e7322..cecde9a850 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/SensorResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/SensorResource.java @@ -95,7 +95,7 @@ public class SensorResource extends AbstractBrooklynRestResource implements Sens Object value = EntityAttributesUtils.tryGetAttribute(entity, findSensor(entity, sensor.getName())); sensorMap.put(sensor.getName(), resolving(value).preferJson(true).asJerseyOutermostReturnValue(false).useDisplayHints(useDisplayHints).raw(raw).context(entity).timeout(Duration.ZERO).renderAs(sensor) - .suppressIfSecret(sensor.getName(), suppressSecrets).resolve()); + .suppressIfSecret(sensor.getName(), suppressSecrets).filterOutputFields(sensor.getName().startsWith("internal")).resolve()); } return sensorMap; } diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/TaskTransformer.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/TaskTransformer.java index 390732fb92..bc11c86bc5 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/TaskTransformer.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/TaskTransformer.java @@ -21,6 +21,7 @@ package org.apache.brooklyn.rest.transform; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; import org.apache.brooklyn.api.entity.Entity; @@ -38,6 +39,7 @@ import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.task.TaskInternal; import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.javalang.Boxing; import org.apache.brooklyn.util.text.StringEscapes; import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; @@ -117,7 +119,7 @@ public class TaskTransformer { "children", new URI(selfLink+"/"+"children")); if (entityLink!=null) links.put("entity", entityLink); - Collection<Object> tags = (Collection<Object>) resolver.getValueForDisplay(task.getTags(), true, false, suppressSecrets); + Collection<Object> tags = (Collection<Object>) resolver.getValueForDisplay(task.getTags(), true, false, suppressSecrets, !includeDetail); Object result = null; String detailedStatus = null; @@ -227,6 +229,7 @@ public class TaskTransformer { } } } + List<Task<?>> finalTasksToScan = ImmutableList.copyOf(tasksToScan); return tasksLoaded.values().stream().map(task->{ boolean includeDetail = (taskSummaryMode == TaskSummaryMode.ALL && !isRecurse) || (taskSummaryMode == TaskSummaryMode.SUBSET && finalTasksToScan.contains(task)); @@ -234,25 +237,36 @@ public class TaskTransformer { return taskTaskSummaryFunction.apply(task); }).collect(Collectors.toList()); } - public static Object suppressOutputs(Object x) { + public static Object filterOutputFields(Object x) { if (x instanceof Map) { Map y = MutableMap.of(); ((Map)x).forEach((k,v) -> { - y.put(k, v!=null && TaskTransformer.IS_OUTPUT.apply(k) ? "(output suppressed)": suppressOutputs(v) ); + y.put(k, v!=null && TaskTransformer.IS_OUTPUT_FIELD.apply(k) ? filterOutputValue(v) : filterOutputFields(v) ); }); return y; }else if (x instanceof Iterable){ List y = MutableList.of(); - ((Iterable)x).forEach(xi -> y.add(suppressOutputs(xi))); + ((Iterable)x).forEach(xi -> y.add(filterOutputFields(xi))); return y; }else { return x; } } - public static final Predicate<Object> IS_OUTPUT = new IsOutputPredicate(); + static Object filterOutputValue(Object v) { + if (v==null || Boxing.isPrimitiveOrBoxedObject(v)) return v; + if (v instanceof CharSequence) { + if (((CharSequence) v).length() < 80) return v; + return "<filtered>"; + } + return "<filtered (type "+v.getClass().getName()+")>"; + } + + public static final Predicate<Object> IS_OUTPUT_FIELD = new IsOutputPredicate(); + + static final Set<String> OUTPUT_VALUES = ImmutableSet.of("output", "stdout", "stderr", + "headers", "content", "content_bytes", "content_json"); - static final ImmutableList<String> OUTPUT_VALUES = ImmutableList.of("output", "stdout", "stderr"); private static class IsOutputPredicate implements Predicate<Object> { @Override public boolean apply(Object name) {
