Skip to content

HDDS-14380. The user who starts Recon process will have administrator privilege #9627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
echonesis wants to merge 3 commits into
base: master
Choose a base branch
from
+119 −22
Open

HDDS-14380. The user who starts Recon process will have administrator privilege #9627

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd073df
HDDS-14380. The user who starts Recon process will have administrator…
echonesis Jan 9, 2026
19cbd26
fix: CR update
echonesis Jan 18, 2026
75dca98
fix: CR update
echonesis Jan 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconControllerModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,15 @@ public class ReconControllerModule extends AbstractModule {
private static final Logger LOG =
LoggerFactory.getLogger(ReconControllerModule.class);

private final ReconServer reconServer;

public ReconControllerModule(ReconServer reconServer) {
this.reconServer = reconServer;
}

@Override
protected void configure() {
bind(ReconServer.class).toInstance(reconServer);
Copy link
Contributor

@devmadhuu devmadhuu Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we need reconServer instance to be passes explicitly to ReconControllerModule ? can we not use Singleton ?

Copy link
Contributor Author

@echonesis echonesis Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follows the same pattern as OM and SCM. In OM, the starter user and admin information are stored as instance fields in the OzoneManager object itself (https://github.com/apache/ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L680-L682). Similarly, we store this information in the ReconServer instance and make it accessible to other components via Guice.

Using bind(ReconServer.class).toInstance(reconServer) allows other components to inject and access the starter user information, just like how OM exposes this through its instance methods.

Copy link
Contributor

@devmadhuu devmadhuu Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, your implementation is correct for the current architecture. The toInstance() pattern is appropriate here because ReconServer is created and initialized before Guice, and we need to bind the specific initialized instance. While OM/SCM store admin info similarly, they don't use Guice, so in a way, the comparison isn't directly applicable.

Copy link
Contributor Author

@echonesis echonesis Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification!

bind(OzoneConfiguration.class).toProvider(ConfigurationProvider.class);
bind(ReconHttpServer.class).in(Singleton.class);
bind(ReconStorageConfig.class).in(Singleton.class);
Expand Down
48 changes: 47 additions & 1 deletion hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,19 @@
import com.google.inject.Injector;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.Collection;
import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.sql.DataSource;
import org.apache.hadoop.hdds.cli.GenericCli;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB;
import org.apache.hadoop.hdds.recon.ReconConfig;
import org.apache.hadoop.hdds.recon.ReconConfigKeys;
import org.apache.hadoop.hdds.scm.server.OzoneStorageContainerManager;
import org.apache.hadoop.hdds.security.SecurityConfig;
import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
import org.apache.hadoop.hdds.server.OzoneAdmins;
import org.apache.hadoop.hdds.utils.HddsServerUtil;
import org.apache.hadoop.ozone.OzoneSecurityUtil;
import org.apache.hadoop.ozone.recon.api.types.FeatureProvider;
Expand Down Expand Up @@ -85,6 +88,7 @@ public class ReconServer extends GenericCli implements Callable<Void> {
private ReconStorageConfig reconStorage;
private CertificateClient certClient;
private ReconTaskStatusMetrics reconTaskStatusMetrics;
private OzoneAdmins reconAdmins;

private volatile boolean isStarted = false;

Expand All @@ -104,9 +108,23 @@ public Void call() throws Exception {
ReconServer.class, originalArgs, LOG, configuration);
ConfigurationProvider.setConfiguration(configuration);

Copy link
Contributor

@priyeshkaratha priyeshkaratha Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call to UserGroupInformation.getCurrentUser() can technically throw an IOException if there are issues with the Kerberos ticket or OS-level user retrieval.

So I am suggesting to ensure the ReconServer startup sequence gracefully handles or logs a clear error message if the current user cannot be identified

Copy link
Contributor Author

@echonesis echonesis Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
I update it as expected in the latest commit.

String reconStarterUser = UserGroupInformation.getCurrentUser().getShortUserName();
Collection<String> adminUsers =
OzoneAdmins.getOzoneAdminsFromConfig(configuration, reconStarterUser);
Copy link
Contributor

@devmadhuu devmadhuu Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of calling Admins and AdminsGroups method separately here, can we use this method directly and then add recon admins and recon admin groups. this will minimise the code lines.

Copy link
Contributor Author

@echonesis echonesis Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion!
Since we need to add Recon-specific admins from OZONE_RECON_ADMINISTRATORS and OZONE_RECON_ADMINISTRATORS_GROUPS, using addAll() seemed like a straightforward approach.

Copy link
Contributor

@devmadhuu devmadhuu Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, can we refactor and extract to a helper method instead of inline code directly in call() method:

private OzoneAdmins createReconAdmins(OzoneConfiguration conf) {
  String starterUser = UserGroupInformation.getCurrentUser().getShortUserName();
  Collection<String> adminUsers =
      OzoneAdmins.getOzoneAdminsFromConfig(conf, starterUser);
  adminUsers.addAll(
      conf.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS));
  
  Collection<String> adminGroups =
      OzoneAdmins.getOzoneAdminsGroupsFromConfig(conf);
  adminGroups.addAll(
      conf.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS_GROUPS));
  
  return new OzoneAdmins(adminUsers, adminGroups);
}

Copy link
Contributor Author

@echonesis echonesis Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
I add it as expected in the latest commit.

adminUsers.addAll(
configuration.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS));

Collection<String> adminGroups =
OzoneAdmins.getOzoneAdminsGroupsFromConfig(configuration);
adminGroups.addAll(
configuration.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS_GROUPS));

