You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2020/07/25 18:24:05 UTC
[impala] branch master updated (9c542ef -> ebb5599)
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from 9c542ef IMPALA-9633: Implement ds_hll_union()
new d83074c IMPALA-9977: Remove duplicate Ranger audit log entries for ALTER events
new ee18dc3 IMPALA-9987: Improve logging around HTTP connections
new 6c3ff19 IMPALA-9988: Integrate ldap filters and proxy users
new ebb5599 IMPALA-10007: Impala development environment does not support Ubuntu 20.4
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/rpc/TAcceptQueueServer.cpp | 4 +-
be/src/rpc/authentication.cc | 5 +-
be/src/transport/THttpTransport.cpp | 4 ++
be/src/util/ldap-util.cc | 53 ++++++++++++----------
be/src/util/ldap-util.h | 14 ++++--
be/src/util/webserver.cc | 18 +++++++-
bin/bootstrap_system.sh | 16 ++++++-
bin/bootstrap_toolchain.py | 3 +-
.../authorization/PrivilegeRequestBuilder.java | 5 +-
.../authorization/ranger/RangerAuditLogTest.java | 14 ++++++
.../impala/customcluster/LdapImpalaShellTest.java | 53 ++++++++++++++++++++++
11 files changed, 154 insertions(+), 35 deletions(-)
[impala] 04/04: IMPALA-10007: Impala development environment does
not support Ubuntu 20.4
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit ebb5599aaaaee8df217e51378079b3647a31f128
Author: Qifan Chen <qc...@cloudera.com>
AuthorDate: Fri Jul 24 16:51:25 2020 -0400
IMPALA-10007: Impala development environment does not support
Ubuntu 20.4
This work addresses the current limitation in Impala development
environment in that Ubuntu 20.4 is not supportd. The fix modifies
bootstrap_system.sh and bootstrap_toolchain.py to specifically
allow the bootstrapping of the development environment on a maching
running Ubuntu 20.4. Limited use shows that the environment is useful
and stable, similar to the one running on Ubuntu 18.4.
Testing on a box running Ubuntu 20.4:
1. Successfully bootstrapped the entire Impala development environment
2. Interacted with the enviroment through the following tools:
gdb
jdb
clang-format
impalad GUI
vim
3. Ran all tests
Limitations found with Ubuntu 20.4 environment.
1. gdb in Impala toolchain is not compatible with Impala C++ test
code ${IMPALA_HOME}/be/build/latest/service\
/unifiedbetests (invoked by ${IMPALA_HOME}/be/build/latest/\
scheduling/admission-controller-test) and reports the following
error, after attaching to the test process.
BFD (GNU Binutils) 2.25.51 internal error, aborting at elf64-x86-64.c
ine 5583 in elf_x86_64_get_plt_sym_val
Change-Id: I4f592f60881fd8f34e2bf393a76f5a921505010a
Reviewed-on: http://gerrit.cloudera.org:8080/16238
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
bin/bootstrap_system.sh | 16 ++++++++++++++--
bin/bootstrap_toolchain.py | 3 ++-
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/bin/bootstrap_system.sh b/bin/bootstrap_system.sh
index b3c1401..305c60b 100755
--- a/bin/bootstrap_system.sh
+++ b/bin/bootstrap_system.sh
@@ -76,6 +76,7 @@ REDHAT8=
UBUNTU=
UBUNTU16=
UBUNTU18=
+UBUNTU20=
IN_DOCKER=
if [[ -f /etc/redhat-release ]]; then
REDHAT=true
@@ -109,6 +110,10 @@ else
then
UBUNTU18=true
echo "Identified Ubuntu 18.04 system."
+ elif [[ $DISTRIB_RELEASE = 20.04 ]]
+ then
+ UBUNTU20=true
+ echo "Identified Ubuntu 20.04 system."
else
echo "This script only supports 16.04 or 18.04 of Ubuntu" >&2
exit 1
@@ -144,6 +149,12 @@ function ubuntu18 {
fi
}
+function ubuntu20 {
+ if [[ "$UBUNTU20" == true ]]; then
+ "$@"
+ fi
+}
+
# Helper function to execute following command only on RedHat
function redhat {
if [[ "$REDHAT" == true ]]; then
@@ -227,11 +238,12 @@ if [[ "$UBUNTU" == true ]]; then
fi
fi
-# Ubuntu 18.04 installs OpenJDK 11 and configures it as the default Java version.
+# Ubuntu 18.04 or 20.04 install OpenJDK 11 and configure it as the default Java version.
# Impala is currently tested with OpenJDK 8, so configure that version as the default.
ARCH_NAME=$(uname -p)
if [[ $ARCH_NAME == 'aarch64' ]]; then
- ubuntu18 sudo update-java-alternatives -s java-1.8.0-openjdk-arm64
+ ubuntu20 sudo update-java-alternatives -s java-1.8.0-openjdk-arm64
+ ubuntu18 sudo update-java-alternatives -s java-1.8.0-openjdk-arm64
else
ubuntu18 sudo update-java-alternatives -s java-1.8.0-openjdk-amd64
fi
diff --git a/bin/bootstrap_toolchain.py b/bin/bootstrap_toolchain.py
index 2e01f9e..647fc00 100755
--- a/bin/bootstrap_toolchain.py
+++ b/bin/bootstrap_toolchain.py
@@ -91,7 +91,8 @@ OS_MAPPING = [
OsMapping("ubuntu15.04", "ec2-package-ubuntu-14-04", None),
OsMapping("ubuntu15.10", "ec2-package-ubuntu-14-04", None),
OsMapping('ubuntu16.04', "ec2-package-ubuntu-16-04", "ubuntu1604"),
- OsMapping('ubuntu18.04', "ec2-package-ubuntu-18-04", "ubuntu1804")
+ OsMapping('ubuntu18.04', "ec2-package-ubuntu-18-04", "ubuntu1804"),
+ OsMapping('ubuntu20.04', "ec2-package-ubuntu-18-04", "ubuntu1804")
]
[impala] 01/04: IMPALA-9977: Remove duplicate Ranger audit log
entries for ALTER events
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit d83074c5992d51d136a509639a038bde5c3393bf
Author: Fang-Yu Rao <fa...@cloudera.com>
AuthorDate: Tue Jul 21 17:11:23 2020 -0700
IMPALA-9977: Remove duplicate Ranger audit log entries for ALTER events
This JIRA could be considered as a follow-up to IMPALA-9625, where we
converted the name of a TAccessEvent to lowercase to avoid duplicate
audits in the Set used to maintain the collected TAccessEvent's so that
there will not be duplicate TAccessEvent's in the file specified by the
flag of "-audit_event_log_dir" when Impala is started.
However, the patch for IMPALA-9625 only considered the audits that are
exported to the specific file mentioned above but not the
PrivilegeRequest's that will be processed by Ranger which in turn would
produce the corresponding audit log entries. Therefore, the
fully-qualified table name that is provided when
Analyzer#registerPrivReq() is called in Analyzer#getTable() is not
necessarily in lowercase, resulting in duplicate AuthzAuditEvent's
stored in the corresponding RangerBufferAuditHandler because the
full table names returned from registerAuthAndAuditEvent() and
getTable() differ. Refer to IMPALA-9625 for more details.
To resolve the inconsistencies, this patch converts the arguments of
database and table names to lowercase when
PrivilegeRequestBuilder#onTable() is building the corresponding
PrivilegeRequest, which will later be added to the Set of
PrivilegeRequest's for Ranger to process.
Testing:
- Added an FE test in RangerAuditLogTest.java to make sure no duplicate
Ranger audit log entries are produced.
- Verified that the patch passes the exhaustive tests in the DEBUG
build.
Change-Id: Iab9b664ad5ee9722182007ee67d14bf47bd03d8a
Reviewed-on: http://gerrit.cloudera.org:8080/16231
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../impala/authorization/PrivilegeRequestBuilder.java | 5 ++++-
.../impala/authorization/ranger/RangerAuditLogTest.java | 14 ++++++++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java b/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
index b115fb3..72127c8 100644
--- a/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
+++ b/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
@@ -79,7 +79,10 @@ public class PrivilegeRequestBuilder {
public PrivilegeRequestBuilder onTable(
String dbName, String tableName, String ownerUser) {
Preconditions.checkState(authorizable_ == null);
- authorizable_ = authzFactory_.newTable(dbName, tableName, ownerUser);
+ // Convert 'dbName' and 'tableName' to lowercase to avoid duplicate audit log entries
+ // for the PrivilegeRequest of ALTER.
+ authorizable_ = authzFactory_.newTable(dbName.toLowerCase(), tableName.toLowerCase(),
+ ownerUser);
return this;
}
diff --git a/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java b/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
index 7de2acb..2b4775c 100644
--- a/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
+++ b/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
@@ -146,6 +146,20 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
assertEquals("show partitions functional.alltypes", events.get(0).getRequestData());
}, "show partitions functional.alltypes",
onTable("functional", "alltypes", TPrivilegeLevel.SELECT));
+
+ // COMPUTE STATS results in two registrations of the PrivilegeRequest for ALTER.
+ // Check we do not have duplicate ALTER events when the fully-qualified table name is
+ // not in lowercase.
+ authzOk(events -> {
+ assertEquals(2, events.size());
+ assertEventEquals("@table", "alter", "functional/alltypes", 1,
+ events.get(0));
+ assertEventEquals("@table", "select", "functional/alltypes", 1,
+ events.get(1));
+ assertEquals("compute stats FUNCTIONAL.ALLTYPES", events.get(0).getRequestData());
+ }, "compute stats FUNCTIONAL.ALLTYPES",
+ onTable("functional", "alltypes", TPrivilegeLevel.ALTER,
+ TPrivilegeLevel.SELECT));
}
@Test
[impala] 03/04: IMPALA-9988: Integrate ldap filters and proxy users
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 6c3ff1927e0aeacd974d2869ef88dc0233de22d6
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
AuthorDate: Tue Jul 21 14:57:23 2020 -0700
IMPALA-9988: Integrate ldap filters and proxy users
IMPALA-2563 recently added support for specifying ldap group and user
filters.
Previously, if an authorized proxy user connected, the filters were
applied to the proxy user. This patch fixes it so that the filters
will be applied to the delegate user instead.
Testing:
- Added a test in LdapImpalaShellTest
Change-Id: I92ff3ca543516a8c4fa0ec7533f7b9d8da6e2434
Reviewed-on: http://gerrit.cloudera.org:8080/16234
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/rpc/authentication.cc | 5 +-
be/src/util/ldap-util.cc | 53 ++++++++++++----------
be/src/util/ldap-util.h | 14 ++++--
.../impala/customcluster/LdapImpalaShellTest.java | 53 ++++++++++++++++++++++
4 files changed, 97 insertions(+), 28 deletions(-)
diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc
index fb95e73..801d99e 100644
--- a/be/src/rpc/authentication.cc
+++ b/be/src/rpc/authentication.cc
@@ -445,8 +445,8 @@ bool BasicAuth(ThriftServer::ConnectionContext* connection_context,
}
string username = decoded.substr(0, colon);
string password = decoded.substr(colon + 1);
- bool ret = AuthManager::GetInstance()->GetLdap()->LdapCheckPass(
- username.c_str(), password.c_str(), password.length());
+ bool ret = AuthManager::GetInstance()->GetLdap()->LdapCheckPass(username.c_str(),
+ password.c_str(), password.length(), connection_context->do_as_user);
if (ret) {
// Authenication was successful, so set the username on the connection.
connection_context->username = username;
@@ -1028,6 +1028,7 @@ void NoAuthProvider::SetupConnectionContext(
ThriftServer::TransportType underlying_transport_type, TTransport* input_transport,
TTransport* output_transport) {
connection_ptr->username = "";
+ connection_ptr->do_as_user = "";
TSocket* socket = nullptr;
switch (underlying_transport_type) {
case ThriftServer::BINARY: {
diff --git a/be/src/util/ldap-util.cc b/be/src/util/ldap-util.cc
index d573207..b8adeb6 100644
--- a/be/src/util/ldap-util.cc
+++ b/be/src/util/ldap-util.cc
@@ -146,7 +146,8 @@ Status ImpalaLdap::Init(const std::string& user_filter, const std::string& group
return Status::OK();
}
-bool ImpalaLdap::LdapCheckPass(const char* user, const char* pass, unsigned passlen) {
+bool ImpalaLdap::LdapCheckPass(
+ const char* user, const char* pass, unsigned passlen, string do_as_user) {
if (passlen == 0 && !FLAGS_ldap_allow_anonymous_binds) {
// Disable anonymous binds.
return false;
@@ -178,32 +179,20 @@ bool ImpalaLdap::LdapCheckPass(const char* user, const char* pass, unsigned pass
VLOG(2) << "Started TLS connection with LDAP server: " << FLAGS_ldap_uri;
}
- // Map the user string into an acceptable LDAP "DN" (distinguished name)
- string user_str = user;
- if (!FLAGS_ldap_domain.empty()) {
- // Append @domain if there isn't already an @ in the user string.
- if (user_str.find("@") == string::npos) {
- user_str = Substitute("$0@$1", user_str, FLAGS_ldap_domain);
- }
- } else if (!FLAGS_ldap_baseDN.empty()) {
- user_str = Substitute("uid=$0,$1", user_str, FLAGS_ldap_baseDN);
- } else if (!FLAGS_ldap_bind_pattern.empty()) {
- user_str = FLAGS_ldap_bind_pattern;
- replace_all(user_str, "#UID", user);
- }
+ string user_dn = ConstructUserDN(user);
// Map the password into a credentials structure
struct berval cred;
cred.bv_val = const_cast<char*>(pass);
cred.bv_len = passlen;
- VLOG_QUERY << "Trying simple LDAP bind for: " << user_str;
+ VLOG_QUERY << "Trying simple LDAP bind for: " << user_dn;
rc = ldap_sasl_bind_s(
- ld, user_str.c_str(), LDAP_SASL_SIMPLE, &cred, nullptr, nullptr, nullptr);
+ ld, user_dn.c_str(), LDAP_SASL_SIMPLE, &cred, nullptr, nullptr, nullptr);
// Free ld
if (rc != LDAP_SUCCESS) {
- LOG(WARNING) << "LDAP authentication failure for " << user_str << " : "
+ LOG(WARNING) << "LDAP authentication failure for " << user_dn << " : "
<< ldap_err2string(rc);
ldap_unbind_ext(ld, nullptr, nullptr);
return false;
@@ -211,16 +200,18 @@ bool ImpalaLdap::LdapCheckPass(const char* user, const char* pass, unsigned pass
VLOG_QUERY << "LDAP bind successful";
- if (!user_filter_.empty() && user_filter_.count(user) != 1) {
- LOG(WARNING) << "LDAP authentication failure for " << user_str << ". Bind was "
+ string filter_user = do_as_user == "" ? user : do_as_user;
+ if (!user_filter_.empty() && user_filter_.count(filter_user) != 1) {
+ LOG(WARNING) << "LDAP authentication failure for " << user_dn << ". Bind was "
<< "successful but user is not in the authorized user list.";
ldap_unbind_ext(ld, nullptr, nullptr);
return false;
}
+ string filter_user_dn = do_as_user == nullptr ? user_dn : ConstructUserDN(do_as_user);
if (!group_filter_.empty()) {
- if (!CheckGroupMembership(ld, user_str)) {
- LOG(WARNING) << "LDAP authentication failure for " << user_str << ". Bind was "
+ if (!CheckGroupMembership(ld, filter_user_dn)) {
+ LOG(WARNING) << "LDAP authentication failure for " << user_dn << ". Bind was "
<< "successful but user is not in any of the required groups.";
ldap_unbind_ext(ld, nullptr, nullptr);
return false;
@@ -231,12 +222,12 @@ bool ImpalaLdap::LdapCheckPass(const char* user, const char* pass, unsigned pass
return true;
}
-bool ImpalaLdap::CheckGroupMembership(LDAP* ld, const string& user_str) {
+bool ImpalaLdap::CheckGroupMembership(LDAP* ld, const string& user_dn) {
// Construct a filter that will search for LDAP entries that represent groups
// (determined by having the group class key) and that contain the user trying to
// authenticate (determined by having a membership entry matching the user).
string filter = Substitute("(&(objectClass=$0)($1=$2))", FLAGS_ldap_group_class_key,
- FLAGS_ldap_group_membership_key, user_str);
+ FLAGS_ldap_group_membership_key, user_dn);
VLOG(2) << "Searching for groups with filter: " << filter;
for (const string& group_dn : group_filter_dns_) {
@@ -308,4 +299,20 @@ string ImpalaLdap::GetShortName(const string& rdn) {
return value[1];
}
+string ImpalaLdap::ConstructUserDN(const std::string& user) {
+ string user_dn = user;
+ if (!FLAGS_ldap_domain.empty()) {
+ // Append @domain if there isn't already an @ in the user string.
+ if (user_dn.find("@") == string::npos) {
+ user_dn = Substitute("$0@$1", user_dn, FLAGS_ldap_domain);
+ }
+ } else if (!FLAGS_ldap_baseDN.empty()) {
+ user_dn = Substitute("uid=$0,$1", user_dn, FLAGS_ldap_baseDN);
+ } else if (!FLAGS_ldap_bind_pattern.empty()) {
+ user_dn = FLAGS_ldap_bind_pattern;
+ replace_all(user_dn, "#UID", user);
+ }
+ return user_dn;
+}
+
} // namespace impala
diff --git a/be/src/util/ldap-util.h b/be/src/util/ldap-util.h
index 8a34905..f624b9d 100644
--- a/be/src/util/ldap-util.h
+++ b/be/src/util/ldap-util.h
@@ -41,16 +41,20 @@ class ImpalaLdap {
const std::string& user_filter, const std::string& group_filter) WARN_UNUSED_RESULT;
/// Attempts to authenticate to LDAP using the given username and password, applying the
- /// user or group filters as approrpriate. 'passlen' is the length of the password.
+ /// user or group filters as appropriate. 'passlen' is the length of the password.
/// Returns true if authentication is successful.
///
+ /// 'do_as_user' is an optional delegate user that 'username' is trying act as a proxy
+ /// for. If provided, the group and user filters will be applied to 'do_as_user' and not
+ /// 'username'.
+ ///
/// Note that this method uses ldap_sasl_bind_s(), which does *not* provide any security
/// to the connection between Impala and the LDAP server. You must either set
/// --ldap_tls or have a URI which has "ldaps://" as the scheme in order to get a secure
/// connection. Use --ldap_ca_certificate to specify the location of the certificate
/// used to confirm the authenticity of the LDAP server certificate.
- bool LdapCheckPass(
- const char* username, const char* password, unsigned passlen) WARN_UNUSED_RESULT;
+ bool LdapCheckPass(const char* username, const char* password, unsigned passlen,
+ std::string do_as_user = "") WARN_UNUSED_RESULT;
private:
/// If non-empty, only users in this set can successfully authenticate.
@@ -68,6 +72,10 @@ class ImpalaLdap {
/// Returns the value part of the first attribute in the provided relative DN.
static std::string GetShortName(const std::string& rdn);
+
+ /// Maps the user string into an acceptable LDAP "DN" (distinguished name) based on the
+ /// values of the LDAP config flags.
+ static std::string ConstructUserDN(const std::string& user);
};
} // namespace impala
diff --git a/fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java b/fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
index f2c4561..a561a5a 100644
--- a/fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
+++ b/fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
@@ -215,4 +215,57 @@ public class LdapImpalaShellTest {
buildCommand(query, "hs2-http", TEST_USER_1, TEST_PASSWORD_1, "/cliservice");
RunShellCommand.Run(command, /* shouldSucceed */ true, TEST_USER_1, "");
}
+
+ /**
+ * Tests the interaction between LDAP user and group filter configs and proxy user
+ * configs.
+ */
+ @Test
+ public void testLdapFiltersWithProxy() throws Exception {
+ String groupDN = "cn=%s,ou=Groups,dc=myorg,dc=com";
+ // These correspond to the values in fe/src/test/resources/users.ldif
+ // Sets up a cluster where TEST_USER_4 can act as a proxy for any other user but
+ // doesn't pass any filters themselves, TEST_USER_1 and TEST_USER_2 can pass the group
+ // filter, and TEST_USER_1 and TEST_USER_3 pass the user filter.
+ setUp(String.format("--ldap_group_filter=%s,another-group "
+ + "--ldap_user_filter=%s,%s,another-user "
+ + "--ldap_group_dn_pattern=%s "
+ + "--ldap_group_membership_key=uniqueMember "
+ + "--ldap_group_class_key=groupOfUniqueNames "
+ + "--authorized_proxy_user_config=%s=*",
+ TEST_USER_GROUP, TEST_USER_1, TEST_USER_3, groupDN, TEST_USER_4));
+
+ String query = "select logged_in_user()";
+
+ // Run as the proxy user with a delegate that passes both filters, should succeed
+ // and return the delegate user's name.
+ String[] command = buildCommand(
+ query, "hs2-http", TEST_USER_4, TEST_PASSWORD_4, "/?doAs=" + TEST_USER_1);
+ RunShellCommand.Run(command, /* shouldSucceed */ true, TEST_USER_1, "");
+
+ // Run as the proxy user with a delegate that only passes the user filter, should
+ // fail.
+ command = buildCommand(
+ query, "hs2-http", TEST_USER_4, TEST_PASSWORD_4, "/?doAs=" + TEST_USER_3);
+ RunShellCommand.Run(
+ command, /* shouldSucceed */ false, "", "Not connected to Impala");
+
+ // Run as the proxy user with a delegate that only passes the group filter, should
+ // fail.
+ command = buildCommand(
+ query, "hs2-http", TEST_USER_4, TEST_PASSWORD_4, "/?doAs=" + TEST_USER_2);
+ RunShellCommand.Run(
+ command, /* shouldSucceed */ false, "", "Not connected to Impala");
+
+ // Run as the proxy with a delegate that doesn't pass either filter, should fail.
+ command = buildCommand(
+ query, "hs2-http", TEST_USER_4, TEST_PASSWORD_4, "/?doAs=" + TEST_USER_4);
+ RunShellCommand.Run(
+ command, /* shouldSucceed */ false, "", "Not connected to Impala");
+
+ // Run as the proxy without a delegate user, should fail since the proxy user won't
+ // pass the filters.
+ command = buildCommand(query, "hs2-http", TEST_USER_4, TEST_PASSWORD_4, "/");
+ RunShellCommand.Run(command, /* shouldSucceed */ false, "", "");
+ }
}
[impala] 02/04: IMPALA-9987: Improve logging around HTTP connections
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit ee18dc383ed7f62630c9e770d14f5851a8747b81
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
AuthorDate: Tue Jul 21 16:15:40 2020 -0700
IMPALA-9987: Improve logging around HTTP connections
- Previously, every new connection resulted in three log messages at
VLOG(1) in TAcceptQueueServer: indicating a new connection being
made, the start of connection setup, and the end of connection
setup. This patch moves the second two messages to VLOG(2).
- The webserver no longer logs an error if LDAP auth in enabled and a
client tries to connect without providing an auth header, as this
case is generally not actually an error and logging it as such can
be confusing.
- VLOG(4) is added to print HTTP headers received by both the Thrift
HTTP server and the webserver for debugging.
Testing:
- Manually enabled different logging levels and checked for expected
logs.
Change-Id: I38a32b8746084ea44b098a6ccce4ce01947ae88f
Reviewed-on: http://gerrit.cloudera.org:8080/16230
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/rpc/TAcceptQueueServer.cpp | 4 ++--
be/src/transport/THttpTransport.cpp | 4 ++++
be/src/util/webserver.cc | 18 +++++++++++++++++-
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/be/src/rpc/TAcceptQueueServer.cpp b/be/src/rpc/TAcceptQueueServer.cpp
index ae4de73..7e21d45 100644
--- a/be/src/rpc/TAcceptQueueServer.cpp
+++ b/be/src/rpc/TAcceptQueueServer.cpp
@@ -224,7 +224,7 @@ void TAcceptQueueServer::SetupConnection(shared_ptr<TAcceptQueueEntry> entry) {
shared_ptr<TTransport> outputTransport;
shared_ptr<TTransport> client = entry->client_;
const string& socket_info = reinterpret_cast<TSocket*>(client.get())->getSocketInfo();
- VLOG(1) << Substitute("TAcceptQueueServer: $0 started connection setup for client $1",
+ VLOG(2) << Substitute("TAcceptQueueServer: $0 started connection setup for client $1",
name_, socket_info);
try {
MonotonicStopWatch timer;
@@ -242,7 +242,7 @@ void TAcceptQueueServer::SetupConnection(shared_ptr<TAcceptQueueEntry> entry) {
if (metrics_enabled_) {
cnxns_setup_time_us_metric_->Update(timer.ElapsedTime() / NANOS_PER_MICRO);
}
- VLOG(1) << Substitute("TAcceptQueueServer: $0 finished connection setup for "
+ VLOG(2) << Substitute("TAcceptQueueServer: $0 finished connection setup for "
"client $1", name_, socket_info);
TAcceptQueueServer::Task* task = new TAcceptQueueServer::Task(
diff --git a/be/src/transport/THttpTransport.cpp b/be/src/transport/THttpTransport.cpp
index e8925d2..652d06c 100644
--- a/be/src/transport/THttpTransport.cpp
+++ b/be/src/transport/THttpTransport.cpp
@@ -21,6 +21,8 @@
#include "transport/THttpTransport.h"
+#include "common/logging.h"
+
namespace apache {
namespace thrift {
namespace transport {
@@ -242,9 +244,11 @@ void THttpTransport::readHeaders() {
}
} else {
if (statusLine) {
+ VLOG(4) << line;
statusLine = false;
finished = parseStatusLine(line);
} else {
+ VLOG(4) << " " << line;
parseHeader(line);
}
}
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index fa6a317..a9ac1eb 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -556,6 +556,15 @@ sq_callback_result_t Webserver::BeginRequestCallbackStatic(
sq_callback_result_t Webserver::BeginRequestCallback(struct sq_connection* connection,
struct sq_request_info* request_info) {
+ if (VLOG_IS_ON(4)) {
+ VLOG(4) << request_info->request_method << " " << request_info->uri << " "
+ << request_info->http_version;
+ for (int i = 0; i < request_info->num_headers; ++i) {
+ VLOG(4) << " " << request_info->http_headers[i].name << ": "
+ << request_info->http_headers[i].value;
+ }
+ }
+
if (strncmp("OPTIONS", request_info->request_method, 7) == 0) {
// Let Squeasel deal with the request. OPTIONS requests should not require
// authentication, so do this before doing SPNEGO.
@@ -603,7 +612,14 @@ sq_callback_result_t Webserver::BeginRequestCallback(struct sq_connection* conne
AddCookie(request_info, &response_headers);
} else {
total_basic_auth_failure_->Increment(1);
- LOG(ERROR) << "Failed to authenticate: " << basic_status.GetDetail();
+ if (!sq_get_header(connection, "Authorization")) {
+ // This case is expected, as some clients will always initially try to connect
+ // without an 'Authorization' and only provide one after getting the
+ // 'WWW-Authenticate' back, so we don't log it as an error.
+ VLOG(2) << "Not authenticated: no Authorization header provided.";
+ } else {
+ LOG(ERROR) << "Failed to authenticate: " << basic_status.GetDetail();
+ }
response_headers.push_back("WWW-Authenticate: Basic");
SendResponse(connection, "401 Authentication Required", "text/plain",
"Must authenticate with Basic authentication.", response_headers);