github-actions[bot] commented on code in PR #68220: URL: https://github.com/apache/doris/pull/68220#discussion_r4057024571
########## fe/fe-authorization/fe-authorization-plugins/fe-authorization-plugin-ranger-common/src/main/java/org/apache/doris/catalog/authorizer/ranger/RangerUserStoreGroups.java: ########## @@ -0,0 +1,143 @@ +// 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.doris.catalog.authorizer.ranger; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.ranger.authorization.hadoop.config.RangerPluginConfig; +import org.apache.ranger.plugin.contextenricher.RangerAdminUserStoreRetriever; +import org.apache.ranger.plugin.contextenricher.RangerUserStoreEnricher; +import org.apache.ranger.plugin.policyengine.RangerPluginContext; +import org.apache.ranger.plugin.service.RangerAuthContext; +import org.apache.ranger.plugin.util.RangerUserStoreUtil; +import org.apache.ranger.plugin.util.ServiceDefUtil; +import org.apache.ranger.plugin.util.ServicePolicies; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** + * Where a request built by a Doris source gets the groups a Ranger policy item may be written against. + * + * <p>A Ranger policy item names users, groups and roles, and Ranger's own plugins fill in the groups from + * the service they run in - the Hive plugin asks Hadoop's group mapping. Doris has nothing to ask: an + * account holds roles, and no part of the engine knows which groups a user is in. So a request that carries + * no groups matches no group item at all, and a service whose policies are kept by group - the normal way + * to run Ranger, so that nobody edits a policy each time somebody joins a team - grants nothing to a Doris + * user; and, worse, denies nothing either. An item that denies a group is exactly as silent about it. + * + * <p>What Ranger provides for a plugin in that position is its <b>user store</b>: the users and groups Ranger + * Admin itself holds, kept current by usersync from LDAP or the OS, which a plugin downloads next to its + * policies through a {@link RangerUserStoreEnricher}. Ranger 2.5 and later reads the groups of the request's + * user out of it when {@code ranger.plugin.<type>.use.rangerGroups} is set; the two sources Doris embeds do + * the same thing here, in the request builder, so that it neither depends on the Ranger version they were + * built against - branches still on 2.4 have no such setting - nor on an operator finding a Ranger property + * no Doris document names. + * + * <p>It is on by default, because a group item that is silently ignored is a bug in the deployment's eyes, + * not a behaviour anybody opted into; the property Ranger reads for this is the one that switches it off, + * so that a deployment which has already decided the question in Ranger's terms has decided it here too. + * Switched off, requests carry no groups and this source matches what it matched before. + */ +public final class RangerUserStoreGroups { + private static final Logger LOG = LogManager.getLogger(RangerUserStoreGroups.class); + + /** + * Suffix of the property switching this off, {@code ranger.plugin.<type>.use.rangerGroups} in full: the + * name Ranger itself reads for the same thing, so that one setting decides both. + */ + public static final String USE_RANGER_GROUPS = ".use.rangerGroups"; + + /** How often the plugin asks Ranger Admin for a newer user store, when nothing configures it. */ + private static final String DEFAULT_REFRESH_INTERVAL_MS = Integer.toString(60 * 1000); + + private RangerUserStoreGroups() { + } + + /** Whether the plugin configured by {@code config} attaches user store groups; on unless switched off. */ + public static boolean enabledFor(RangerPluginConfig config) { + // No configuration at all - a plugin stubbed out in a test - has not switched anything off. + return config == null || config.getBoolean(config.getPropertyPrefix() + USE_RANGER_GROUPS, true); + } + + /** + * Puts a user store enricher on the service definition Ranger just downloaded, unless one is there or + * this is switched off, so that the plugin fetches and refreshes the user store {@link #groupsOf} reads. + * + * <p>Called by {@link BackgroundLoadedRangerPlugin#setPolicies} before handing the policies on, on every + * call and not only the first: a download of policy deltas comes with its own copy of the service + * definition, which is why {@code RangerBasePlugin} re-adds the enricher on deltas too. The retriever + * class and the refresh interval are read under the option names Ranger itself reads them under, so that + * an operator who has tuned them for Ranger's own {@code use.rangerGroups} has tuned them here. + */ + public static void addUserStoreEnricher(RangerPluginConfig config, ServicePolicies policies) { + if (policies == null || config == null || !enabledFor(config)) { + return; + } + String retriever = config.get(RangerUserStoreEnricher.USERSTORE_RETRIEVER_CLASSNAME_OPTION, + RangerAdminUserStoreRetriever.class.getCanonicalName()); + String refreshIntervalMs = config.get(RangerUserStoreEnricher.USERSTORE_REFRESHER_POLLINGINTERVAL_OPTION, + DEFAULT_REFRESH_INTERVAL_MS); + // Ranger logs the addition itself, once per download that needed it; the operator-facing line about + // why the store is downloaded at all is the plugin's, written once when it starts (see describe). + if (ServiceDefUtil.addUserStoreEnricher(policies, retriever, refreshIntervalMs) && LOG.isDebugEnabled()) { + LOG.debug("Ranger service {} will download its user store every {} ms", policies.getServiceName(), + refreshIntervalMs); + } + } + + /** One line for the plugin's start-up log, saying what this does for it and how to switch it off. */ + public static String describe(RangerPluginConfig config) { + String property = config.getPropertyPrefix() + USE_RANGER_GROUPS; + return enabledFor(config) + ? "Ranger service " + config.getServiceName() + ": requests carry the groups Ranger's user store" + + " puts the user in, so that policy items written against a group apply; set " + + property + "=false to switch that off" + : "Ranger service " + config.getServiceName() + ": " + property + "=false, so requests carry no" + + " groups and policy items written against a group never apply"; + } + + /** + * The groups the user store {@code plugin} has downloaded puts {@code user} in. + * + * <p>Read once the plugin's first load has ended, since the store arrives with it: asked earlier, this + * would say "no groups" about a user the store is about to put in several. Empty when this is switched + * off, when no store arrived - Ranger Admin could not be reached, and nothing was cached - and when the + * store does not know the user, which is the case for every account that exists in Doris only. Empty + * and not null on purpose: a request with an empty group set matches items written against users and + * roles exactly as it did before. + */ + public static Set<String> groupsOf(BackgroundLoadedRangerPlugin plugin, String user) { + if (user == null || !enabledFor(plugin.getConfig())) { + return Collections.emptySet(); + } + plugin.awaitLoaded(); + // The auth context the policy engine publishes is where Ranger's own request processing reads the + // store from; it is replaced together with the engine, and the store is carried over when it is. + RangerPluginContext pluginContext = plugin.getPluginContext(); + RangerAuthContext authContext = pluginContext == null ? null : pluginContext.getAuthContext(); + RangerUserStoreUtil userStore = authContext == null ? null : authContext.getUserStoreUtil(); + Set<String> groups = userStore == null ? null : userStore.getUserGroups(user); + if (groups == null || groups.isEmpty()) { Review Comment: [P1] Do not treat an unavailable user store as no groups. A failed initial Ranger user-store retrieval can leave the policy engine live with an empty-backed store. Returning `Collections.emptySet()` here removes group deny matches, so the direct-user allow in the mixed allow/group-deny case can win during the outage. Please track user-store validity separately and fail closed or retain an explicitly valid last-known snapshot. ########## fe/fe-authorization/fe-authorization-plugins/fe-authorization-plugin-ranger-common/src/main/java/org/apache/doris/catalog/authorizer/ranger/RangerUserStoreGroups.java: ########## @@ -0,0 +1,143 @@ +// 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.doris.catalog.authorizer.ranger; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.ranger.authorization.hadoop.config.RangerPluginConfig; +import org.apache.ranger.plugin.contextenricher.RangerAdminUserStoreRetriever; +import org.apache.ranger.plugin.contextenricher.RangerUserStoreEnricher; +import org.apache.ranger.plugin.policyengine.RangerPluginContext; +import org.apache.ranger.plugin.service.RangerAuthContext; +import org.apache.ranger.plugin.util.RangerUserStoreUtil; +import org.apache.ranger.plugin.util.ServiceDefUtil; +import org.apache.ranger.plugin.util.ServicePolicies; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** + * Where a request built by a Doris source gets the groups a Ranger policy item may be written against. + * + * <p>A Ranger policy item names users, groups and roles, and Ranger's own plugins fill in the groups from + * the service they run in - the Hive plugin asks Hadoop's group mapping. Doris has nothing to ask: an + * account holds roles, and no part of the engine knows which groups a user is in. So a request that carries + * no groups matches no group item at all, and a service whose policies are kept by group - the normal way + * to run Ranger, so that nobody edits a policy each time somebody joins a team - grants nothing to a Doris + * user; and, worse, denies nothing either. An item that denies a group is exactly as silent about it. + * + * <p>What Ranger provides for a plugin in that position is its <b>user store</b>: the users and groups Ranger + * Admin itself holds, kept current by usersync from LDAP or the OS, which a plugin downloads next to its + * policies through a {@link RangerUserStoreEnricher}. Ranger 2.5 and later reads the groups of the request's + * user out of it when {@code ranger.plugin.<type>.use.rangerGroups} is set; the two sources Doris embeds do + * the same thing here, in the request builder, so that it neither depends on the Ranger version they were + * built against - branches still on 2.4 have no such setting - nor on an operator finding a Ranger property + * no Doris document names. + * + * <p>It is on by default, because a group item that is silently ignored is a bug in the deployment's eyes, + * not a behaviour anybody opted into; the property Ranger reads for this is the one that switches it off, + * so that a deployment which has already decided the question in Ranger's terms has decided it here too. + * Switched off, requests carry no groups and this source matches what it matched before. + */ +public final class RangerUserStoreGroups { + private static final Logger LOG = LogManager.getLogger(RangerUserStoreGroups.class); + + /** + * Suffix of the property switching this off, {@code ranger.plugin.<type>.use.rangerGroups} in full: the + * name Ranger itself reads for the same thing, so that one setting decides both. + */ + public static final String USE_RANGER_GROUPS = ".use.rangerGroups"; + + /** How often the plugin asks Ranger Admin for a newer user store, when nothing configures it. */ + private static final String DEFAULT_REFRESH_INTERVAL_MS = Integer.toString(60 * 1000); + + private RangerUserStoreGroups() { + } + + /** Whether the plugin configured by {@code config} attaches user store groups; on unless switched off. */ + public static boolean enabledFor(RangerPluginConfig config) { + // No configuration at all - a plugin stubbed out in a test - has not switched anything off. + return config == null || config.getBoolean(config.getPropertyPrefix() + USE_RANGER_GROUPS, true); + } + + /** + * Puts a user store enricher on the service definition Ranger just downloaded, unless one is there or + * this is switched off, so that the plugin fetches and refreshes the user store {@link #groupsOf} reads. + * + * <p>Called by {@link BackgroundLoadedRangerPlugin#setPolicies} before handing the policies on, on every + * call and not only the first: a download of policy deltas comes with its own copy of the service + * definition, which is why {@code RangerBasePlugin} re-adds the enricher on deltas too. The retriever + * class and the refresh interval are read under the option names Ranger itself reads them under, so that + * an operator who has tuned them for Ranger's own {@code use.rangerGroups} has tuned them here. + */ + public static void addUserStoreEnricher(RangerPluginConfig config, ServicePolicies policies) { + if (policies == null || config == null || !enabledFor(config)) { + return; + } + String retriever = config.get(RangerUserStoreEnricher.USERSTORE_RETRIEVER_CLASSNAME_OPTION, + RangerAdminUserStoreRetriever.class.getCanonicalName()); + String refreshIntervalMs = config.get(RangerUserStoreEnricher.USERSTORE_REFRESHER_POLLINGINTERVAL_OPTION, + DEFAULT_REFRESH_INTERVAL_MS); + // Ranger logs the addition itself, once per download that needed it; the operator-facing line about + // why the store is downloaded at all is the plugin's, written once when it starts (see describe). + if (ServiceDefUtil.addUserStoreEnricher(policies, retriever, refreshIntervalMs) && LOG.isDebugEnabled()) { + LOG.debug("Ranger service {} will download its user store every {} ms", policies.getServiceName(), + refreshIntervalMs); + } + } + + /** One line for the plugin's start-up log, saying what this does for it and how to switch it off. */ + public static String describe(RangerPluginConfig config) { + String property = config.getPropertyPrefix() + USE_RANGER_GROUPS; + return enabledFor(config) + ? "Ranger service " + config.getServiceName() + ": requests carry the groups Ranger's user store" + + " puts the user in, so that policy items written against a group apply; set " + + property + "=false to switch that off" + : "Ranger service " + config.getServiceName() + ": " + property + "=false, so requests carry no" + + " groups and policy items written against a group never apply"; + } + + /** + * The groups the user store {@code plugin} has downloaded puts {@code user} in. + * + * <p>Read once the plugin's first load has ended, since the store arrives with it: asked earlier, this + * would say "no groups" about a user the store is about to put in several. Empty when this is switched + * off, when no store arrived - Ranger Admin could not be reached, and nothing was cached - and when the + * store does not know the user, which is the case for every account that exists in Doris only. Empty + * and not null on purpose: a request with an empty group set matches items written against users and + * roles exactly as it did before. + */ + public static Set<String> groupsOf(BackgroundLoadedRangerPlugin plugin, String user) { + if (user == null || !enabledFor(plugin.getConfig())) { + return Collections.emptySet(); + } + plugin.awaitLoaded(); + // The auth context the policy engine publishes is where Ranger's own request processing reads the + // store from; it is replaced together with the engine, and the store is carried over when it is. + RangerPluginContext pluginContext = plugin.getPluginContext(); Review Comment: [P1] Publish the user-store snapshot atomically. This path reads `RangerPluginContext.authContext` and `RangerAuthContext.userStoreUtil` directly, but Ranger's user-store refresher replaces those plain fields on another thread. Without a happens-before edge or versioned immutable snapshot, requests can retain revoked group membership (or an empty snapshot) after refresh. Please use a supported volatile/atomic publication boundary before attaching groups to authorization requests. ########## fe/fe-authorization/fe-authorization-plugins/fe-authorization-plugin-ranger-common/src/main/java/org/apache/doris/catalog/authorizer/ranger/RangerAccessController.java: ########## @@ -35,7 +35,6 @@ import org.apache.ranger.plugin.policyengine.RangerAccessResourceImpl; import org.apache.ranger.plugin.policyengine.RangerAccessResult; import org.apache.ranger.plugin.policyengine.RangerAccessResultProcessor; -import org.apache.ranger.plugin.service.RangerBasePlugin; Review Comment: [P2] Check the closed fence before the unbounded load wait. `checkPrivilege`, `getRowFilters`, and `getDataMasks` all await the plugin's initial Ranger load before consulting `closed`. If a catalog is detached while that load is blocked, a stale controller reference waits through the full REST timeout instead of refusing promptly, even though cleanup intentionally returns while loading. Please check closure before waiting or make the wait cancellable by lifecycle closure, while retaining the read-lock fence before plugin calls. ########## fe/fe-authorization/fe-authorization-plugins/fe-authorization-plugin-ranger-common/src/main/java/org/apache/doris/catalog/authorizer/ranger/BackgroundLoadedRangerPlugin.java: ########## @@ -0,0 +1,245 @@ +// 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.doris.catalog.authorizer.ranger; + +import com.google.common.annotations.VisibleForTesting; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.ranger.plugin.policyengine.RangerAccessRequest; +import org.apache.ranger.plugin.policyengine.RangerAccessResult; +import org.apache.ranger.plugin.policyengine.RangerAccessResultProcessor; +import org.apache.ranger.plugin.service.RangerBasePlugin; +import org.apache.ranger.plugin.util.ServicePolicies; + +import java.util.Collection; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +/** + * The plugin a Doris Ranger source answers out of: a {@link RangerBasePlugin} that loads without holding up + * whoever built it, and whose requests can carry the groups Ranger keeps for a user. + * + * <p><b>The first load runs on a thread of its own.</b> {@code RangerBasePlugin.init()} does not return until + * it has downloaded the service's roles and its policies from the Ranger admin - and, because the policies + * arrive with the user store enricher on them (below), the user store as well: three REST calls, bounded + * only by the plugin's REST timeouts and retries ({@code policy.rest.client.connection.timeoutMs} is two + * minutes by default, {@code read.timeoutMs} thirty seconds). The plugin behind + * {@code access_controller_type=ranger-doris} is built in the {@code Env} constructor, on the thread starting + * the FE, so that used to be time an FE could not start for: a user store the size of an enterprise + * directory, or an admin that is slow or unreachable, held the whole process. Here {@link #init()} starts + * that load and returns, and {@link #awaitLoaded()} is where + * whoever needs its outcome waits - which every answer this plugin gives does first, so a check made before + * the load has ended is answered after it, never out of an engine that has nothing in it yet. Whatever else + * the FE does to start runs meanwhile, and the check that pays for Ranger's latency is the first one that + * needs Ranger, not every process that embeds it. + * + * <p>What a check waits for is exactly what the constructor used to guarantee: that the load has + * <em>ended</em> - with the policies from the admin, or from the local cache when the admin could not be + * reached, or with nothing at all, in which case the engine is null and {@code RangerAccessController} + * refuses, as it always has. It is not shortened by a timeout of its own: the load is bounded by the REST + * timeouts the operator already tunes, and answering out of an empty engine before it has ended would be + * refusing checks the policies are about to allow - and, worse, passing ones a policy written against a + * group is about to deny. + * + * <p>Stopping it is {@link #cleanup()}, as before. Stopped while still loading - a {@code CREATE CATALOG} + * dry run, the loser of a race in a factory - it finishes the load first and stops itself then, on the + * loading thread: what is running is a REST call, which an interrupt does not cut short, so waiting for it + * here would put the whole REST timeout back onto the thread closing a catalog. + * + * <p><b>Requests carry Ranger's own groups.</b> Doris has none to offer, and a policy item written against a + * group matches nothing without them; the store they are read from is asked for with the policies, see + * {@link RangerUserStoreGroups}. + * + * <p>A plugin that is never {@link #init() initialized} - a test's, answering out of overrides of its own - + * has nothing to wait for, and {@link #awaitLoaded()} returns at once. + */ +public abstract class BackgroundLoadedRangerPlugin extends RangerBasePlugin { + private static final Logger LOG = LogManager.getLogger(BackgroundLoadedRangerPlugin.class); + + /** Released once the first load has ended, however it ended. */ + private final CountDownLatch loaded = new CountDownLatch(1); + /** The thread running the first load, from the moment {@link #init()} starts it. */ + private final AtomicReference<Thread> loader = new AtomicReference<>(); + /** Whether {@link #cleanup()} has been asked for; read by the loader when the load ends. */ + private volatile boolean stopRequested; + /** Whether the stop has run. It runs once, from whichever of {@link #cleanup()} and the loader is last. */ + private final AtomicBoolean stopped = new AtomicBoolean(); + + protected BackgroundLoadedRangerPlugin(String serviceType, String serviceName, String appId) { + super(serviceType, serviceName, appId); + } + + /** + * Starts the first load - {@code RangerBasePlugin.init()}, roles, policies and user store - on a thread + * of its own, and returns at once. Once per plugin. + */ + @Override + public void init() { + Thread thread = new Thread(this::load, "RangerPluginLoader(serviceType=" + getServiceType() + + ", serviceName=" + getServiceName() + ")"); + if (!loader.compareAndSet(null, thread)) { + throw new IllegalStateException("Ranger plugin for service " + getServiceName() + + " has already been initialized"); + } + LOG.info(RangerUserStoreGroups.describe(getConfig())); + // A daemon: it ends with the load, and a load still running when the FE exits is not worth waiting + // for. What it inherits, and what the refresher threads it goes on to start inherit from it, is the + // context classloader of the thread calling this - the plugin's own, set by the engine around the + // factory call - which is what keeps Ranger's class-name lookups working after that call returns. + thread.setDaemon(true); + thread.start(); + } + + private void load() { + long startedAtNanos = System.nanoTime(); + try { + if (stopRequested) { + // Stopped before the load began; there is nothing to load for. + return; + } + firstLoad(); + LOG.info("Ranger service {} loaded in {} ms: policies version {}, roles version {}, user store" + + " version {}", getServiceName(), + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startedAtNanos), getPoliciesVersion(), + getRolesVersion(), getUserStoreVersion()); + } catch (Throwable e) { Review Comment: [P1] Refuse after post-publication initialization failures. In Ranger 2.8, the root policy engine is published before configured chained plugins are initialized. If a chained-plugin `init()` throws, this catch still counts down `loaded` and leaves the root engine usable, despite logging that every check will be refused. Please clean up or record a failed state before releasing readiness, and add a failure-after-publication test. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
