Skip to content
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.

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

HDFS-16495: RBF should prepend the client ip rather than append it. #4054

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public String toString() {

/** The caller context builder. */
public static final class Builder {
private static final String KEY_VALUE_SEPARATOR = ":";
public static final String KEY_VALUE_SEPARATOR = ":";
/**
* The illegal separators include '\t', '\n', '='.
* User should not set illegal separator.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ private Object invokeMethod(
+ router.getRouterId());
}

appendClientIpPortToCallerContextIfAbsent();
addClientIpToCallerContext();

Object ret = null;
if (rpcMonitor != null) {
Expand Down Expand Up @@ -588,19 +588,32 @@ private Object invokeMethod(
/**
* For tracking which is the actual client address.
* It adds trace info "clientIp:ip" and "clientPort:port"
* to caller context if they are absent.
* in the caller context, removing the old values if they were
* already present.
*/
private void appendClientIpPortToCallerContextIfAbsent() {
private void addClientIpToCallerContext() {
CallerContext ctx = CallerContext.getCurrent();
String origContext = ctx == null ? null : ctx.getContext();
byte[] origSignature = ctx == null ? null : ctx.getSignature();
CallerContext.setCurrent(
new CallerContext.Builder(origContext, contextFieldSeparator)
.appendIfAbsent(CLIENT_IP_STR, Server.getRemoteAddress())
.appendIfAbsent(CLIENT_PORT_STR,
CallerContext.Builder builder =
new CallerContext.Builder("", contextFieldSeparator)
.append(CLIENT_IP_STR, Server.getRemoteAddress())
.append(CLIENT_PORT_STR,
Integer.toString(Server.getRemotePort()))
.setSignature(origSignature)
.build());
.setSignature(origSignature);
// Append the original caller context
if (origContext != null) {
for (String part : origContext.split(contextFieldSeparator)) {
String[] keyValue =
part.split(CallerContext.Builder.KEY_VALUE_SEPARATOR, 2);
if (keyValue.length == 2) {
builder.appendIfAbsent(keyValue[0], keyValue[1]);
} else if (keyValue.length == 1) {
builder.append(keyValue[0]);
}
}
}
CallerContext.setCurrent(builder.build());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1950,9 +1950,10 @@ public void testMkdirsWithCallerContext() throws IOException {
FsPermission permission = new FsPermission("755");
routerProtocol.mkdirs(dirPath, permission, false);

// The audit log should contains "callerContext=clientContext,clientIp:"
assertTrue(auditlog.getOutput()
.contains("callerContext=clientContext,clientIp:"));
// The audit log should contains "callerContext=clientIp:...,clientContext"
final String logOutput = auditlog.getOutput();
assertTrue(logOutput.contains("callerContext=clientIp:"));
assertTrue(logOutput.contains(",clientContext"));
assertTrue(verifyFileExists(routerFS, dirPath));
}

Expand Down Expand Up @@ -1997,9 +1998,9 @@ public void testAddClientIpPortToCallerContext() throws IOException {
// Create a directory via the router.
routerProtocol.getFileInfo(dirPath);

// The audit log should contains the original clientIp and clientPort
// The audit log should not contain the original clientIp and clientPort
// set by client.
assertTrue(auditLog.getOutput().contains("clientIp:1.1.1.1"));
assertTrue(auditLog.getOutput().contains("clientPort:1234"));
assertFalse(auditLog.getOutput().contains("clientIp:1.1.1.1"));
assertFalse(auditLog.getOutput().contains("clientPort:1234"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,8 @@ public void testCallerContextWithMultiDestinations() throws IOException {
for (String line : auditLog.getOutput().split("\n")) {
if (line.contains(auditFlag)) {
// assert origin caller context exist in audit log
assertTrue(line.contains("callerContext=clientContext"));
String callerContext = line.substring(
line.indexOf("callerContext=clientContext"));
String callerContext = line.substring(line.indexOf("callerContext="));
assertTrue(callerContext.contains("clientContext"));
// assert client ip info exist in caller context
assertTrue(callerContext.contains(clientIpInfo));
// assert client ip info appears only once in caller context
Expand Down