roryqi commented on code in PR #13865:
URL: https://github.com/apache/gravitino/pull/13865#discussion_r4292531470
##########
docs/table-maintenance-service/optimizer-configuration.md:
##########
@@ -214,11 +328,18 @@ Create a policy through `POST
/api/metalakes/{metalake}/policies`:
"enabled": true,
"content": {
"olderThanDays": 3,
- "dryRun": true
+ "dryRun": true,
+ "timeTrigger": "0 0 3 * * ?",
+ "minIntervalMs": 86400000
}
}
```
+Optional TMS peer fields on content (`timeTrigger`, `commitTrigger`,
`minIntervalMs`, `jobOptions`,
+`updateOptions`) are shared with compaction and other maintenance policy
types. `commitTrigger` is
Review Comment:
The policy wire field is `updaterOptions`, not `updateOptions`:
`PolicyExecutionSpec` and `ExecutablePolicyContentSerde` only read/write
`updaterOptions`. Following this documentation would silently drop this option
bag during deserialization. Could we rename this here, and use `updaterOptions`
/ `overlayUpdaterOptions` consistently in the new helper and Javadocs?
##########
core/src/main/java/org/apache/gravitino/maintenance/policy/MaintenanceAuthConfigs.java:
##########
@@ -0,0 +1,258 @@
+/*
+ * 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.maintenance.policy;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import javax.annotation.Nullable;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+
+/**
+ * Cluster-static TMS client auth in {@code gravitino.conf}.
+ *
+ * <p>{@link #GRAVITINO_AUTH_PREFIX} is a <strong>fixed</strong> camelCase bag
in {@code
+ * gravitino.conf} ({@code authType}, {@code oauthServerUri}, …). {@link
#loadGravitinoAuth} maps
+ * those suffixes onto built-in updater short names ({@code auth_type}, {@code
oauth_server_uri}, …)
+ * before overlaying onto policy {@code updateOptions}.
+ *
+ * <p>{@link #IRC_AUTH_PREFIX} is an <strong>open</strong> passthrough for
Spark / Iceberg REST
+ * properties (for example {@code rest.auth.type}); any non-empty suffix under
the prefix is loaded
+ * and overlaid onto policy {@code jobOptions}. Auth wins on duplicate keys.
Live {@code runJob}
+ * wiring is a follow-up.
+ */
+public final class MaintenanceAuthConfigs {
+
+ /** Prefix for Gravitino control-plane client auth (overlaid onto {@code
updateOptions}). */
+ public static final String GRAVITINO_AUTH_PREFIX =
"gravitino.maintenance.gravitinoAuth.";
+
+ /**
+ * Prefix for Iceberg REST / Spark client properties overlaid onto {@code
jobOptions}. Suffixes
+ * are not enumerated; operators may pass arbitrary catalog auth keys such
as {@code
+ * rest.auth.type}.
+ */
+ public static final String IRC_AUTH_PREFIX =
"gravitino.maintenance.ircAuth.";
+
+ /** Auth mode: {@code none}, {@code simple}, {@code basic}, or {@code
oauth}. */
+ public static final String AUTH_TYPE_SUFFIX = "authType";
+
+ /** HTTP basic / simple user principal. */
+ public static final String USERNAME_SUFFIX = "username";
+
+ /** HTTP basic password or secret reference. */
+ public static final String PASSWORD_SUFFIX = "password";
+
+ /** OAuth2 token server base URI. */
+ public static final String OAUTH_SERVER_URI_SUFFIX = "oauthServerUri";
+
+ /** OAuth2 token path relative to the server URI. */
+ public static final String OAUTH_PATH_SUFFIX = "oauthPath";
+
+ /** OAuth2 client credential, often {@code client_id:client_secret}. */
+ public static final String OAUTH_CREDENTIAL_SUFFIX = "oauthCredential";
+
+ /** OAuth2 scope. */
+ public static final String OAUTH_SCOPE_SUFFIX = "oauthScope";
+
+ /**
+ * Maps camelCase {@code gravitino.conf} suffixes to {@code updateOptions} /
updater-option short
+ * names consumed by {@code GravitinoAuthSettings}.
+ */
+ private static final Map<String, String> GRAVITINO_AUTH_TO_UPDATE_OPTIONS =
+ ImmutableMap.<String, String>builder()
+ .put(AUTH_TYPE_SUFFIX, "auth_type")
+ .put(USERNAME_SUFFIX, "username")
+ .put(PASSWORD_SUFFIX, "password")
+ .put(OAUTH_SERVER_URI_SUFFIX, "oauth_server_uri")
+ .put(OAUTH_PATH_SUFFIX, "oauth_path")
+ .put(OAUTH_CREDENTIAL_SUFFIX, "oauth_credential")
+ .put(OAUTH_SCOPE_SUFFIX, "oauth_scope")
+ .build();
+
+ public static final ConfigEntry<Optional<String>>
GRAVITINO_MAINTENANCE_GRAVITINO_AUTH_TYPE =
+ optionalAuth(
+ GRAVITINO_AUTH_PREFIX,
+ AUTH_TYPE_SUFFIX,
+ "TMS Gravitino client auth mode (none, simple, basic, or oauth).");
+
+ public static final ConfigEntry<Optional<String>>
GRAVITINO_MAINTENANCE_GRAVITINO_AUTH_USERNAME =
+ optionalAuth(GRAVITINO_AUTH_PREFIX, USERNAME_SUFFIX, "TMS Gravitino
client user principal.");
+
+ public static final ConfigEntry<Optional<String>>
GRAVITINO_MAINTENANCE_GRAVITINO_AUTH_PASSWORD =
+ optionalAuth(
+ GRAVITINO_AUTH_PREFIX, PASSWORD_SUFFIX, "TMS Gravitino client HTTP
basic password.");
+
+ public static final ConfigEntry<Optional<String>>
+ GRAVITINO_MAINTENANCE_GRAVITINO_AUTH_OAUTH_SERVER_URI =
+ optionalAuth(
+ GRAVITINO_AUTH_PREFIX,
+ OAUTH_SERVER_URI_SUFFIX,
+ "TMS Gravitino client OAuth2 token server URI.");
+
+ public static final ConfigEntry<Optional<String>>
+ GRAVITINO_MAINTENANCE_GRAVITINO_AUTH_OAUTH_PATH =
+ optionalAuth(
+ GRAVITINO_AUTH_PREFIX, OAUTH_PATH_SUFFIX, "TMS Gravitino client
OAuth2 token path.");
+
+ public static final ConfigEntry<Optional<String>>
+ GRAVITINO_MAINTENANCE_GRAVITINO_AUTH_OAUTH_CREDENTIAL =
+ optionalAuth(
+ GRAVITINO_AUTH_PREFIX,
+ OAUTH_CREDENTIAL_SUFFIX,
+ "TMS Gravitino client OAuth2 client credential.");
+
+ public static final ConfigEntry<Optional<String>>
+ GRAVITINO_MAINTENANCE_GRAVITINO_AUTH_OAUTH_SCOPE =
+ optionalAuth(
+ GRAVITINO_AUTH_PREFIX, OAUTH_SCOPE_SUFFIX, "TMS Gravitino client
OAuth2 scope.");
+
+ private MaintenanceAuthConfigs() {}
+
+ /**
+ * Loads fixed Gravitino auth entries from server config and remaps
camelCase conf suffixes to
+ * updater short names for {@code updateOptions}.
+ *
+ * @param config gravitino server config
+ * @return unmodifiable map keyed by updater short names; empty when unset
+ */
+ public static Map<String, String> loadGravitinoAuth(Config config) {
+ Map<String, String> loaded = loadAuthBag(config, GRAVITINO_AUTH_PREFIX);
+ if (loaded.isEmpty()) {
+ return Collections.emptyMap();
+ }
+ Map<String, String> updateOptions = new LinkedHashMap<>();
+ GRAVITINO_AUTH_TO_UPDATE_OPTIONS.forEach(
+ (suffix, updateKey) -> {
+ String value = loaded.get(suffix);
+ if (value != null) {
+ updateOptions.put(updateKey, value);
+ }
+ });
+ loaded.forEach(
+ (suffix, value) -> {
+ if (!GRAVITINO_AUTH_TO_UPDATE_OPTIONS.containsKey(suffix)) {
+ updateOptions.put(suffix, value);
Review Comment:
This makes the documented fixed schema open-ended: a typo such as `authTyp`
is forwarded unchanged, while `auth_type` remains absent and the downstream
client defaults to `none`. `loadAuthBag` also accepts an empty suffix. Could we
reject unknown Gravitino-auth suffixes and blank suffixes, and add tests for
both? Ideally, supported `authType` values and their required fields should
also be validated while loading the server config so a bad cluster-static
setting fails fast instead of failing in a submitted job.
--
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]
