diqiu50 commented on code in PR #13858:
URL: https://github.com/apache/gravitino/pull/13858#discussion_r4292802486
##########
core/src/main/java/org/apache/gravitino/connector/HiddenPropertyMaskUtils.java:
##########
@@ -176,4 +182,120 @@ public static Map<String, String> maskHiddenProperties(
Map<String, String> properties, PropertiesMetadata metadata) {
return maskHiddenProperties(properties,
classifyHiddenProperties(properties, metadata));
}
+
+ /**
+ * Parses {@code value} as JSON and masks sensitive-named keys in objects
(recursively through
+ * nested objects and arrays). Blank strings and non-JSON plain text
(trimmed value does not start
+ * with '{' or '[') are returned unchanged. Values that look like a JSON
object/array but fail to
+ * parse are replaced with {@link #MASKED_VALUE}. Scalar JSON roots are
returned unchanged.
+ *
+ * <p>Useful when a caller already knows a string is a nested JSON map (for
example a CLI argument
+ * following {@code --spark-conf}).
+ *
+ * @param value JSON string to mask; may be null
+ * @return masked JSON, {@link #MASKED_VALUE} on object/array parse failure,
or the original value
+ * when blank, plain non-JSON, scalar JSON, or null
+ */
+ @Nullable
+ public static String maskSensitiveJsonValue(@Nullable String value) {
+ if (value == null) {
+ return null;
+ }
+ return maskNestedJsonValue(value);
+ }
+
+ /**
+ * Parses {@code value} as JSON and masks sensitive-named keys in objects
(recursively through
+ * nested objects and arrays). Blank / plain non-JSON strings are returned
unchanged. Malformed
+ * object/array-looking JSON is replaced with {@link #MASKED_VALUE}.
+ */
+ private static String maskNestedJsonValue(String value) {
+ // Empty / whitespace-only values carry no secret (common for unresolved
{{spark_conf:-}}).
+ if (StringUtils.isBlank(value)) {
+ return value;
+ }
+ // Bare words (e.g. --options=verbose) are not JSON maps; do not treat
parse failure as secret.
+ String trimmed = value.trim();
+ if (!trimmed.startsWith("{") && !trimmed.startsWith("[")) {
+ return value;
+ }
+ ObjectMapper mapper = JsonUtils.anyFieldMapper();
+ try {
+ Object root = mapper.readValue(value, Object.class);
+ if (!(root instanceof Map) && !(root instanceof List)) {
+ // Scalar JSON (string/number/boolean/null) — nothing to walk by key
name.
+ return value;
+ }
+ MaskedJson masked = maskJsonNode(root);
+ return masked.changed ? mapper.writeValueAsString(masked.value) : value;
+ } catch (Exception e) {
Review Comment:
Nit (non-blocking): when malformed JSON is replaced with `******`, users
can't tell why their argument changed. Consider a `LOG.debug`/`warn` here
(without printing the value).
##########
core/src/main/java/org/apache/gravitino/connector/HiddenPropertyMaskUtils.java:
##########
@@ -176,4 +182,120 @@ public static Map<String, String> maskHiddenProperties(
Map<String, String> properties, PropertiesMetadata metadata) {
return maskHiddenProperties(properties,
classifyHiddenProperties(properties, metadata));
}
+
+ /**
+ * Parses {@code value} as JSON and masks sensitive-named keys in objects
(recursively through
+ * nested objects and arrays). Blank strings and non-JSON plain text
(trimmed value does not start
+ * with '{' or '[') are returned unchanged. Values that look like a JSON
object/array but fail to
+ * parse are replaced with {@link #MASKED_VALUE}. Scalar JSON roots are
returned unchanged.
+ *
+ * <p>Useful when a caller already knows a string is a nested JSON map (for
example a CLI argument
+ * following {@code --spark-conf}).
+ *
+ * @param value JSON string to mask; may be null
+ * @return masked JSON, {@link #MASKED_VALUE} on object/array parse failure,
or the original value
+ * when blank, plain non-JSON, scalar JSON, or null
+ */
+ @Nullable
+ public static String maskSensitiveJsonValue(@Nullable String value) {
+ if (value == null) {
+ return null;
+ }
+ return maskNestedJsonValue(value);
+ }
+
+ /**
+ * Parses {@code value} as JSON and masks sensitive-named keys in objects
(recursively through
+ * nested objects and arrays). Blank / plain non-JSON strings are returned
unchanged. Malformed
+ * object/array-looking JSON is replaced with {@link #MASKED_VALUE}.
+ */
+ private static String maskNestedJsonValue(String value) {
+ // Empty / whitespace-only values carry no secret (common for unresolved
{{spark_conf:-}}).
+ if (StringUtils.isBlank(value)) {
+ return value;
+ }
+ // Bare words (e.g. --options=verbose) are not JSON maps; do not treat
parse failure as secret.
+ String trimmed = value.trim();
+ if (!trimmed.startsWith("{") && !trimmed.startsWith("[")) {
+ return value;
+ }
+ ObjectMapper mapper = JsonUtils.anyFieldMapper();
+ try {
+ Object root = mapper.readValue(value, Object.class);
+ if (!(root instanceof Map) && !(root instanceof List)) {
+ // Scalar JSON (string/number/boolean/null) — nothing to walk by key
name.
+ return value;
+ }
+ MaskedJson masked = maskJsonNode(root);
+ return masked.changed ? mapper.writeValueAsString(masked.value) : value;
+ } catch (Exception e) {
+ // Malformed object/array JSON on a caller-declared nest key — do not
leave a secret blob.
+ return MASKED_VALUE;
+ }
+ }
+
+ private static MaskedJson maskJsonNode(Object node) {
+ if (node instanceof Map) {
+ Map<?, ?> map = (Map<?, ?>) node;
+ if (map.isEmpty()) {
+ return new MaskedJson(node, false);
+ }
+ Map<String, Object> out = new LinkedHashMap<>(map.size());
+ boolean changed = false;
+ for (Map.Entry<?, ?> entry : map.entrySet()) {
+ if (entry.getKey() == null) {
Review Comment:
Nit (non-blocking): Jackson never produces a null key when parsing JSON, so
this branch is unreachable and can be removed.
##########
core/src/main/java/org/apache/gravitino/connector/HiddenPropertyMaskUtils.java:
##########
@@ -176,4 +182,120 @@ public static Map<String, String> maskHiddenProperties(
Map<String, String> properties, PropertiesMetadata metadata) {
return maskHiddenProperties(properties,
classifyHiddenProperties(properties, metadata));
}
+
+ /**
+ * Parses {@code value} as JSON and masks sensitive-named keys in objects
(recursively through
+ * nested objects and arrays). Blank strings and non-JSON plain text
(trimmed value does not start
+ * with '{' or '[') are returned unchanged. Values that look like a JSON
object/array but fail to
+ * parse are replaced with {@link #MASKED_VALUE}. Scalar JSON roots are
returned unchanged.
+ *
+ * <p>Useful when a caller already knows a string is a nested JSON map (for
example a CLI argument
+ * following {@code --spark-conf}).
+ *
+ * @param value JSON string to mask; may be null
+ * @return masked JSON, {@link #MASKED_VALUE} on object/array parse failure,
or the original value
+ * when blank, plain non-JSON, scalar JSON, or null
+ */
+ @Nullable
+ public static String maskSensitiveJsonValue(@Nullable String value) {
+ if (value == null) {
+ return null;
+ }
+ return maskNestedJsonValue(value);
+ }
+
+ /**
+ * Parses {@code value} as JSON and masks sensitive-named keys in objects
(recursively through
+ * nested objects and arrays). Blank / plain non-JSON strings are returned
unchanged. Malformed
+ * object/array-looking JSON is replaced with {@link #MASKED_VALUE}.
+ */
+ private static String maskNestedJsonValue(String value) {
+ // Empty / whitespace-only values carry no secret (common for unresolved
{{spark_conf:-}}).
+ if (StringUtils.isBlank(value)) {
+ return value;
+ }
+ // Bare words (e.g. --options=verbose) are not JSON maps; do not treat
parse failure as secret.
+ String trimmed = value.trim();
+ if (!trimmed.startsWith("{") && !trimmed.startsWith("[")) {
+ return value;
+ }
+ ObjectMapper mapper = JsonUtils.anyFieldMapper();
+ try {
+ Object root = mapper.readValue(value, Object.class);
Review Comment:
Nit (non-blocking): parsing into `Object` and writing back can change
numeric literals (e.g. `1e2` becomes `100.0`, and large or high-precision
numbers may lose fidelity). It only applies when something was masked, so the
impact is small. Parsing with `JsonNode` (or enabling
`USE_BIG_DECIMAL_FOR_FLOATS`) would avoid it.
##########
core/src/main/java/org/apache/gravitino/job/JobRuntimeTemplateMaskUtils.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.job;
+
+import com.google.common.collect.ImmutableSet;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import javax.annotation.Nullable;
+import org.apache.gravitino.connector.HiddenPropertyMaskUtils;
+import org.apache.gravitino.dto.job.JobTemplateDTO;
+import org.apache.gravitino.dto.job.ShellJobTemplateDTO;
+import org.apache.gravitino.dto.job.SparkJobTemplateDTO;
+
+/**
+ * Display-masks credentials in a Spark job's {@code updater_options} and
{@code spark_conf} JSON
+ * before that JSON is returned by an API or placed on an event.
Sensitive-named keys inside the
+ * JSON are replaced with {@link HiddenPropertyMaskUtils#MASKED_VALUE}. This
is not encryption: the
+ * stored runtime template and the map passed to job execution stay plaintext.
+ *
+ * <p>Only those two JSON bags are masked.
+ *
+ * <ul>
+ * <li>Spark template arguments {@code --updater-options} and {@code
--spark-conf}. One list
+ * element is one argument, so the JSON is the whole next element (or
the text after {@code =}
+ * in {@code --flag=json}). It is not split on whitespace.
+ * <li>{@code jobConf} entries {@code updater_options} and {@code
spark_conf} on the run-job
+ * events. Those names are the Spark template placeholders {@code
{{updater_options}}} and
+ * {@code {{spark_conf}}}, not the hyphenated CLI flags.
+ * </ul>
+ *
+ * <p>Shell jobs have neither bag. Other arguments ({@code --password}, {@code
--conf}, {@code -c},
+ * {@code --options}) and map fields ({@code environments}, {@code
customFields}, Spark {@code
+ * configs}) are copied unchanged. A blank value, which is what {@code
{{spark_conf:-}}} becomes
+ * when unset, is left unchanged.
+ */
+public final class JobRuntimeTemplateMaskUtils {
+
+ /**
+ * Argument flags whose following value is a JSON map that may contain
credentials.
+ *
+ * <ul>
+ * <li>{@code --updater-options} — Gravitino / updater credentials
+ * <li>{@code --spark-conf} — Spark configs, including catalog passwords
+ * </ul>
+ */
+ static final Set<String> JSON_ARGUMENT_FLAGS =
+ ImmutableSet.of("--updater-options", "--spark-conf");
+
+ /**
+ * {@code jobConf} keys for the Spark placeholders {@code
{{updater_options}}} and {@code
+ * {{spark_conf}}}. The CLI flags in {@link #JSON_ARGUMENT_FLAGS} use
hyphens and are a different
+ * spelling of the same two JSON bags.
+ */
+ static final Set<String> JSON_JOB_CONF_KEYS =
ImmutableSet.of("updater_options", "spark_conf");
+
+ private JobRuntimeTemplateMaskUtils() {}
+
+ /**
+ * Returns a copy of {@code jobConf} for run-job events. Only the Spark
placeholder values {@code
+ * updater_options} and {@code spark_conf} are masked. A top-level key such
as {@code password} is
+ * copied unchanged, because this method does not scan every entry. Does not
mutate the input;
+ * callers must pass the original map to job execution.
+ *
+ * @param jobConf runtime configuration from the runJob caller; may be null
+ * @return a masked copy, or null when {@code jobConf} is null
+ */
+ @Nullable
+ public static Map<String, String> maskSensitiveJobConf(@Nullable Map<String,
String> jobConf) {
+ if (jobConf == null) {
+ return null;
+ }
+ Map<String, String> masked = new LinkedHashMap<>();
+ for (Map.Entry<String, String> entry : jobConf.entrySet()) {
+ String key = entry.getKey();
+ String value = entry.getValue();
+ if (key != null && JSON_JOB_CONF_KEYS.contains(key)) {
+ masked.put(key, HiddenPropertyMaskUtils.maskSensitiveJsonValue(value));
+ } else {
+ masked.put(key, value);
+ }
+ }
+ return masked;
+ }
+
+ /**
+ * Returns a copy of a Spark runtime template with {@code --updater-options}
and {@code
+ * --spark-conf} JSON masked, or {@code null} when the input is null. Spark
{@code configs},
+ * {@code environments}, and {@code customFields} are not masked. A Shell
template is rebuilt with
+ * the same argument walk so a hand-written template that uses these flags
does not leak; built-in
+ * credential JSON lives on the Spark template.
+ *
+ * @param jobTemplateDTO the resolved runtime job template DTO
+ * @return a masked copy, or null
+ */
+ @Nullable
+ public static JobTemplateDTO maskSensitiveFields(@Nullable JobTemplateDTO
jobTemplateDTO) {
Review Comment:
Nit (non-blocking): the DTO is rebuilt field by field, so a field added to
`SparkJobTemplateDTO`/`ShellJobTemplateDTO` later would be silently dropped
here. Consider a copy helper on the DTO, or a test that checks all fields are
carried over.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
