Skip to content

Commit

Permalink
RANGER-4821: fixed bugs and code smell reported by sonarqube
Browse files Browse the repository at this point in the history 
Signed-off-by: Madhan Neethiraj <madhan@apache.org>
  • Loading branch information
@monika-kachhadiya @mneethiraj
monika-kachhadiya authored and mneethiraj committed Jun 18, 2024
1 parent d8a670c commit b3b5ece
Show file tree
Hide file tree
Showing 62 changed files with 842 additions and 1,143 deletions.
4 changes: 2 additions & 2 deletions ...t/src/main/java/org/apache/ranger/audit/destination/AmazonCloudWatchAuditDestination.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
@ThreadSafe
public class AmazonCloudWatchAuditDestination extends AuditDestination {

private static Logger LOG = LoggerFactory.getLogger(AmazonCloudWatchAuditDestination.class);
private static final Logger LOG = LoggerFactory.getLogger(AmazonCloudWatchAuditDestination.class);

public static final String PROP_LOG_GROUP_NAME = "log_group";
public static final String PROP_LOG_STREAM_PREFIX = "log_stream_prefix";
Expand Down Expand Up @@ -92,7 +92,7 @@ public void stop() {
}

@Override
synchronized public boolean log(Collection<AuditEventBase> collection) {
public synchronized boolean log(Collection<AuditEventBase> collection) {
boolean ret = false;
AWSLogs client = getClient();

Expand Down
54 changes: 30 additions & 24 deletions ...udit/src/main/java/org/apache/ranger/audit/destination/ElasticSearchAuditDestination.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;

import com.google.common.util.concurrent.ThreadFactoryBuilder;
import org.apache.commons.lang.StringUtils;
Expand Down Expand Up @@ -74,8 +75,8 @@ public class ElasticSearchAuditDestination extends AuditDestination {
public static final String CONFIG_PREFIX = "ranger.audit.elasticsearch";
public static final String DEFAULT_INDEX = "ranger_audits";

private String index = "index";
private volatile RestHighLevelClient client = null;
private String index = CONFIG_INDEX;
private final AtomicReference<RestHighLevelClient> clientRef = new AtomicReference<>(null);
private String protocol;
private String user;
private int port;
Expand Down Expand Up @@ -128,12 +129,12 @@ public boolean log(Collection<AuditEventBase> events) {
ArrayList<AuditEventBase> eventList = new ArrayList<>(events);
BulkRequest bulkRequest = new BulkRequest();
try {
for (AuditEventBase event : eventList) {
eventList.forEach(event -> {
AuthzAuditEvent authzEvent = (AuthzAuditEvent) event;
String id = authzEvent.getEventId();
Map<String, Object> doc = toDoc(authzEvent);
bulkRequest.add(new IndexRequest(index).id(id).source(doc));
}
});
} catch (Exception ex) {
addFailedCount(eventList.size());
logFailedEvent(eventList, ex);
Expand Down Expand Up @@ -173,28 +174,32 @@ public boolean log(Collection<AuditEventBase> events) {
*/
@Override
public void flush() {

// Empty flush method
}

public boolean isAsync() {
return true;
}

synchronized RestHighLevelClient getClient() {
RestHighLevelClient client = clientRef.get();
if (client == null) {
synchronized (ElasticSearchAuditDestination.class) {
client = clientRef.get();
if (client == null) {
client = newClient();
clientRef.set(client);
}
}
}
if (subject != null) {
KerberosTicket ticket = CredentialsProviderUtil.getTGT(subject);
try {
if (new Date().getTime() > ticket.getEndTime().getTime()) {
client = null;
clientRef.set(null);
CredentialsProviderUtil.ticketExpireTime80 = 0;
newClient();
client = newClient();
clientRef.set(client);
} else if (CredentialsProviderUtil.ticketWillExpire(ticket)) {
subject = CredentialsProviderUtil.login(user, password);
}
Expand All @@ -212,7 +217,7 @@ public static RestClientBuilder getRestClientBuilder(String urls, String protoco
RestClientBuilder restClientBuilder = RestClient.builder(
MiscUtil.toArray(urls, ",").stream()
.map(x -> new HttpHost(x, port, protocol))
.<HttpHost>toArray(i -> new HttpHost[i])
.toArray(HttpHost[]::new)
);
ThreadFactory clientThreadFactory = new ThreadFactoryBuilder()
.setNameFormat("ElasticSearch rest client %s")
Expand Down Expand Up @@ -258,24 +263,25 @@ private RestHighLevelClient newClient() {
}
RestClientBuilder restClientBuilder =
getRestClientBuilder(hosts, protocol, user, password, port);
RestHighLevelClient restHighLevelClient = new RestHighLevelClient(restClientBuilder);
if (LOG.isDebugEnabled()) {
LOG.debug("Initialized client");
}
boolean exits = false;
try {
exits = restHighLevelClient.indices().open(new OpenIndexRequest(this.index), RequestOptions.DEFAULT).isShardsAcknowledged();
} catch (Exception e) {
LOG.warn("Error validating index " + this.index);
}
if (exits) {
try (RestHighLevelClient restHighLevelClient = new RestHighLevelClient(restClientBuilder)) {
if (LOG.isDebugEnabled()) {
LOG.debug("Index exists");
LOG.debug("Initialized client");
}
} else {
LOG.info("Index does not exist");
boolean exists = false;
try {
exists = restHighLevelClient.indices().open(new OpenIndexRequest(this.index), RequestOptions.DEFAULT).isShardsAcknowledged();
} catch (Exception e) {
LOG.warn("Error validating index " + this.index);
}
if (exists) {
if (LOG.isDebugEnabled()) {
LOG.debug("Index exists");
}
} else {
LOG.info("Index does not exist");
}
return restHighLevelClient;
}
return restHighLevelClient;
} catch (Throwable t) {
lastLoggedAt.updateAndGet(lastLoggedAt -> {
long now = System.currentTimeMillis();
Expand Down Expand Up @@ -311,7 +317,7 @@ private String getStringProperty(Properties props, String propName, String defau
}

Map<String, Object> toDoc(AuthzAuditEvent auditEvent) {
Map<String, Object> doc = new HashMap<String, Object>();
Map<String, Object> doc = new HashMap<>();
doc.put("id", auditEvent.getEventId());
doc.put("access", auditEvent.getAccessType());
doc.put("enforcer", auditEvent.getAclEnforcer());
Expand Down
36 changes: 12 additions & 24 deletions agents-audit/src/main/java/org/apache/ranger/audit/destination/FileAuditDestination.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,31 +75,24 @@ public void init(Properties prop, String propPrefix) {
+ PROP_FILE_FILE_ROLLOVER, fileRolloverSec);

if (logFolderProp == null || logFolderProp.isEmpty()) {
logger.error("File destination folder is not configured. Please set "
+ propPrefix
+ "."
+ PROP_FILE_LOCAL_DIR
+ ". name="
+ getName());
logger.error("File destination folder is not configured. Please set {}. {}. name= {}", propPrefix, PROP_FILE_LOCAL_DIR, getName());
return;
}
logFolder = new File(logFolderProp);
if (!logFolder.isDirectory()) {
logFolder.mkdirs();
if (!logFolder.isDirectory()) {
logger.error("FileDestination folder not found and can't be created. folder="
+ logFolder.getAbsolutePath() + ", name=" + getName());
logger.error("FileDestination folder not found and can't be created. folder={}, name={}", logFolder.getAbsolutePath(), getName());
return;
}
}
logger.info("logFolder=" + logFolder + ", name=" + getName());
logger.info("logFolder={}, name={}", logFolder, getName());

if (logFileNameFormat == null || logFileNameFormat.isEmpty()) {
logFileNameFormat = "%app-type%_ranger_audit.log";
}

logger.info("logFileNameFormat=" + logFileNameFormat + ", destName="
+ getName());
logger.info("logFileNameFormat={}, destName={}", logFileNameFormat, getName());

initDone = true;
}
Expand All @@ -110,7 +103,7 @@ synchronized public boolean logJSON(Collection<String> events) {
addTotalCount(events.size());

if (isStopped) {
logError("log() called after stop was requested. name=" + getName());
logError("logJSON() called after stop was requested. name={}", getName());
addDeferredCount(events.size());
return false;
}
Expand Down Expand Up @@ -141,7 +134,7 @@ public boolean log(Collection<AuditEventBase> events) {
if (isStopped) {
addTotalCount(events.size());
addDeferredCount(events.size());
logError("log() called after stop was requested. name=" + getName());
logError("log() called after stop was requested. name={}", getName());
return false;
}
List<String> jsonList = new ArrayList<String>();
Expand All @@ -152,7 +145,7 @@ public boolean log(Collection<AuditEventBase> events) {
addTotalCount(1);
addFailedCount(1);
logFailedEvent(event);
logger.error("Error converting to JSON. event=" + event);
logger.error("Error converting to JSON. event={}", event);
}
}
return logJSON(jsonList);
Expand All @@ -178,8 +171,7 @@ synchronized public void stop() {
logWriter.flush();
logWriter.close();
} catch (Throwable t) {
logger.error("Error on closing log writter. Exception will be ignored. name="
+ getName() + ", fileName=" + currentFileName);
logger.error("Error on closing log writer. Exception will be ignored. name= {}, fileName= {}", getName(), currentFileName);
}
logWriter = null;
}
Expand Down Expand Up @@ -211,16 +203,14 @@ synchronized private PrintWriter getLogFileStream() throws Exception {
if (!newLogFile.exists()) {
// Move the file
if (!outLogFile.renameTo(newLogFile)) {
logger.error("Error renameing file. " + outLogFile
+ " to " + newLogFile);
logger.error("Error renameing file. {} to {} " , outLogFile, newLogFile);
}
break;
}
}
}
if (!outLogFile.exists()) {
logger.info("Creating new file. destName=" + getName()
+ ", fileName=" + fileName);
logger.info("Creating new file. destName={} , fileName={} ", getName(), fileName);
// Open the file
logWriter = new PrintWriter(new BufferedWriter(new FileWriter(
outLogFile)));
Expand All @@ -239,14 +229,12 @@ private void closeFileIfNeeded() {
return;
}
if (System.currentTimeMillis() - fileCreateTime.getTime() > fileRolloverSec * 1000) {
logger.info("Closing file. Rolling over. name=" + getName()
+ ", fileName=" + currentFileName);
logger.info("Closing file. Rolling over. name={} , fileName={}", getName(), currentFileName);
try {
logWriter.flush();
logWriter.close();
} catch (Throwable t) {
logger.error("Error on closing log writter. Exception will be ignored. name="
+ getName() + ", fileName=" + currentFileName);
logger.error("Error on closing log writter. Exception will be ignored. name={} , fileName={}", getName(), currentFileName);
}
logWriter = null;
currentFileName = null;
Expand Down
16 changes: 8 additions & 8 deletions agents-audit/src/main/java/org/apache/ranger/audit/destination/HDFSAuditDestination.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ synchronized public boolean logJSON(final Collection<String> events) {
}
if (isStopped) {
addDeferredCount(events.size());
logError("log() called after stop was requested. name=" + getName());
logError("log() called after stop was requested. name={}", getName());
return false;
}
try {
Expand All @@ -87,7 +87,7 @@ synchronized public boolean logJSON(final Collection<String> events) {
return false;
} finally {
if (logger.isDebugEnabled()) {
logger.debug("Flushing HDFS audit. Event Size:" + events.size());
logger.debug("Flushing HDFS audit. Event Size:{}", events.size());
}
if (auditWriter != null) {
flush();
Expand All @@ -104,7 +104,7 @@ synchronized public boolean logFile(final File file) {
return false;
}
if (isStopped) {
logError("log() called after stop was requested. name=" + getName());
logError("log() called after stop was requested. name={}", getName());
return false;
}

Expand All @@ -117,7 +117,7 @@ synchronized public boolean logFile(final File file) {
logError("Error writing to log file.", t);
return false;
} finally {
logger.info("Flushing HDFS audit. File:" + file.getAbsolutePath() + file.getName());
logger.info("Flushing HDFS audit. File:{}{}", file.getAbsolutePath(), file.getName());
if (auditWriter != null) {
flush();
}
Expand All @@ -128,7 +128,7 @@ synchronized public boolean logFile(final File file) {
@Override
public void flush() {
if (logger.isDebugEnabled()) {
logger.debug("==> HDFSAuditDestination.flush() called. name=" + getName());
logger.debug("==> HDFSAuditDestination.flush() called. name={}", getName());
}
MiscUtil.executePrivilegedAction(new PrivilegedAction<Void>() {
@Override
Expand All @@ -138,7 +138,7 @@ public Void run() {
}
});
if (logger.isDebugEnabled()) {
logger.debug("<== HDFSAuditDestination.flush() called. name=" + getName());
logger.debug("<== HDFSAuditDestination.flush() called. name={}", getName());
}
}

Expand All @@ -154,15 +154,15 @@ public boolean log(Collection<AuditEventBase> events) {
logStatusIfRequired();
addTotalCount(events.size());
addDeferredCount(events.size());
logError("log() called after stop was requested. name=" + getName());
logError("log() called after stop was requested. name={}", getName());
return false;
}
List<String> jsonList = new ArrayList<String>();
for (AuditEventBase event : events) {
try {
jsonList.add(MiscUtil.stringify(event));
} catch (Throwable t) {
logger.error("Error converting to JSON. event=" + event);
logger.error("Error converting to JSON. event={}", event);
addTotalCount(1);
addFailedCount(1);
logFailedEvent(event);
Expand Down
Loading

0 comments on commit b3b5ece

Please sign in to comment.