reconAdmins = new OzoneAdmins(adminUsers, adminGroups);
LOG.info("Recon start with adminUsers: {}", reconAdmins.getAdminUsernames());

LOG.info("Initializing Recon server...");
try {
injector = Guice.createInjector(new ReconControllerModule(),
injector = Guice.createInjector(new ReconControllerModule(this),
new ReconRestServletModule(configuration),
new ReconSchemaGenerationModule());

Expand Down Expand Up @@ -427,4 +445,32 @@ public ReconTaskController getReconTaskController() {
ReconHttpServer getHttpServer() {
return httpServer;
}

/**
* Get the collection of Recon admin usernames.
*
* @return Collection of admin usernames
*/
public Collection<String> getReconAdminUsernames() {
return reconAdmins.getAdminUsernames();
}

/**
* Get the collection of Recon admin groups.
*
* @return Collection of admin groups
*/
public Collection<String> getReconAdminGroups() {
Copy link
Contributor

@devmadhuu devmadhuu Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific usecase or need to define these admin and admin group getter methods ?

Copy link
Contributor Author

@echonesis echonesis Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @devmadhuu !
These methods follow the same pattern as OM's getOmAdminUsernames() and getOmAdminGroups() for consistency. They're not currently used in Recon.
I will remove them in a follow-up commit if preferred.

return reconAdmins.getAdminGroups();
}

/**
* Check if a user is a Recon administrator.
*
* @param user UserGroupInformation
* @return true if the user is an admin, false otherwise
*/
public boolean isAdmin(UserGroupInformation user) {
return reconAdmins.isAdmin(user);
}
}
27 changes: 8 additions & 19 deletions ...ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/filters/ReconAdminFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.inject.Singleton;
import java.io.IOException;
import java.security.Principal;
import java.util.Collection;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
Expand All @@ -30,10 +29,7 @@
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.recon.ReconConfigKeys;
import org.apache.hadoop.hdds.server.OzoneAdmins;
import org.apache.hadoop.ozone.OzoneConfigKeys;
import org.apache.hadoop.ozone.recon.ReconServer;
import org.apache.hadoop.security.UserGroupInformation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -48,15 +44,17 @@ public class ReconAdminFilter implements Filter {
private static final Logger LOG =
LoggerFactory.getLogger(ReconAdminFilter.class);

private final OzoneConfiguration conf;
private final ReconServer reconServer;

@Inject
ReconAdminFilter(OzoneConfiguration conf) {
this.conf = conf;
ReconAdminFilter(ReconServer reconServer) {
this.reconServer = reconServer;
}

@Override
public void init(FilterConfig filterConfig) throws ServletException { }
public void init(FilterConfig filterConfig) throws ServletException {
LOG.info("ReconAdminFilter initialized");
}

@Override
public void doFilter(ServletRequest servletRequest,
Expand Down Expand Up @@ -100,15 +98,6 @@ public void doFilter(ServletRequest servletRequest,
public void destroy() { }

private boolean hasPermission(UserGroupInformation user) {
Collection<String> admins =
conf.getStringCollection(OzoneConfigKeys.OZONE_ADMINISTRATORS);
admins.addAll(
conf.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS));
Collection<String> adminGroups =
conf.getStringCollection(OzoneConfigKeys.OZONE_ADMINISTRATORS_GROUPS);
adminGroups.addAll(
conf.getStringCollection(
ReconConfigKeys.OZONE_RECON_ADMINISTRATORS_GROUPS));
return new OzoneAdmins(admins, adminGroups).isAdmin(user);
return reconServer.isAdmin(user);
}
}
60 changes: 58 additions & 2 deletions ...-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/filters/TestAdminFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.google.common.collect.Sets;
import java.io.IOException;
import java.security.Principal;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
import javax.servlet.FilterChain;
Expand All @@ -34,7 +37,9 @@
import javax.ws.rs.Path;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.recon.ReconConfigKeys;
import org.apache.hadoop.hdds.server.OzoneAdmins;
import org.apache.hadoop.ozone.OzoneConfigKeys;
import org.apache.hadoop.ozone.recon.ReconServer;
import org.apache.hadoop.ozone.recon.api.AdminOnly;
import org.apache.hadoop.ozone.recon.api.ClusterStateEndpoint;
import org.apache.hadoop.ozone.recon.api.MetricsProxyEndpoint;
Expand Down Expand Up @@ -170,8 +175,31 @@ public void testAdminFilterNoAdmins() throws Exception {
testAdminFilterWithPrincipal(new OzoneConfiguration(), "reject", false);
}

@Test
public void testAdminFilterStarterUserAutoAdmin() throws Exception {
OzoneConfiguration conf = new OzoneConfiguration();
String currentUser = UserGroupInformation.getCurrentUser().getShortUserName();

testAdminFilterWithPrincipal(conf, currentUser, true);
testAdminFilterWithPrincipal(conf, "otheruser", false);
}

@Test
public void testAdminFilterStarterUserPlusConfiguredAdmins() throws Exception {
OzoneConfiguration conf = new OzoneConfiguration();
conf.setStrings(OzoneConfigKeys.OZONE_ADMINISTRATORS, "configadmin");

String currentUser = UserGroupInformation.getCurrentUser().getShortUserName();

testAdminFilterWithPrincipal(conf, currentUser, true);
testAdminFilterWithPrincipal(conf, "configadmin", true);
testAdminFilterWithPrincipal(conf, "reject", false);
}

private void testAdminFilterWithPrincipal(OzoneConfiguration conf,
String principalToUse, boolean shouldPass) throws Exception {
ReconServer mockReconServer = createMockReconServer(conf);

Principal mockPrincipal = mock(Principal.class);
when(mockPrincipal.getName()).thenReturn(principalToUse);
HttpServletRequest mockRequest = mock(HttpServletRequest.class);
Expand All @@ -180,13 +208,41 @@ private void testAdminFilterWithPrincipal(OzoneConfiguration conf,
HttpServletResponse mockResponse = mock(HttpServletResponse.class);
FilterChain mockFilterChain = mock(FilterChain.class);

new ReconAdminFilter(conf).doFilter(mockRequest, mockResponse,
mockFilterChain);
ReconAdminFilter filter = new ReconAdminFilter(mockReconServer);
filter.init(null);
filter.doFilter(mockRequest, mockResponse, mockFilterChain);

if (shouldPass) {
verify(mockFilterChain).doFilter(mockRequest, mockResponse);
} else {
verify(mockResponse).setStatus(HttpServletResponse.SC_FORBIDDEN);
}
}

/**
* Creates a mock ReconServer that mimics the actual admin initialization logic.
*/
private ReconServer createMockReconServer(OzoneConfiguration conf) throws IOException {
String reconStarterUser = UserGroupInformation.getCurrentUser().getShortUserName();
Collection<String> adminUsers =
OzoneAdmins.getOzoneAdminsFromConfig(conf, reconStarterUser);
adminUsers.addAll(
conf.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS));

Collection<String> adminGroups =
OzoneAdmins.getOzoneAdminsGroupsFromConfig(conf);
adminGroups.addAll(
conf.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS_GROUPS));

OzoneAdmins reconAdmins = new OzoneAdmins(adminUsers, adminGroups);

ReconServer mockReconServer = mock(ReconServer.class);
when(mockReconServer.isAdmin(any(UserGroupInformation.class)))
.thenAnswer(invocation -> {
UserGroupInformation user = invocation.getArgument(0);
return reconAdmins.isAdmin(user);
});

return mockReconServer;
}
}