Copilot commented on code in PR #620: URL: https://github.com/apache/ranger/pull/620#discussion_r2245998121
########## security-admin/src/main/java/org/apache/ranger/biz/RangerLogLevelService.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.ranger.biz; + +import java.lang.reflect.Method; +import org.slf4j.ILoggerFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +/** + * Service class to handle logging-related operations. + * This class uses reflection to avoid compile-time dependencies on specific + * logging frameworks, allowing it to be portable. + */ +@Component +public class RangerLogLevelService { + + private static final Logger LOG = LoggerFactory.getLogger(RangerLogLevelService.class); + + /** + * Reloads the logging configuration by detecting the underlying SLF4J implementation. + */ + public void reloadLogConfiguration() { + ILoggerFactory iLoggerFactory = LoggerFactory.getILoggerFactory(); + String loggerFactoryClassName = iLoggerFactory.getClass().getName(); + + LOG.info("Detected SLF4J binding: {}", loggerFactoryClassName); + + if (loggerFactoryClassName.startsWith("org.apache.logging.slf4j")) { + reloadLog4j2Configuration(); + } else if (loggerFactoryClassName.startsWith("ch.qos.logback.classic")) { + reloadLogbackConfiguration(iLoggerFactory); + } else { + String message = "Dynamic log configuration reload is not supported for SLF4J binding: " + loggerFactoryClassName; + LOG.warn(message); + throw new UnsupportedOperationException(message); + } + } + + /** + * Reloads the Log4j 2 configuration using reflection. + */ + private void reloadLog4j2Configuration() { + try { + LOG.debug("Attempting to reload Log4j 2 configuration via reflection."); + Class<?> configuratorClass = Class.forName("org.apache.logging.log4j.core.config.Configurator"); + Method reconfigureMethod = configuratorClass.getMethod("reconfigure"); + reconfigureMethod.invoke(null); // Static method call + LOG.info("Successfully triggered Log4j 2 configuration reload."); + } catch (Exception e) { + LOG.error("Failed to reload Log4j 2 configuration using reflection", e); + throw new RuntimeException("Failed to reload Log4j 2 configuration", e); + } + } + + /** + * Reloads the Logback configuration using reflection. + */ + private void reloadLogbackConfiguration(ILoggerFactory iLoggerFactory) { + try { + LOG.debug("Attempting to reload Logback configuration via reflection."); + // Get the context + Object context = iLoggerFactory; // The ILoggerFactory is the LoggerContext in Logback + + // Create a JoranConfigurator instance + Class<?> joranConfiguratorClass = Class.forName("ch.qos.logback.classic.joran.JoranConfigurator"); + Object configurator = joranConfiguratorClass.newInstance(); Review Comment: The newInstance() method is deprecated since Java 9. Consider using joranConfiguratorClass.getDeclaredConstructor().newInstance() instead for better compatibility with newer Java versions. ########## security-admin/src/main/java/org/apache/ranger/rest/LogLevelREST.java: ########## @@ -0,0 +1,70 @@ +/* + * 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.ranger.rest; + +import org.apache.ranger.biz.RangerLogLevelService; +import org.apache.ranger.common.MessageEnums; +import org.apache.ranger.common.RESTErrorUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.context.annotation.Scope; +import org.springframework.stereotype.Component; + +import javax.inject.Inject; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +/** + * REST API for logging-related operations. + */ +@Path("loggers") +@Component +@Scope("singleton") +public class LogLevelREST { + private static final Logger LOG = LoggerFactory.getLogger(LogLevelREST.class); + + @Inject + RangerLogLevelService logLevelService; + + @Inject + RESTErrorUtil restErrorUtil; + + /** + * An endpoint to dynamically reload the logging configuration from the + * log4j2 properties file on the classpath. Review Comment: The comment mentions 'log4j2 properties file' but the implementation supports both Log4j2 and Logback configurations. The comment should be updated to reflect that it reloads logging configuration from the appropriate configuration file (logback.xml or log4j2 configuration). ```suggestion * appropriate configuration file (e.g., logback.xml or a Log4j2 configuration file) * on the classpath. ``` ########## security-admin/src/main/java/org/apache/ranger/biz/RangerLogLevelService.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.ranger.biz; + +import java.lang.reflect.Method; +import org.slf4j.ILoggerFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +/** + * Service class to handle logging-related operations. + * This class uses reflection to avoid compile-time dependencies on specific + * logging frameworks, allowing it to be portable. + */ +@Component +public class RangerLogLevelService { + + private static final Logger LOG = LoggerFactory.getLogger(RangerLogLevelService.class); + + /** + * Reloads the logging configuration by detecting the underlying SLF4J implementation. + */ + public void reloadLogConfiguration() { + ILoggerFactory iLoggerFactory = LoggerFactory.getILoggerFactory(); + String loggerFactoryClassName = iLoggerFactory.getClass().getName(); + + LOG.info("Detected SLF4J binding: {}", loggerFactoryClassName); + + if (loggerFactoryClassName.startsWith("org.apache.logging.slf4j")) { + reloadLog4j2Configuration(); + } else if (loggerFactoryClassName.startsWith("ch.qos.logback.classic")) { Review Comment: The hardcoded SLF4J binding class name prefixes should be extracted as constants to improve maintainability and reduce the risk of typos. ```suggestion if (loggerFactoryClassName.startsWith(SLF4J_LOG4J2_PREFIX)) { reloadLog4j2Configuration(); } else if (loggerFactoryClassName.startsWith(SLF4J_LOGBACK_PREFIX)) { ``` ########## security-admin/src/main/java/org/apache/ranger/biz/RangerLogLevelService.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.ranger.biz; + +import java.lang.reflect.Method; +import org.slf4j.ILoggerFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +/** + * Service class to handle logging-related operations. + * This class uses reflection to avoid compile-time dependencies on specific + * logging frameworks, allowing it to be portable. + */ +@Component +public class RangerLogLevelService { + + private static final Logger LOG = LoggerFactory.getLogger(RangerLogLevelService.class); + + /** + * Reloads the logging configuration by detecting the underlying SLF4J implementation. + */ + public void reloadLogConfiguration() { + ILoggerFactory iLoggerFactory = LoggerFactory.getILoggerFactory(); + String loggerFactoryClassName = iLoggerFactory.getClass().getName(); + + LOG.info("Detected SLF4J binding: {}", loggerFactoryClassName); + + if (loggerFactoryClassName.startsWith("org.apache.logging.slf4j")) { + reloadLog4j2Configuration(); + } else if (loggerFactoryClassName.startsWith("ch.qos.logback.classic")) { + reloadLogbackConfiguration(iLoggerFactory); + } else { + String message = "Dynamic log configuration reload is not supported for SLF4J binding: " + loggerFactoryClassName; + LOG.warn(message); + throw new UnsupportedOperationException(message); + } + } + + /** + * Reloads the Log4j 2 configuration using reflection. + */ + private void reloadLog4j2Configuration() { + try { + LOG.debug("Attempting to reload Log4j 2 configuration via reflection."); + Class<?> configuratorClass = Class.forName("org.apache.logging.log4j.core.config.Configurator"); + Method reconfigureMethod = configuratorClass.getMethod("reconfigure"); + reconfigureMethod.invoke(null); // Static method call + LOG.info("Successfully triggered Log4j 2 configuration reload."); + } catch (Exception e) { + LOG.error("Failed to reload Log4j 2 configuration using reflection", e); + throw new RuntimeException("Failed to reload Log4j 2 configuration", e); + } + } + + /** + * Reloads the Logback configuration using reflection. + */ + private void reloadLogbackConfiguration(ILoggerFactory iLoggerFactory) { + try { + LOG.debug("Attempting to reload Logback configuration via reflection."); + // Get the context + Object context = iLoggerFactory; // The ILoggerFactory is the LoggerContext in Logback + + // Create a JoranConfigurator instance + Class<?> joranConfiguratorClass = Class.forName("ch.qos.logback.classic.joran.JoranConfigurator"); + Object configurator = joranConfiguratorClass.newInstance(); + + // Set the context: configurator.setContext(context) + Method setContextMethod = joranConfiguratorClass.getMethod("setContext", Class.forName("ch.qos.logback.core.Context")); + setContextMethod.invoke(configurator, context); + + // Reset the context: context.reset() + Method resetMethod = context.getClass().getMethod("reset"); + resetMethod.invoke(context); + + // Find the configuration file URL (e.g., logback.xml) + java.net.URL configUrl = this.getClass().getClassLoader().getResource("logback.xml"); Review Comment: Consider validating the configuration file path and ensuring it points to an expected location to prevent potential security issues from loading arbitrary configuration files. ########## security-admin/src/main/java/org/apache/ranger/biz/RangerLogLevelService.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.ranger.biz; + +import java.lang.reflect.Method; +import org.slf4j.ILoggerFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +/** + * Service class to handle logging-related operations. + * This class uses reflection to avoid compile-time dependencies on specific + * logging frameworks, allowing it to be portable. + */ +@Component +public class RangerLogLevelService { + + private static final Logger LOG = LoggerFactory.getLogger(RangerLogLevelService.class); + + /** + * Reloads the logging configuration by detecting the underlying SLF4J implementation. + */ + public void reloadLogConfiguration() { + ILoggerFactory iLoggerFactory = LoggerFactory.getILoggerFactory(); + String loggerFactoryClassName = iLoggerFactory.getClass().getName(); + + LOG.info("Detected SLF4J binding: {}", loggerFactoryClassName); + + if (loggerFactoryClassName.startsWith("org.apache.logging.slf4j")) { + reloadLog4j2Configuration(); + } else if (loggerFactoryClassName.startsWith("ch.qos.logback.classic")) { + reloadLogbackConfiguration(iLoggerFactory); + } else { + String message = "Dynamic log configuration reload is not supported for SLF4J binding: " + loggerFactoryClassName; + LOG.warn(message); + throw new UnsupportedOperationException(message); + } + } + + /** + * Reloads the Log4j 2 configuration using reflection. + */ + private void reloadLog4j2Configuration() { + try { + LOG.debug("Attempting to reload Log4j 2 configuration via reflection."); + Class<?> configuratorClass = Class.forName("org.apache.logging.log4j.core.config.Configurator"); + Method reconfigureMethod = configuratorClass.getMethod("reconfigure"); + reconfigureMethod.invoke(null); // Static method call + LOG.info("Successfully triggered Log4j 2 configuration reload."); + } catch (Exception e) { + LOG.error("Failed to reload Log4j 2 configuration using reflection", e); + throw new RuntimeException("Failed to reload Log4j 2 configuration", e); + } + } + + /** + * Reloads the Logback configuration using reflection. + */ + private void reloadLogbackConfiguration(ILoggerFactory iLoggerFactory) { + try { + LOG.debug("Attempting to reload Logback configuration via reflection."); + // Get the context + Object context = iLoggerFactory; // The ILoggerFactory is the LoggerContext in Logback + + // Create a JoranConfigurator instance + Class<?> joranConfiguratorClass = Class.forName("ch.qos.logback.classic.joran.JoranConfigurator"); + Object configurator = joranConfiguratorClass.newInstance(); + + // Set the context: configurator.setContext(context) + Method setContextMethod = joranConfiguratorClass.getMethod("setContext", Class.forName("ch.qos.logback.core.Context")); + setContextMethod.invoke(configurator, context); + + // Reset the context: context.reset() + Method resetMethod = context.getClass().getMethod("reset"); + resetMethod.invoke(context); + + // Find the configuration file URL (e.g., logback.xml) + java.net.URL configUrl = this.getClass().getClassLoader().getResource("logback.xml"); + if (configUrl == null) { + configUrl = this.getClass().getClassLoader().getResource("logback-test.xml"); Review Comment: The hardcoded configuration file paths 'logback.xml' and 'logback-test.xml' should be extracted as constants to improve maintainability and make them configurable if needed. ```suggestion java.net.URL configUrl = this.getClass().getClassLoader().getResource(LOGBACK_XML); if (configUrl == null) { configUrl = this.getClass().getClassLoader().getResource(LOGBACK_TEST_XML); ``` -- 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: dev-unsubscr...@ranger.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
