You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by la...@apache.org on 2023/06/04 14:22:51 UTC
[kudu] branch branch-1.17.x updated (9613d12fa -> c2fe33a03)
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a change to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
from 9613d12fa [java] alternative distro for Gradle propdeps plugin
new 951190557 [tools] add bound_http_address into DaemonInfoPB
new a8ddd7e46 [thirdparty] handle OpenSSL errors properly in curl and squeasel
new 563274ae2 jwt: add control points for test binaries
new 368b070fd [Python] Refactor tests to use assertRaisesRegex
new 93cdca8d3 [util] remove unused JWT-related code
new c2fe33a03 jwt: introduce MiniOidc
The 6 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:
.../org/apache/kudu/backup/TestKuduBackup.scala | 6 +-
.../apache/kudu/test/cluster/MiniKuduCluster.java | 45 ++++-
.../org/apache/kudu/test/TestMiniKuduCluster.java | 17 ++
python/kudu/compat.py | 12 +-
python/kudu/tests/test_client.py | 58 +++++--
python/kudu/tests/test_scanner.py | 1 -
python/kudu/tests/test_scantoken.py | 1 -
python/kudu/tests/test_schema.py | 83 ++++++---
python/kudu/tests/util.py | 4 +-
src/kudu/rpc/messenger.h | 1 -
src/kudu/tools/tool.proto | 13 +-
src/kudu/tools/tool_action_test.cc | 34 +++-
src/kudu/util/CMakeLists.txt | 12 ++
src/kudu/util/jwt-util-test.cc | 192 ++++++---------------
src/kudu/util/jwt-util.cc | 7 +-
src/kudu/util/jwt-util.h | 6 +-
src/kudu/util/jwt.h | 15 --
src/kudu/util/mini_oidc.cc | 163 +++++++++++++++++
src/kudu/util/mini_oidc.h | 93 ++++++++++
thirdparty/download-thirdparty.sh | 8 +-
.../patches/curl-handle-openssl-errors.patch | 124 +++++++++++++
.../patches/squeasel-handle-openssl-errors.patch | 191 ++++++++++++++++++++
22 files changed, 854 insertions(+), 232 deletions(-)
create mode 100644 src/kudu/util/mini_oidc.cc
create mode 100644 src/kudu/util/mini_oidc.h
create mode 100644 thirdparty/patches/curl-handle-openssl-errors.patch
create mode 100644 thirdparty/patches/squeasel-handle-openssl-errors.patch
[kudu] 05/06: [util] remove unused JWT-related code
Posted by la...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 93cdca8d325c62007d86066de7c7fae27edf7519
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Mon Apr 17 12:34:43 2023 -0700
[util] remove unused JWT-related code
This patch doesn't contain any functional changes.
Change-Id: I18bfc17a2caf52a632a62f5772b676d6a9362fde
Reviewed-on: http://gerrit.cloudera.org:8080/19759
Reviewed-by: Attila Bukor <ab...@apache.org>
Tested-by: Kudu Jenkins
(cherry picked from commit 9d5539daf0bdd7f8b42a5c4bab0aaeb198f37f2b)
Reviewed-on: http://gerrit.cloudera.org:8080/19974
Reviewed-by: Yingchun Lai <la...@apache.org>
---
src/kudu/rpc/messenger.h | 1 -
src/kudu/util/jwt.h | 15 ---------------
2 files changed, 16 deletions(-)
diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h
index 127e7880d..c352c1e13 100644
--- a/src/kudu/rpc/messenger.h
+++ b/src/kudu/rpc/messenger.h
@@ -384,7 +384,6 @@ class Messenger {
return token_verifier_;
}
- const JwtVerifier* jwt_verifier() const { return jwt_verifier_.get(); }
JwtVerifier* mutable_jwt_verifier() { return jwt_verifier_.get(); }
std::optional<security::SignedTokenPB> authn_token() const {
diff --git a/src/kudu/util/jwt.h b/src/kudu/util/jwt.h
index 990c3b900..b4f321e86 100644
--- a/src/kudu/util/jwt.h
+++ b/src/kudu/util/jwt.h
@@ -34,19 +34,4 @@ class JwtVerifier {
virtual Status VerifyToken(const std::string& bytes_raw, std::string* subject) const = 0;
};
-// Minimal implementation of a JWT verifier to be used when a more full-fledged
-// implementation is not available.
-class SimpleJwtVerifier : public JwtVerifier {
- public:
- SimpleJwtVerifier() = default;
- ~SimpleJwtVerifier() override = default;
- Status Init() override { return Status::OK(); }
- Status VerifyToken(const std::string& /*bytes_raw*/,
- std::string* /*subject*/) const override {
- return Status::NotAuthorized("JWT verification not configured");
- }
- private:
- DISALLOW_COPY_AND_ASSIGN(SimpleJwtVerifier);
-};
-
} // namespace kudu
[kudu] 02/06: [thirdparty] handle OpenSSL errors properly in curl and squeasel
Posted by la...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit a8ddd7e46ceaad882f2fe642462be4fb1ae78a0b
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu May 4 23:43:27 2023 -0700
[thirdparty] handle OpenSSL errors properly in curl and squeasel
This patch improves handling of OpenSSL errors in squeasel and curl,
making the code more robust.
This patch addresses the flakiness in the newly added JWT tests
that were failing in DEBUG-enabled builds because of the assertions
triggered by the SCOPED_OPENSSL_NO_PENDING_ERRORS macro.
I'm planning to submit the patches to the CURL project upstream and
post a PR to the squeasel repo [1] correspondingly. To expedite the
fix and avoid too many unrelated test failures for every CR submitted
to the upstream gerrit, it makes sense to just patch the current source
of these two thirdparty components as an interim solution.
[1] https://github.com/cloudera/squeasel
Change-Id: I209ceb35ded66222c90b82e98e7d191f33573217
Reviewed-on: http://gerrit.cloudera.org:8080/19849
Tested-by: Kudu Jenkins
Reviewed-by: Zoltan Chovan <zc...@cloudera.com>
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
(cherry picked from commit cc646190d3a16bf4f22ac9c32253cf7d4ad05cca)
Reviewed-on: http://gerrit.cloudera.org:8080/19978
Reviewed-by: Yingchun Lai <la...@apache.org>
---
thirdparty/download-thirdparty.sh | 8 +-
.../patches/curl-handle-openssl-errors.patch | 124 +++++++++++++
.../patches/squeasel-handle-openssl-errors.patch | 191 +++++++++++++++++++++
3 files changed, 320 insertions(+), 3 deletions(-)
diff --git a/thirdparty/download-thirdparty.sh b/thirdparty/download-thirdparty.sh
index 8d68e8e3b..88b4cfd32 100755
--- a/thirdparty/download-thirdparty.sh
+++ b/thirdparty/download-thirdparty.sh
@@ -277,11 +277,12 @@ fetch_and_patch \
"patch -p1 < $TP_DIR/patches/rapidjson-assertions-for-clang-warnings.patch" \
"patch -p1 < $TP_DIR/patches/rapidjson-avoid-pointer-arithmetic-on-null-pointer.patch"
-SQUEASEL_PATCHLEVEL=0
+SQUEASEL_PATCHLEVEL=1
fetch_and_patch \
squeasel-${SQUEASEL_VERSION}.tar.gz \
$SQUEASEL_SOURCE \
- $SQUEASEL_PATCHLEVEL
+ $SQUEASEL_PATCHLEVEL \
+ "patch -p1 < $TP_DIR/patches/squeasel-handle-openssl-errors.patch"
MUSTACHE_PATCHLEVEL=0
fetch_and_patch \
@@ -302,12 +303,13 @@ fetch_and_patch \
$GCOVR_SOURCE \
$GCOVR_PATCHLEVEL
-CURL_PATCHLEVEL=1
+CURL_PATCHLEVEL=2
fetch_and_patch \
curl-${CURL_VERSION}.tar.gz \
$CURL_SOURCE \
$CURL_PATCHLEVEL \
"patch -p1 < $TP_DIR/patches/curl-custom-openssl-library.patch" \
+ "patch -p1 < $TP_DIR/patches/curl-handle-openssl-errors.patch" \
"autoreconf -fvi"
CRCUTIL_PATCHLEVEL=0
diff --git a/thirdparty/patches/curl-handle-openssl-errors.patch b/thirdparty/patches/curl-handle-openssl-errors.patch
new file mode 100644
index 000000000..73626dc71
--- /dev/null
+++ b/thirdparty/patches/curl-handle-openssl-errors.patch
@@ -0,0 +1,124 @@
+--- a/lib/vtls/openssl.c 2023-05-03 19:15:43.000000000 -0700
++++ b/lib/vtls/openssl.c 2023-05-04 20:58:10.000000000 -0700
+@@ -405,6 +405,18 @@
+ return buf;
+ }
+
++static void ossl_error_details(struct Curl_easy *data, const char *err_msg) {
++ char err_buf[256];
++ while(true) {
++ unsigned long ssl_err = ERR_get_error();
++ if(!ssl_err) {
++ break;
++ }
++ ossl_strerror(ssl_err, err_buf, sizeof(err_buf));
++ failf(data, "%s: %s", err_msg, err_buf);
++ }
++}
++
+ /* Return an extra data index for the connection data.
+ * This index can be used with SSL_get_ex_data() and SSL_set_ex_data().
+ */
+@@ -796,14 +808,17 @@
+ }
+
+ if(SSL_CTX_use_PrivateKey(ctx, pri) != 1) {
+- failf(data, "unable to use private key from PKCS12 file '%s'",
+- cert_file);
++ failf(data, "unable to use private key from PKCS12 file '%s': %s",
++ cert_file, ossl_strerror(ERR_get_error(), error_buffer,
++ sizeof(error_buffer)));
+ goto fail;
+ }
+
+- if(!SSL_CTX_check_private_key (ctx)) {
++ if(SSL_CTX_check_private_key(ctx) != 1) {
+ failf(data, "private key from PKCS12 file '%s' "
+- "does not match certificate in same file", cert_file);
++ "does not match certificate in same file: %s", cert_file,
++ ossl_strerror(ERR_get_error(), error_buffer,
++ sizeof(error_buffer)));
+ goto fail;
+ }
+ /* Set Certificate Verification chain */
+@@ -860,8 +875,10 @@
+ /* FALLTHROUGH */
+ case SSL_FILETYPE_ASN1:
+ if(SSL_CTX_use_PrivateKey_file(ctx, key_file, file_type) != 1) {
+- failf(data, "unable to set private key file: '%s' type %s",
+- key_file, key_type?key_type:"PEM");
++ failf(data, "unable to set private key file: '%s' type %s: %s",
++ key_file, key_type ? key_type : "PEM",
++ ossl_strerror(ERR_get_error(), error_buffer,
++ sizeof(error_buffer)));
+ return 0;
+ }
+ break;
+@@ -903,7 +920,9 @@
+ return 0;
+ }
+ if(SSL_CTX_use_PrivateKey(ctx, priv_key) != 1) {
+- failf(data, "unable to set private key");
++ failf(data, "unable to set private key: %s",
++ ossl_strerror(ERR_get_error(), error_buffer,
++ sizeof(error_buffer)));
+ EVP_PKEY_free(priv_key);
+ return 0;
+ }
+@@ -975,7 +994,9 @@
+ /* Now we know that a key and cert have been set against
+ * the SSL context */
+ if(!SSL_CTX_check_private_key(ctx)) {
+- failf(data, "Private key does not match the certificate public key");
++ failf(data, "private key does not match the certificate public key: %s",
++ ossl_strerror(ERR_get_error(), error_buffer,
++ sizeof(error_buffer)));
+ return 0;
+ }
+ }
+@@ -2724,18 +2745,21 @@
+ /* tell SSL where to find CA certificates that are used to verify
+ the servers certificate. */
+ if(!SSL_CTX_load_verify_locations(BACKEND->ctx, ssl_cafile, ssl_capath)) {
++
++ static const char * const err_msg =
++ "error setting certificate verify locations";
+ if(verifypeer) {
+ /* Fail if we insist on successfully verifying the server. */
+- failf(data, "error setting certificate verify locations:\n"
+- " CAfile: %s\n CApath: %s",
++ failf(data, "%s:\n CAfile: %s\n CApath: %s",
++ err_msg,
+ ssl_cafile ? ssl_cafile : "none",
+ ssl_capath ? ssl_capath : "none");
++ ossl_error_details(data, err_msg);
+ return CURLE_SSL_CACERT_BADFILE;
+ }
+ /* Just continue with a warning if no strict certificate verification
+ is required. */
+- infof(data, "error setting certificate verify locations,"
+- " continuing anyway:\n");
++ infof(data, "%s, continuing anyway:\n", err_msg);
+ }
+ else {
+ /* Everything is fine. */
+@@ -2762,7 +2786,9 @@
+ X509_LOOKUP_file());
+ if(!lookup ||
+ (!X509_load_crl_file(lookup, ssl_crlfile, X509_FILETYPE_PEM)) ) {
+- failf(data, "error loading CRL file: %s", ssl_crlfile);
++ static const char * const err_msg = "error loading CRL file";
++ failf(data, "%s: %s", err_msg, ssl_crlfile);
++ ossl_error_details(data, err_msg);
+ return CURLE_SSL_CRL_BADFILE;
+ }
+ /* Everything is fine. */
+@@ -2994,6 +3020,8 @@
+ result = CURLE_SSL_CONNECT_ERROR;
+ ossl_strerror(errdetail, error_buffer, sizeof(error_buffer));
+ }
++ // Clear the rest of the errors as well.
++ ERR_clear_error();
+
+ /* detail is already set to the SSL error above */
+
diff --git a/thirdparty/patches/squeasel-handle-openssl-errors.patch b/thirdparty/patches/squeasel-handle-openssl-errors.patch
new file mode 100644
index 000000000..23dbfff13
--- /dev/null
+++ b/thirdparty/patches/squeasel-handle-openssl-errors.patch
@@ -0,0 +1,191 @@
+diff --git a/squeasel.c b/squeasel.c
+index d716783..ff40dcc 100644
+--- a/squeasel.c
++++ b/squeasel.c
+@@ -904,11 +904,26 @@ int sq_get_bound_addresses(const struct sq_context *ctx, struct sockaddr_in ***a
+ return rc;
+ }
+
++#if !defined(NO_SSL)
++
++// Return the last OpenSSL error message and clean the thread's error queue.
++static const char *ssl_error(void) {
++ unsigned long err = ERR_peek_error();
++ if (err != 0) {
++ // Clear the thread's error queue.
++ ERR_clear_error();
++ return ERR_error_string(err, NULL);
++ }
++ return "";
++}
++
++#endif
+
+ // Write data to the IO channel - opened file descriptor, socket or SSL
+-// descriptor. Return number of bytes written.
++// descriptor. Return number of bytes written. The 'conn' parameter is used
++// only for reporting on errors, if any.
+ static int64_t push(FILE *fp, SOCKET sock, SSL *ssl, const char *buf,
+- int64_t len) {
++ int64_t len, struct sq_connection *conn) {
+ int64_t sent;
+ int n, k;
+
+@@ -922,6 +937,12 @@ static int64_t push(FILE *fp, SOCKET sock, SSL *ssl, const char *buf,
+ #ifndef NO_SSL
+ if (ssl != NULL) {
+ n = SSL_write(ssl, buf + sent, k);
++ if (n <= 0) {
++ int err = SSL_get_error(ssl, n);
++ if (err != 0 && ERR_peek_error() != 0) {
++ cry(conn, "SSL_write error: %s", ssl_error());
++ }
++ }
+ } else
+ #endif
+ if (fp != NULL) {
+@@ -966,6 +987,12 @@ static int pull(FILE *fp, struct sq_connection *conn, char *buf, int len) {
+ #ifndef NO_SSL
+ } else if (conn->ssl != NULL) {
+ nread = SSL_read(conn->ssl, buf, len);
++ if (nread <= 0) {
++ int err = SSL_get_error(conn->ssl, nread);
++ if (err != 0 && ERR_peek_error() != 0) {
++ cry(conn, "SSL_read error: %s", ssl_error());
++ }
++ }
+ #endif
+ } else {
+ RETRY_ON_EINTR(nread, recv(conn->client.sock, buf, (size_t) len, 0));
+@@ -1050,14 +1077,14 @@ int sq_write(struct sq_connection *conn, const void *buf, size_t len) {
+ allowed = len;
+ }
+ if ((total = push(NULL, conn->client.sock, conn->ssl, (const char *) buf,
+- (int64_t) allowed)) == allowed) {
++ (int64_t) allowed, conn)) == allowed) {
+ buf = (char *) buf + total;
+ conn->last_throttle_bytes += total;
+ while (total < (int64_t) len && conn->ctx->stop_flag == 0) {
+ allowed = conn->throttle > (int64_t) len - total ?
+ (int64_t) len - total : conn->throttle;
+ if ((n = push(NULL, conn->client.sock, conn->ssl, (const char *) buf,
+- (int64_t) allowed)) != allowed) {
++ (int64_t) allowed, conn)) != allowed) {
+ break;
+ }
+ sleep(1);
+@@ -1069,7 +1096,7 @@ int sq_write(struct sq_connection *conn, const void *buf, size_t len) {
+ }
+ } else {
+ total = push(NULL, conn->client.sock, conn->ssl, (const char *) buf,
+- (int64_t) len);
++ (int64_t) len, conn);
+ }
+ return (int) total;
+ }
+@@ -2479,7 +2506,7 @@ static int forward_body_data(struct sq_connection *conn, FILE *fp,
+ if ((int64_t) buffered_len > conn->content_len) {
+ buffered_len = (int) conn->content_len;
+ }
+- push(fp, sock, ssl, body, (int64_t) buffered_len);
++ push(fp, sock, ssl, body, (int64_t) buffered_len, conn);
+ conn->consumed_content += buffered_len;
+ }
+
+@@ -2490,7 +2517,7 @@ static int forward_body_data(struct sq_connection *conn, FILE *fp,
+ to_read = (int) (conn->content_len - conn->consumed_content);
+ }
+ nread = pull(NULL, conn, buf, to_read);
+- if (nread <= 0 || push(fp, sock, ssl, buf, nread) != nread) {
++ if (nread <= 0 || push(fp, sock, ssl, buf, nread, conn) != nread) {
+ break;
+ }
+ conn->consumed_content += nread;
+@@ -3788,16 +3815,20 @@ static int set_uid_option(struct sq_context *ctx) {
+ static pthread_mutex_t *ssl_mutexes;
+
+ static int sslize(struct sq_connection *conn, SSL_CTX *s, int (*func)(SSL *)) {
+- return (conn->ssl = SSL_new(s)) != NULL &&
+- SSL_set_fd(conn->ssl, conn->client.sock) == 1 &&
+- func(conn->ssl) == 1;
+-}
+-
+-// Return OpenSSL error message
+-static const char *ssl_error(void) {
+- unsigned long err;
+- err = ERR_get_error();
+- return err == 0 ? "" : ERR_error_string(err, NULL);
++ conn->ssl = SSL_new(s);
++ if (conn->ssl == NULL) {
++ cry(conn, "SSL_new failed: %s", ssl_error());
++ return 0;
++ }
++ if (SSL_set_fd(conn->ssl, conn->client.sock) != 1) {
++ cry(conn, "SSL_set_fd failed: %s", ssl_error());
++ return 0;
++ }
++ if (func(conn->ssl) != 1) {
++ cry(conn, "sslize failed: %s", ssl_error());
++ return 0;
++ }
++ return 1;
+ }
+
+ static void ssl_locking_callback(int mode, int mutex_num, const char *file,
+@@ -3899,8 +3930,8 @@ static int set_ssl_option(struct sq_context *ctx) {
+ }
+
+ if ((SSL_CTX_set_options(ctx->ssl_ctx, options) & options) != options) {
+- cry(fc(ctx), "SSL_CTX_set_options (server) error: could not set options (%d)",
+- options);
++ cry(fc(ctx), "SSL_CTX_set_options (server) error: could not set options (%d) %s",
++ options, ssl_error());
+ return 0;
+ }
+
+@@ -3913,18 +3944,21 @@ static int set_ssl_option(struct sq_context *ctx) {
+ // set up certificate itself. In this case, skip sertificate setting.
+ if ((ctx->callbacks.init_ssl == NULL ||
+ !ctx->callbacks.init_ssl(ctx->ssl_ctx, ctx->user_data)) &&
+- (SSL_CTX_use_certificate_file(ctx->ssl_ctx, pem, 1) == 0 ||
+- SSL_CTX_use_PrivateKey_file(ctx->ssl_ctx, private_key, 1) == 0)) {
++ (SSL_CTX_use_certificate_file(ctx->ssl_ctx, pem, 1) != 1 ||
++ SSL_CTX_use_PrivateKey_file(ctx->ssl_ctx, private_key, 1) != 1)) {
+ cry(fc(ctx), "%s: cannot open %s: %s", __func__, pem, ssl_error());
+ return 0;
+ }
+
+ if (pem != NULL) {
+- (void) SSL_CTX_use_certificate_chain_file(ctx->ssl_ctx, pem);
++ if (SSL_CTX_use_certificate_chain_file(ctx->ssl_ctx, pem) != 1) {
++ cry(fc(ctx), "%s: cannot open %s: %s", __func__, pem, ssl_error());
++ return 0;
++ }
+ }
+
+ if (ctx->config[SSL_CIPHERS] != NULL) {
+- if (SSL_CTX_set_cipher_list(ctx->ssl_ctx, ctx->config[SSL_CIPHERS]) == 0) {
++ if (SSL_CTX_set_cipher_list(ctx->ssl_ctx, ctx->config[SSL_CIPHERS]) != 1) {
+ cry(fc(ctx), "SSL_CTX_set_cipher_list: error setting ciphers (%s): %s",
+ ctx->config[SSL_CIPHERS], ssl_error());
+ return 0;
+@@ -4018,7 +4052,9 @@ static void close_connection(struct sq_connection *conn) {
+ #ifndef NO_SSL
+ if (conn->ssl != NULL) {
+ // Run SSL_shutdown twice to ensure completly close SSL connection
+- SSL_shutdown(conn->ssl);
++ if (SSL_shutdown(conn->ssl) < 0) {
++ cry(conn, "error shutting down SSL connection: %s", ssl_error());
++ }
+ SSL_free(conn->ssl);
+ conn->ssl = NULL;
+ }
+@@ -4053,7 +4089,7 @@ struct sq_connection *sq_connect(const char *host, int port, int use_ssl,
+ #ifndef NO_SSL
+ } else if (use_ssl && (conn->client_ssl_ctx =
+ SSL_CTX_new(SSLv23_client_method())) == NULL) {
+- snprintf(ebuf, ebuf_len, "SSL_CTX_new error");
++ snprintf(ebuf, ebuf_len, "SSL_CTX_new error %s", ssl_error());
+ closesocket(sock);
+ free(conn);
+ conn = NULL;
[kudu] 06/06: jwt: introduce MiniOidc
Posted by la...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit c2fe33a03cc01f3544675e9fa5b7aff08f550a34
Author: Zoltan Chovan <zc...@cloudera.com>
AuthorDate: Thu Feb 2 10:39:23 2023 +0100
jwt: introduce MiniOidc
This patch takes some existing test utilities and encapsulates them into
the new MiniOidc class. The MiniOidc serves the purpose of being the
OpenID Connect Discovery endpoint, as the host for JWKSs of each
account, and the dispenser of JWTs.
This encapsulation will be useful in testing JWTs from non-C++ tests
that typically rely on exposing Mini* components via control shell.
Co-authored-by: Andrew Wong <aw...@apache.org>
Change-Id: I26e9b3bcd0946adbe4642a19c5ef1124e39632c6
Reviewed-on: http://gerrit.cloudera.org:8080/18473
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <al...@apache.org>
(cherry picked from commit 400d25681135b233c943a1b37d55718261578ff2)
Reviewed-on: http://gerrit.cloudera.org:8080/19967
Reviewed-by: Yingchun Lai <la...@apache.org>
Tested-by: Yingchun Lai <la...@apache.org>
---
src/kudu/util/CMakeLists.txt | 12 +++
src/kudu/util/jwt-util-test.cc | 192 +++++++++++------------------------------
src/kudu/util/jwt-util.cc | 7 +-
src/kudu/util/jwt-util.h | 6 +-
src/kudu/util/mini_oidc.cc | 163 ++++++++++++++++++++++++++++++++++
src/kudu/util/mini_oidc.h | 93 ++++++++++++++++++++
6 files changed, 323 insertions(+), 150 deletions(-)
diff --git a/src/kudu/util/CMakeLists.txt b/src/kudu/util/CMakeLists.txt
index 5223d9ef6..bf7d8a843 100644
--- a/src/kudu/util/CMakeLists.txt
+++ b/src/kudu/util/CMakeLists.txt
@@ -353,6 +353,17 @@ target_link_libraries(kudu_curl_util
glog
gutil)
+#######################################
+# mini_oidc
+#######################################
+set (MINI_OIDC_SRCS mini_oidc.cc)
+add_library(mini_oidc ${MINI_OIDC_SRCS})
+target_link_libraries(mini_oidc
+ server_process
+ gutil
+ kudu_test_util
+ kudu_util)
+
#######################################
# kudu_jwt_util
#######################################
@@ -630,6 +641,7 @@ ADD_KUDU_TEST(jwt-util-test)
if(NOT NO_TESTS)
target_link_libraries(jwt-util-test
kudu_jwt_util
+ mini_oidc
server_process)
endif()
diff --git a/src/kudu/util/jwt-util-test.cc b/src/kudu/util/jwt-util-test.cc
index 9ccc23cf0..bb7cc2dbe 100644
--- a/src/kudu/util/jwt-util-test.cc
+++ b/src/kudu/util/jwt-util-test.cc
@@ -36,7 +36,6 @@
#include <jwt-cpp/traits/kazuho-picojson/defaults.h>
#include <jwt-cpp/traits/kazuho-picojson/traits.h>
-#include "kudu/gutil/map-util.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/server/webserver.h"
#include "kudu/server/webserver_options.h"
@@ -44,6 +43,7 @@
#include "kudu/util/env.h"
#include "kudu/util/jwt-util-internal.h"
#include "kudu/util/jwt_test_certs.h"
+#include "kudu/util/mini_oidc.h"
#include "kudu/util/net/sockaddr.h"
#include "kudu/util/scoped_cleanup.h"
#include "kudu/util/slice.h"
@@ -58,6 +58,7 @@ using std::unique_ptr;
using std::unordered_map;
using std::vector;
using strings::Substitute;
+using kudu::MiniOidc;
/// Utility class for creating a file that will be automatically deleted upon test
/// completion.
@@ -933,158 +934,61 @@ TEST(JwtUtilTest, VerifyJWKSUrl) {
}
namespace {
-
-// $0: account_id
-// $1: jwks_uri
-const char kDiscoveryFormat[] = R"({
- "issuer": "auth0/$0",
- "token_endpoint": "dummy.endpoint.com",
- "response_types_supported": [
- "id_token"
- ],
- "claims_supported": [
- "sub",
- "aud",
- "iss",
- "exp"
- ],
- "subject_types_supported": [
- "public"
- ],
- "id_token_signing_alg_values_supported": [
- "RS256"
- ],
- "jwks_uri": "$1"
-})";
-
-void JWKSDiscoveryHandler(const Webserver::WebRequest& req,
- Webserver::PrerenderedWebResponse* resp,
- const JWKSMockServer& jwks_server) {
- const auto* account_id = FindOrNull(req.parsed_args, "accountid");
- if (!account_id) {
- resp->output << "expected 'accountId' query";
- resp->status_code = HttpStatusCode::BadRequest;
- return;
- }
- resp->output << Substitute(kDiscoveryFormat, *account_id,
- jwks_server.url_for_account(*account_id));
- resp->status_code = HttpStatusCode::Ok;
-}
-
const char kValidAccount[] = "new-phone";
const char kInvalidAccount[] = "who-is-this";
-const char kMissingAccount[] = "no-where";
-
-class JWKSDiscoveryEndpointMockServer {
- public:
- Status Start() {
- unordered_map<string, string> account_id_to_resp({
- {
- // Create an account that has valid keys.
- kValidAccount,
- Substitute(kJwksRsaFileFormat, kKid1, "RS256",
- kRsaPubKeyJwkN, kRsaPubKeyJwkE, kKid2, "RS256", kRsaInvalidPubKeyJwkN,
- kRsaPubKeyJwkE),
- },
- {
- // The keys associated with this account are invalid.
- kInvalidAccount,
- Substitute(kJwksRsaFileFormat, kKid1, "RS256",
- kRsaInvalidPubKeyJwkN, kRsaPubKeyJwkE, kKid2, "RS256",
- kRsaInvalidPubKeyJwkN, kRsaPubKeyJwkE),
- },
- });
- RETURN_NOT_OK(jwks_server_.StartWithAccounts(account_id_to_resp));
-
- WebserverOptions opts;
- opts.port = 0;
- webserver_.reset(new Webserver(opts));
- webserver_->RegisterPrerenderedPathHandler(
- "/.well-known/openid-configuration'", "openid-configuration",
- // Pass the 'accountId' query arguments to return a response that
- // points to the JWKS endpoint for the account.
- [this] (const Webserver::WebRequest& req, Webserver::PrerenderedWebResponse* resp) {
- JWKSDiscoveryHandler(req, resp, jwks_server_);
- },
- /*is_styled*/false, /*is_on_nav_bar*/false);
- RETURN_NOT_OK(webserver_->Start());
- vector<Sockaddr> addrs;
- RETURN_NOT_OK(webserver_->GetBoundAddresses(&addrs));
- RETURN_NOT_OK(addr_.ParseString("127.0.0.1", addrs[0].port()));
- url_ = Substitute("http://$0/.well-known/openid-configuration'", addr_.ToString());
- return Status::OK();
- }
-
- const string& url() const {
- return url_;
- }
- private:
- unique_ptr<Webserver> webserver_;
- JWKSMockServer jwks_server_;
- string url_;
- Sockaddr addr_;
-};
-
+const char kMissingAccount[] = "no-one";
} // anonymous namespace
-TEST(JwtUtilTest, VerifyJWKSDiscoveryEndpoint) {
- JWKSDiscoveryEndpointMockServer discovery_endpoint;
- ASSERT_OK(discovery_endpoint.Start());
- PerAccountKeyBasedJwtVerifier jwt_verifier(discovery_endpoint.url());
- {
- auto valid_user_token =
- jwt::create()
- .set_issuer(Substitute("auth0/$0", kValidAccount))
- .set_type("JWT")
- .set_algorithm("RS256")
- .set_key_id(kKid1)
- .set_subject(kValidAccount)
- .sign(jwt::algorithm::rs256(kRsaPubKeyPem, kRsaPrivKeyPem, "", ""));
- string subject;
- ASSERT_OK(jwt_verifier.VerifyToken(valid_user_token, &subject));
- ASSERT_EQ(kValidAccount, subject);
- }
- {
- auto invalid_user_token =
- jwt::create()
- .set_issuer(Substitute("auth0/$0", kInvalidAccount))
- .set_type("JWT")
- .set_algorithm("RS256")
- .set_key_id(kKid1)
- .set_subject(kInvalidAccount)
- .sign(jwt::algorithm::rs256(kRsaPubKeyPem, kRsaPrivKeyPem, "", ""));
- string subject;
- Status s = jwt_verifier.VerifyToken(invalid_user_token, &subject);
- ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
- }
- {
- auto missing_user_token =
- jwt::create()
- .set_issuer(Substitute("auth0/$0", kMissingAccount))
- .set_type("JWT")
- .set_algorithm("RS256")
- .set_key_id(kKid1)
- .set_subject(kMissingAccount)
- .sign(jwt::algorithm::rs256(kRsaPubKeyPem, kRsaPrivKeyPem, "", ""));
- string subject;
- Status s = jwt_verifier.VerifyToken(missing_user_token, &subject);
- ASSERT_TRUE(s.IsRemoteError()) << s.ToString();
+TEST(JwtUtilTest, VerifyOIDCDiscoveryEndpoint) {
+ MiniOidcOptions opts;
+ opts.account_ids = {
+ { kValidAccount, /*is_valid*/true },
+ { kInvalidAccount, /*is_valid*/false },
+ };
+ MiniOidc oidc(std::move(opts));
+ ASSERT_OK(oidc.Start());
+ const PerAccountKeyBasedJwtVerifier jwt_verifier(oidc.url());
+
+ // Create and verify a token on the happy path.
+ const string kSubject = "kudu";
+ auto valid_user_token =
+ MiniOidc::CreateJwt(kValidAccount, kSubject, /*is_valid*/true);
+ string subject;
+ ASSERT_OK(jwt_verifier.VerifyToken(valid_user_token, &subject));
+ ASSERT_EQ(kSubject, subject);
+
+ // Verify some expected failure scenarios.
+ const unordered_map<string, string> invalid_jwts {
+ { MiniOidc::CreateJwt(kInvalidAccount, kSubject, false), "invalid issuer with invalid "
+ "subject" },
+ { MiniOidc::CreateJwt(kInvalidAccount, kSubject, true), "invalid issuer with valid subject" },
+ { MiniOidc::CreateJwt(kValidAccount, kSubject, false), "valid issuer with invalid key id" },
+ { MiniOidc::CreateJwt(kMissingAccount, kSubject, true), "missing account" },
+ };
+
+ for (const auto& [jwt, msg] : invalid_jwts) {
+ string invalid_subject;
+ const Status s = jwt_verifier.VerifyToken(jwt, &invalid_subject);
+ EXPECT_FALSE(s.ok()) << Substitute("failed case $0: $1", msg, s.ToString());
}
}
TEST(JwtUtilTest, VerifyJWKSDiscoveryEndpointMultipleClients) {
- JWKSDiscoveryEndpointMockServer discovery_endpoint;
- ASSERT_OK(discovery_endpoint.Start());
- PerAccountKeyBasedJwtVerifier jwt_verifier(discovery_endpoint.url());
+ MiniOidcOptions opts;
+ opts.account_ids = {
+ { kValidAccount, /*is_valid*/true }
+ };
+ MiniOidc oidc(std::move(opts));
+ ASSERT_OK(oidc.Start());
+ PerAccountKeyBasedJwtVerifier jwt_verifier(oidc.url());
+
{
+ const string kSubject = "kudu";
auto valid_user_token =
- jwt::create()
- .set_issuer(Substitute("auth0/$0", kValidAccount))
- .set_type("JWT")
- .set_algorithm("RS256")
- .set_key_id(kKid1)
- .set_subject(kValidAccount)
- .sign(jwt::algorithm::rs256(kRsaPubKeyPem, kRsaPrivKeyPem, "", ""));
+ MiniOidc::CreateJwt(kValidAccount, kSubject, /*is_valid*/true);
+ string subject;
+ ASSERT_OK(jwt_verifier.VerifyToken(valid_user_token, &subject));
+ ASSERT_EQ(kSubject, subject);
int constexpr n = 8;
std::vector<std::thread> threads;
@@ -1095,7 +999,7 @@ TEST(JwtUtilTest, VerifyJWKSDiscoveryEndpointMultipleClients) {
threads.emplace_back([&](){
string subject;
CHECK_OK(jwt_verifier.VerifyToken(valid_user_token, &subject));
- CHECK_EQ(kValidAccount, subject);
+ CHECK_EQ(kSubject, subject);
latch.CountDown();
});
}
diff --git a/src/kudu/util/jwt-util.cc b/src/kudu/util/jwt-util.cc
index 78280ae1e..cff633ad8 100644
--- a/src/kudu/util/jwt-util.cc
+++ b/src/kudu/util/jwt-util.cc
@@ -968,10 +968,11 @@ Status PerAccountKeyBasedJwtVerifier::JWTHelperForToken(const JWTHelper::JWTDeco
}
}
- // Otherwise, use the Discovery Endpoint to determine what 'jwks_uri' to use.
+ // Otherwise, use the OIDC Discovery Endpoint to determine what 'jwks_uri' to
+ // use.
kudu::EasyCurl curl;
kudu::faststring dst;
- const auto discovery_endpoint = Substitute("$0?accountId=$1", discovery_base_, account_id);
+ const auto discovery_endpoint = Substitute("$0?accountId=$1", oidc_uri_, account_id);
curl.set_timeout(
kudu::MonoDelta::FromSeconds(static_cast<int64_t>(FLAGS_jwks_pulling_timeout_s)));
curl.set_verify_peer(false);
@@ -1017,7 +1018,7 @@ Status PerAccountKeyBasedJwtVerifier::JWTHelperForToken(const JWTHelper::JWTDeco
Status PerAccountKeyBasedJwtVerifier::Init() {
for (auto& [account_id, verifier] : jwt_by_account_id_) {
- verifier->Init(Substitute("$0?accountId=$1", discovery_base_, account_id),
+ verifier->Init(Substitute("$0?accountId=$1", oidc_uri_, account_id),
/*is_local_file*/false);
}
return Status::OK();
diff --git a/src/kudu/util/jwt-util.h b/src/kudu/util/jwt-util.h
index c98ce2b58..e25bbd8d4 100644
--- a/src/kudu/util/jwt-util.h
+++ b/src/kudu/util/jwt-util.h
@@ -117,8 +117,8 @@ class KeyBasedJwtVerifier : public JwtVerifier {
class PerAccountKeyBasedJwtVerifier : public JwtVerifier {
public:
- explicit PerAccountKeyBasedJwtVerifier(std::string jwks_uri)
- : discovery_base_(std::move(jwks_uri)) {}
+ explicit PerAccountKeyBasedJwtVerifier(std::string oidc_uri)
+ : oidc_uri_(std::move(oidc_uri)) {}
~PerAccountKeyBasedJwtVerifier() override = default;
@@ -131,7 +131,7 @@ class PerAccountKeyBasedJwtVerifier : public JwtVerifier {
// Returns an error if the token doesn't contain the appropriate fields.
Status JWTHelperForToken(const JWTHelper::JWTDecodedToken& token, JWTHelper** helper) const;
- std::string discovery_base_;
+ const std::string oidc_uri_;
// Marked as mutable so that PerAccountKeyBasedJwtVerifier::JWTHelperForToken is able to emplace
// new JWTHelpers in it.
mutable std::unordered_map<std::string, std::shared_ptr<JWTHelper>> jwt_by_account_id_;
diff --git a/src/kudu/util/mini_oidc.cc b/src/kudu/util/mini_oidc.cc
new file mode 100644
index 000000000..803ed1e14
--- /dev/null
+++ b/src/kudu/util/mini_oidc.cc
@@ -0,0 +1,163 @@
+// 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.
+#include "kudu/util/mini_oidc.h"
+
+#include <functional>
+#include <ostream>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include <jwt-cpp/jwt.h>
+#include <jwt-cpp/traits/kazuho-picojson/defaults.h>
+#include <jwt-cpp/traits/kazuho-picojson/traits.h>
+
+#include "kudu/gutil/map-util.h"
+#include "kudu/gutil/strings/substitute.h"
+#include "kudu/server/webserver.h"
+#include "kudu/server/webserver_options.h"
+#include "kudu/util/jwt_test_certs.h"
+#include "kudu/util/net/sockaddr.h"
+#include "kudu/util/web_callback_registry.h"
+
+using std::string;
+using std::vector;
+using strings::Substitute;
+
+namespace kudu {
+
+namespace {
+
+// $0: account_id
+// $1: jwks_uri
+const char kDiscoveryFormat[] = R"({
+ "issuer": "auth0/$0",
+ "token_endpoint": "dummy.endpoint.com",
+ "response_types_supported": [
+ "id_token"
+ ],
+ "claims_supported": [
+ "sub",
+ "aud",
+ "iss",
+ "exp"
+ ],
+ "subject_types_supported": [
+ "public"
+ ],
+ "id_token_signing_alg_values_supported": [
+ "RS256"
+ ],
+ "jwks_uri": "$1"
+})";
+
+void OidcDiscoveryHandler(const Webserver::WebRequest& req,
+ Webserver::PrerenderedWebResponse* resp,
+ const string& jwks_server_url) {
+ const auto* account_id = FindOrNull(req.parsed_args, "accountid");
+ if (!account_id) {
+ resp->output << "expected 'accountId' query";
+ resp->status_code = HttpStatusCode::BadRequest;
+ return;
+ }
+ resp->output << Substitute(kDiscoveryFormat, *account_id,
+ Substitute("$0/$1", jwks_server_url, *account_id));
+ resp->status_code = HttpStatusCode::Ok;
+}
+
+} // anonymous namespace
+
+MiniOidc::MiniOidc(MiniOidcOptions options) : options_(std::move(options)) {
+ if (options_.lifetime_ms == 0) {
+ // Setting default TTL for JWTs to 60 minutes
+ options_.lifetime_ms = 3600000;
+ }
+}
+
+// Explicitly defined outside the header to ensure users of the header don't
+// need to include member destructors.
+MiniOidc::~MiniOidc() {
+ Stop();
+}
+
+Status MiniOidc::Start() {
+ // Start the JWKS server and register path handlers for each of the accounts
+ // we've been configured to server.
+ WebserverOptions jwks_opts;
+ jwks_opts.port = 0;
+ jwks_server_.reset(new Webserver(jwks_opts));
+
+ for (const auto& [account_id, valid] : options_.account_ids) {
+ jwks_server_->RegisterPrerenderedPathHandler(Substitute("/jwks/$0", account_id), account_id,
+ [account_id = account_id, valid = valid] (const Webserver::WebRequest& /*req*/,
+ Webserver::PrerenderedWebResponse* resp) {
+ // NOTE: 'kKid1' points at a valid key, while 'kKid2' points at an
+ // invalid key.
+ resp->output << Substitute(kJwksRsaFileFormat, kKid1, "RS256",
+ valid ? kRsaPubKeyJwkN : kRsaInvalidPubKeyJwkN,
+ kRsaPubKeyJwkE, kKid2, "RS256", kRsaInvalidPubKeyJwkN, kRsaPubKeyJwkE),
+ resp->status_code = HttpStatusCode::Ok;
+ },
+ /*is_styled*/false, /*is_on_nav_bar*/false);
+ }
+ RETURN_NOT_OK(jwks_server_->Start());
+ vector<Sockaddr> bound_addrs;
+ Sockaddr addr;
+ RETURN_NOT_OK(jwks_server_->GetBoundAddresses(&bound_addrs));
+ RETURN_NOT_OK(addr.ParseString(bound_addrs[0].host(), bound_addrs[0].port()));
+ string const jwks_url = Substitute("http://$0/jwks", addr.ToString());
+
+ // Now start the OIDC Discovery server that points to the JWKS endpoints.
+ WebserverOptions oidc_opts;
+ oidc_opts.port = 0;
+ oidc_server_.reset(new Webserver(oidc_opts));
+ oidc_server_->RegisterPrerenderedPathHandler(
+ "/.well-known/openid-configuration", "openid-configuration",
+ // Pass the 'accountId' query arguments to return a response that
+ // points to the JWKS endpoint for the account.
+ [jwks_url] (const Webserver::WebRequest& req, Webserver::PrerenderedWebResponse* resp) {
+ OidcDiscoveryHandler(req, resp, jwks_url);
+ },
+ /*is_styled*/false, /*is_on_nav_bar*/false);
+ RETURN_NOT_OK(oidc_server_->Start());
+ bound_addrs.clear();
+ RETURN_NOT_OK(oidc_server_->GetBoundAddresses(&bound_addrs));
+ RETURN_NOT_OK(addr.ParseString(bound_addrs[0].host(), bound_addrs[0].port()));
+ oidc_url_ = Substitute("http://$0/.well-known/openid-configuration", addr.ToString());
+ return Status::OK();
+}
+
+void MiniOidc::Stop() {
+ if (oidc_server_) {
+ oidc_server_->Stop();
+ }
+ if (jwks_server_) {
+ jwks_server_->Stop();
+ }
+}
+
+string MiniOidc::CreateJwt(const string& account_id, const string& subject, bool is_valid) {
+ return jwt::create()
+ .set_issuer(Substitute("auth0/$0", account_id))
+ .set_type("JWT")
+ .set_algorithm("RS256")
+ .set_key_id(is_valid ? kKid1 : kKid2)
+ .set_subject(subject)
+ .sign(jwt::algorithm::rs256(kRsaPubKeyPem, kRsaPrivKeyPem, "", ""));
+}
+
+} // namespace kudu
diff --git a/src/kudu/util/mini_oidc.h b/src/kudu/util/mini_oidc.h
new file mode 100644
index 000000000..82f09ba35
--- /dev/null
+++ b/src/kudu/util/mini_oidc.h
@@ -0,0 +1,93 @@
+// 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.
+#pragma once
+
+#include <cstdint>
+#include <memory>
+#include <string>
+#include <unordered_map>
+
+#include "kudu/gutil/port.h"
+#include "kudu/util/status.h"
+
+namespace kudu {
+
+class Webserver;
+
+struct MiniOidcOptions {
+ // Port to bind to.
+ //
+ // Default: 0 (ephemeral port).
+ uint16_t port = 0;
+
+ // Lifetime of generated JWTs in milliseconds
+ uint64_t lifetime_ms = 0;
+
+ // Maps account IDs to add to whether or not to create JWKS with invalid keys.
+ std::unordered_map<std::string, bool> account_ids;
+};
+
+// Serves the following endpoints for testing a cluster:
+//
+// OIDC Discovery Endpoint: <url>/.well-known/openid-configuration/<account-id>
+// - Returns a JSON document containing the JWKS endpoint associated with the
+// given account ID, among other metadata
+// - See https://swagger.io/docs/specification/authentication/openid-connect-discovery/
+// for more details
+//
+// JWKS Endpoint: <url>/jwks/<account-id>
+// - Returns a JSON document containing the JWKS associated with the given
+// account ID
+// - See https://auth0.com/docs/secure/tokens/json-web-tokens/json-web-key-sets
+// for more details
+//
+// Verification of a JWT associated with a given account ID (typically denoted
+// as the final segment of the 'iss' field in the JWT payload) comprises of the
+// following steps:
+// - Query the OIDC Discovery Endpoint, specifying an 'accountId' argument from
+// the JWT.
+// - The endpoint returns a document containing the field 'jwks_uri', the JWKS
+// Endpoint.
+// - Query the JWKS Endpoint returns the JWKS associated with the account ID.
+// - The JWKS is used to verify the JWT.
+class MiniOidc {
+ public:
+ explicit MiniOidc(MiniOidcOptions options);
+ ~MiniOidc();
+
+ Status Start() WARN_UNUSED_RESULT;
+ void Stop();
+
+ // Creates a JWT with the given `account_id` and `subject`. If `is_valid` is set to false, the
+ // created token will be invalid.
+ static std::string CreateJwt(const std::string& account_id,
+ const std::string& subject,
+ bool is_valid);
+ const std::string& url() const {
+ return oidc_url_;
+ }
+ private:
+ MiniOidcOptions options_;
+
+ std::unique_ptr<Webserver> oidc_server_;
+
+ std::string oidc_url_;
+
+ std::unique_ptr<Webserver> jwks_server_;
+};
+
+} // namespace kudu
[kudu] 04/06: [Python] Refactor tests to use assertRaisesRegex
Posted by la...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 368b070fdfd16d45da400517dd9d3ee10180d5db
Author: Marton Greber <gr...@gmail.com>
AuthorDate: Mon Mar 20 11:01:14 2023 +0000
[Python] Refactor tests to use assertRaisesRegex
The Python unittest.TestCase class offers the assertRaises() and
assertRaisesRegex() functions to test that an exception is raised when
callable is called [1].
Currently assertRaises(exception) is used in the Python
client tests, to assert the type of a given error. This patch refactors
usages of assertRaises() to assertRaisesRegex(exception, regex) in order
to assert the error message as well.
In Python3.2 assertRaisesRegexp has been renamed to assertRaisesRegex
(without the trailing 'p') [2]. To be able to use assertRaisesRegex
throughout the Python codebase, a compatibility class called
CompatUnitTest is added.
[1] https://docs.python.org/3/library/unittest.html#unittest.TestCase
[2] https://docs.python.org/3.9/library/unittest.html
#unittest.TestCase.assertRaisesRegex
Change-Id: I41e2d69996ee0ed0f0418ae184d95239f2739efb
Reviewed-on: http://gerrit.cloudera.org:8080/19633
Tested-by: Kudu Jenkins
Reviewed-by: Attila Bukor <ab...@apache.org>
(cherry picked from commit 08bd36a8745d20c5aa688f5c33a05615857d40e0)
Reviewed-on: http://gerrit.cloudera.org:8080/19981
Reviewed-by: Marton Greber <gr...@gmail.com>
Reviewed-by: Yingchun Lai <la...@apache.org>
---
python/kudu/compat.py | 12 +++++-
python/kudu/tests/test_client.py | 58 +++++++++++++++++---------
python/kudu/tests/test_scanner.py | 1 -
python/kudu/tests/test_scantoken.py | 1 -
python/kudu/tests/test_schema.py | 83 +++++++++++++++++++++++++------------
python/kudu/tests/util.py | 4 +-
6 files changed, 108 insertions(+), 51 deletions(-)
diff --git a/python/kudu/compat.py b/python/kudu/compat.py
index c3343a6cb..6f8b415ce 100644
--- a/python/kudu/compat.py
+++ b/python/kudu/compat.py
@@ -18,7 +18,7 @@
# flake8: noqa
import itertools
-
+import contextlib
try:
import numpy as np
except ImportError:
@@ -97,6 +97,16 @@ else:
return list(o.items())
+if sys.version_info < (3, 1):
+ class CompatUnitTest(unittest.TestCase):
+ @contextlib.contextmanager
+ def assertRaisesRegex(self, expected_exception, expected_regex, *args, **kwargs):
+ with self.assertRaisesRegexp(expected_exception, expected_regex, *args, **kwargs):
+ yield
+else:
+ class CompatUnitTest(unittest.TestCase):
+ pass
+
integer_types = six.integer_types
if np is not None:
integer_types += (np.integer,)
diff --git a/python/kudu/tests/test_client.py b/python/kudu/tests/test_client.py
index ce76a87d3..44d8e4a29 100755
--- a/python/kudu/tests/test_client.py
+++ b/python/kudu/tests/test_client.py
@@ -16,7 +16,7 @@
# specific language governing permissions and limitations
# under the License.
-from kudu.compat import unittest, long
+from kudu.compat import CompatUnitTest, long
from kudu.tests.common import KuduTestBase
from kudu.client import (Partitioning,
RangePartition,
@@ -32,7 +32,7 @@ import datetime
from pytz import utc
-class TestClient(KuduTestBase, unittest.TestCase):
+class TestClient(KuduTestBase, CompatUnitTest):
def setUp(self):
pass
@@ -102,7 +102,8 @@ class TestClient(KuduTestBase, unittest.TestCase):
assert not self.client.table_exists(name)
# Should raise a more meaningful exception at some point
- with self.assertRaises(kudu.KuduNotFound):
+ error_msg = 'the table does not exist: table_name: "{0}"'.format(name)
+ with self.assertRaisesRegex(kudu.KuduNotFound, error_msg):
self.client.delete_table(name)
def test_table_nonexistent(self):
@@ -476,10 +477,12 @@ class TestClient(KuduTestBase, unittest.TestCase):
# inserts. In this case, at the second position it would like to get an int64 (the type
# of the auto-incrementing counter), therefore we get type error. (Specifying the
# auto-incremeintg counter is obviously rejected from the server side)
- with self.assertRaises(TypeError):
+ error_msg = 'an integer is required'
+ with self.assertRaisesRegex(TypeError, error_msg):
op = table.new_insert((1,'text'))
- with self.assertRaises(TypeError):
+ error_msg = 'an integer is required'
+ with self.assertRaisesRegex(TypeError, error_msg):
op = table.new_insert([1,'text'])
finally:
@@ -534,8 +537,10 @@ class TestClient(KuduTestBase, unittest.TestCase):
self.client.new_session(flush_mode='sync')
self.client.new_session(flush_mode='background')
- with self.assertRaises(ValueError):
- self.client.new_session(flush_mode='foo')
+ flush_mode = 'foo'
+ error_msg = 'Invalid flush mode: {0}'.format(flush_mode)
+ with self.assertRaisesRegex(ValueError, error_msg):
+ self.client.new_session(flush_mode=flush_mode)
def test_session_mutation_buffer_settings(self):
self.client.new_session(flush_mode=kudu.FLUSH_AUTO_BACKGROUND,
@@ -553,16 +558,20 @@ class TestClient(KuduTestBase, unittest.TestCase):
def test_session_mutation_buffer_errors(self):
session = self.client.new_session(flush_mode=kudu.FLUSH_AUTO_BACKGROUND)
- with self.assertRaises(OverflowError):
+ error_msg = 'can\'t convert negative value to unsigned int'
+ with self.assertRaisesRegex(OverflowError, error_msg):
session.set_mutation_buffer_max_num(-1)
- with self.assertRaises(kudu.errors.KuduInvalidArgument):
+ error_msg = '120: watermark must be between 0 and 100 inclusive'
+ with self.assertRaisesRegex(kudu.errors.KuduInvalidArgument, error_msg):
session.set_mutation_buffer_flush_watermark(1.2)
- with self.assertRaises(OverflowError):
+ error_msg = 'can\'t convert negative value to unsigned int'
+ with self.assertRaisesRegex(OverflowError, error_msg):
session.set_mutation_buffer_flush_interval(-1)
- with self.assertRaises(OverflowError):
+ error_msg = 'can\'t convert negative value to size_t'
+ with self.assertRaisesRegex(OverflowError, error_msg):
session.set_mutation_buffer_space(-1)
def test_connect_timeouts(self):
@@ -599,11 +608,13 @@ class TestClient(KuduTestBase, unittest.TestCase):
op[key[0]] = 'test'
# Test incorrectly typed data
- with self.assertRaises(TypeError):
+ error_msg = 'an integer is required'
+ with self.assertRaisesRegex(TypeError, error_msg):
op['int_val'] = 'incorrect'
# Test setting NULL in a not-null column
- with self.assertRaises(kudu.errors.KuduInvalidArgument):
+ error_msg = 'column not nullable: key'
+ with self.assertRaisesRegex(kudu.errors.KuduInvalidArgument, error_msg):
op['key'] = None
def test_alter_table_rename(self):
@@ -821,22 +832,30 @@ class TestClient(KuduTestBase, unittest.TestCase):
# negatives
alterer = self.client.new_table_alterer(table)
alterer.drop_column(col_name)
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'can\'t drop column: {0}'\
+ .format(Schema.get_auto_incrementing_column_name())
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
alterer.alter()
alterer = self.client.new_table_alterer(table)
alterer.add_column(col_name)
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'can\'t add a column with reserved name: {0}'\
+ .format(Schema.get_auto_incrementing_column_name())
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
alterer.alter()
alterer = self.client.new_table_alterer(table)
alterer.alter_column(col_name, "new_column_name")
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'can\'t change name for column: {0}'\
+ .format(Schema.get_auto_incrementing_column_name())
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
alterer.alter()
alterer = self.client.new_table_alterer(table)
alterer.alter_column(col_name).remove_default()
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'can\'t change remove default for column: {0}'\
+ .format(Schema.get_auto_incrementing_column_name())
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
alterer.alter()
# positives
@@ -869,11 +888,12 @@ class TestClient(KuduTestBase, unittest.TestCase):
def test_require_authn(self):
# Kerberos is not enabled on the cluster, so requiring
# authentication is expected to fail.
- with self.assertRaises(kudu.KuduBadStatus):
+ error_msg = 'client requires authentication, but server does not have Kerberos enabled'
+ with self.assertRaisesRegex(kudu.KuduBadStatus, error_msg):
client = kudu.connect(self.master_hosts, self.master_ports,
require_authentication=True)
-class TestMonoDelta(unittest.TestCase):
+class TestMonoDelta(CompatUnitTest):
def test_empty_ctor(self):
delta = kudu.TimeDelta()
diff --git a/python/kudu/tests/test_scanner.py b/python/kudu/tests/test_scanner.py
index fa406252d..c4d17839b 100644
--- a/python/kudu/tests/test_scanner.py
+++ b/python/kudu/tests/test_scanner.py
@@ -18,7 +18,6 @@
from __future__ import division
-from kudu.compat import unittest
from kudu.tests.util import TestScanBase
from kudu.tests.common import KuduTestBase, TimeoutError
import kudu
diff --git a/python/kudu/tests/test_scantoken.py b/python/kudu/tests/test_scantoken.py
index 9214adb98..c69b43a66 100644
--- a/python/kudu/tests/test_scantoken.py
+++ b/python/kudu/tests/test_scantoken.py
@@ -16,7 +16,6 @@
# specific language governing permissions and limitations
# under the License.
-from kudu.compat import unittest
from kudu.tests.util import TestScanBase
from kudu.tests.common import KuduTestBase
import kudu
diff --git a/python/kudu/tests/test_schema.py b/python/kudu/tests/test_schema.py
index 069201f64..cbee07b1e 100644
--- a/python/kudu/tests/test_schema.py
+++ b/python/kudu/tests/test_schema.py
@@ -18,13 +18,13 @@
from __future__ import division
-from kudu.compat import unittest
+from kudu.compat import CompatUnitTest
from kudu.errors import KuduInvalidArgument
import kudu
from kudu.schema import Schema
-class TestSchema(unittest.TestCase):
+class TestSchema(CompatUnitTest):
def setUp(self):
self.columns = [('one', 'int32', False),
@@ -59,8 +59,10 @@ class TestSchema(unittest.TestCase):
assert self.schema.primary_keys() == ['one', 'two']
def test_getitem_boundschecking(self):
- with self.assertRaises(IndexError):
- self.schema[4]
+ idx = 4
+ error_msg = 'Column index {0} is not in range'.format(idx)
+ with self.assertRaisesRegex(IndexError, error_msg):
+ self.schema[idx]
def test_getitem_wraparound(self):
# wraparound
@@ -75,7 +77,8 @@ class TestSchema(unittest.TestCase):
assert result.equals(expected)
- with self.assertRaises(KeyError):
+ error_msg = 'not_found'
+ with self.assertRaisesRegex(KeyError, error_msg):
self.schema['not_found']
def test_schema_equals(self):
@@ -113,8 +116,10 @@ class TestSchema(unittest.TestCase):
bar = builder.add_column('bar', 'string')
bar.compression(kudu.COMPRESSION_ZLIB)
- with self.assertRaises(ValueError):
- bar = builder.add_column('qux', 'string', compression='unknown')
+ compression = 'unknown'
+ error_msg = 'Invalid compression type: {0}'.format(compression)
+ with self.assertRaisesRegex(ValueError, error_msg):
+ bar = builder.add_column('qux', 'string', compression=compression)
builder.set_primary_keys(['key'])
builder.build()
@@ -136,7 +141,8 @@ class TestSchema(unittest.TestCase):
bar = builder.add_column('bar', 'string')
bar.encoding(kudu.ENCODING_PLAIN)
- with self.assertRaises(ValueError):
+ error_msg = 'Invalid encoding type'
+ with self.assertRaisesRegex(ValueError, error_msg):
builder.add_column('qux', 'string', encoding='unknown')
builder.set_primary_keys(['key'])
@@ -169,7 +175,8 @@ class TestSchema(unittest.TestCase):
.primary_key()
.nullable(False))
- with self.assertRaises(kudu.KuduInvalidArgument):
+ error_msg = 'no precision provided for decimal column: key'
+ with self.assertRaisesRegex(kudu.KuduInvalidArgument, error_msg):
builder.build()
def test_precision_on_non_decimal_column(self):
@@ -181,7 +188,8 @@ class TestSchema(unittest.TestCase):
.precision(9)
.scale(2))
- with self.assertRaises(kudu.KuduInvalidArgument):
+ error_msg = 'precision is not valid on a 2 column: key'
+ with self.assertRaisesRegex(kudu.KuduInvalidArgument, error_msg):
builder.build()
def test_date(self):
@@ -220,7 +228,8 @@ class TestSchema(unittest.TestCase):
.primary_key()
.nullable(False))
- with self.assertRaises(kudu.KuduInvalidArgument):
+ error_msg = 'no length provided for VARCHAR column: key'
+ with self.assertRaisesRegex(kudu.KuduInvalidArgument, error_msg):
builder.build()
def test_varchar_invalid_length(self):
@@ -231,7 +240,8 @@ class TestSchema(unittest.TestCase):
.length(0)
.nullable(False))
- with self.assertRaises(kudu.KuduInvalidArgument):
+ error_msg = 'length must be between 1 and 65535: key'
+ with self.assertRaisesRegex(kudu.KuduInvalidArgument, error_msg):
builder.build()
def test_length_on_non_varchar_column(self):
@@ -242,17 +252,20 @@ class TestSchema(unittest.TestCase):
.nullable(False)
.length(10))
- with self.assertRaises(kudu.KuduInvalidArgument):
+ error_msg = 'no precision provided for decimal column: key'
+ with self.assertRaisesRegex(kudu.KuduInvalidArgument, error_msg):
builder.build()
def test_unsupported_col_spec_methods_for_create_table(self):
builder = kudu.schema_builder()
builder.add_column('test', 'int64').rename('test')
- with self.assertRaises(kudu.KuduNotSupported):
+ error_msg = 'cannot rename a column during CreateTable: test'
+ with self.assertRaisesRegex(kudu.KuduNotSupported, error_msg):
builder.build()
builder.add_column('test', 'int64').remove_default()
- with self.assertRaises(kudu.KuduNotSupported):
+ error_msg = 'cannot rename a column during CreateTable: test'
+ with self.assertRaisesRegex(kudu.KuduNotSupported, error_msg):
builder.build()
def test_set_column_spec_pk(self):
@@ -352,7 +365,8 @@ class TestSchema(unittest.TestCase):
builder.add_column('key1', 'int64').nullable(False)
builder.add_column('key2', 'double').nullable(False)
builder.set_non_unique_primary_keys(['key2', 'key1'])
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'primary key columns must be listed first in the schema: key'
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
schema = builder.build()
def test_set_non_unique_primary_keys_not_first(self):
@@ -361,7 +375,8 @@ class TestSchema(unittest.TestCase):
(builder.add_column('key', 'int64')
.nullable(False))
builder.set_non_unique_primary_keys(['key'])
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'primary key columns must be listed first in the schema: key'
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
schema = builder.build()
def test_set_non_unique_primary_keys_same_name_twice(self):
@@ -370,7 +385,8 @@ class TestSchema(unittest.TestCase):
.nullable(False))
builder.add_column('data1', 'double')
builder.set_non_unique_primary_keys(['key', 'key'])
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'primary key columns must be listed first in the schema: key'
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
schema = builder.build()
def test_unique_and_non_unique_primary_key_on_same_column(self):
@@ -403,7 +419,8 @@ class TestSchema(unittest.TestCase):
(builder.add_column('key', 'double')
.nullable(False)
.non_unique_primary_key())
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'primary key column must be the first column'
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_unique_and_non_unique_primary_key_on_different_cols(self):
@@ -414,7 +431,8 @@ class TestSchema(unittest.TestCase):
(builder.add_column('key2', 'double')
.nullable(False)
.non_unique_primary_key())
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'multiple columns specified for primary key: key1, key2'
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_non_unique_and_unique_primary_key_on_different_cols(self):
@@ -425,7 +443,8 @@ class TestSchema(unittest.TestCase):
(builder.add_column('key2', 'double')
.nullable(False)
.primary_key())
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'multiple columns specified for primary key: key1, key2'
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_multiple_non_unique_primary_keys(self):
@@ -436,7 +455,8 @@ class TestSchema(unittest.TestCase):
(builder.add_column('key2', 'double')
.nullable(False)
.non_unique_primary_key())
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'multiple columns specified for primary key: key1, key2'
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_non_unique_primary_key_and_set_non_unique_primary_keys(self):
@@ -446,7 +466,9 @@ class TestSchema(unittest.TestCase):
.non_unique_primary_key())
builder.add_column('data1', 'double')
builder.set_non_unique_primary_keys(['key'])
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = ('primary key specified by both SetNonUniquePrimaryKey\(\)'
+ ' and on a specific column: key')
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_primary_key_and_set_non_unique_primary_keys(self):
@@ -456,7 +478,9 @@ class TestSchema(unittest.TestCase):
.primary_key())
builder.add_column('data1', 'double')
builder.set_non_unique_primary_keys(['key'])
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = ('primary key specified by both SetNonUniquePrimaryKey\(\)'
+ ' and on a specific column: key')
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_primary_key_and_set_primary_keys(self):
@@ -466,7 +490,9 @@ class TestSchema(unittest.TestCase):
.primary_key())
builder.add_column('data1', 'double')
builder.set_primary_keys(['key'])
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = ('primary key specified by both SetPrimaryKey\(\)'
+ ' and on a specific column: key')
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_non_unique_primary_key_and_set_primary_keys(self):
@@ -476,7 +502,9 @@ class TestSchema(unittest.TestCase):
.non_unique_primary_key())
builder.add_column('data1', 'double')
builder.set_primary_keys(['key'])
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = ('primary key specified by both SetPrimaryKey\(\)'
+ ' and on a specific column: key')
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_set_non_unique_and_set_unique_primary_key(self):
@@ -507,7 +535,8 @@ class TestSchema(unittest.TestCase):
.nullable(False)
.primary_key())
builder.add_column(Schema.get_auto_incrementing_column_name(), 'double')
- with self.assertRaises(KuduInvalidArgument):
+ error_msg = 'auto_incrementing_id is a reserved column name'
+ with self.assertRaisesRegex(KuduInvalidArgument, error_msg):
builder.build()
def test_default_value(self):
diff --git a/python/kudu/tests/util.py b/python/kudu/tests/util.py
index c3c91a5ae..3f6c0c5bc 100644
--- a/python/kudu/tests/util.py
+++ b/python/kudu/tests/util.py
@@ -18,14 +18,14 @@
# under the License.
from decimal import Decimal
-from kudu.compat import unittest
+from kudu.compat import CompatUnitTest
from kudu.client import Partitioning
from kudu.tests.common import KuduTestBase
import kudu
import datetime
import pytz
-class TestScanBase(KuduTestBase, unittest.TestCase):
+class TestScanBase(KuduTestBase, CompatUnitTest):
@classmethod
def setUpClass(self):
[kudu] 01/06: [tools] add bound_http_address into DaemonInfoPB
Posted by la...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 95119055741496d3611ef0cff7cac4b018b015df
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Mon May 22 13:09:41 2023 -0700
[tools] add bound_http_address into DaemonInfoPB
This patch adds bound_http_address field into the DaemonInfoPB message.
The new field will be used by test scenarios in follow-up changelists.
Change-Id: I746876e961a9fe42fb20c72c6ccff4afb3fb7943
Reviewed-on: http://gerrit.cloudera.org:8080/19912
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
(cherry picked from commit eae0ebf91e94be8341a18cfbb8e4c9dca5d6ef47)
Reviewed-on: http://gerrit.cloudera.org:8080/19985
Reviewed-by: Yingchun Lai <la...@apache.org>
Tested-by: Yingchun Lai <la...@apache.org>
---
src/kudu/tools/tool.proto | 3 +++
src/kudu/tools/tool_action_test.cc | 10 ++++++----
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/kudu/tools/tool.proto b/src/kudu/tools/tool.proto
index 1fa7878a1..b98345bba 100644
--- a/src/kudu/tools/tool.proto
+++ b/src/kudu/tools/tool.proto
@@ -153,6 +153,9 @@ message DaemonInfoPB {
// Daemon's bound RPC address.
optional HostPortPB bound_rpc_address = 2;
+
+ // Bound address of the embedded web server.
+ optional HostPortPB bound_http_address = 3;
}
// Response to a GetMastersRequestPB.
diff --git a/src/kudu/tools/tool_action_test.cc b/src/kudu/tools/tool_action_test.cc
index 11153aa98..25f6787b0 100644
--- a/src/kudu/tools/tool_action_test.cc
+++ b/src/kudu/tools/tool_action_test.cc
@@ -244,11 +244,12 @@ Status ProcessRequest(const ControlShellRequestPB& req,
{
RETURN_NOT_OK(CheckClusterExists(*cluster));
for (int i = 0; i < (*cluster)->num_masters(); i++) {
- HostPortPB pb = HostPortToPB((*cluster)->master(i)->bound_rpc_hostport());
+ const auto* m = (*cluster)->master(i);
DaemonInfoPB* info = resp->mutable_get_masters()->mutable_masters()->Add();
info->mutable_id()->set_type(MASTER);
info->mutable_id()->set_index(i);
- *info->mutable_bound_rpc_address() = std::move(pb);
+ *info->mutable_bound_rpc_address() = HostPortToPB(m->bound_rpc_hostport());
+ *info->mutable_bound_http_address() = HostPortToPB(m->bound_http_hostport());
}
break;
}
@@ -256,11 +257,12 @@ Status ProcessRequest(const ControlShellRequestPB& req,
{
RETURN_NOT_OK(CheckClusterExists(*cluster));
for (int i = 0; i < (*cluster)->num_tablet_servers(); i++) {
- HostPortPB pb = HostPortToPB((*cluster)->tablet_server(i)->bound_rpc_hostport());
+ const auto* ts = (*cluster)->tablet_server(i);
DaemonInfoPB* info = resp->mutable_get_tservers()->mutable_tservers()->Add();
info->mutable_id()->set_type(TSERVER);
info->mutable_id()->set_index(i);
- *info->mutable_bound_rpc_address() = std::move(pb);
+ *info->mutable_bound_rpc_address() = HostPortToPB(ts->bound_rpc_hostport());
+ *info->mutable_bound_http_address() = HostPortToPB(ts->bound_http_hostport());
}
break;
}
[kudu] 03/06: jwt: add control points for test binaries
Posted by la...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 563274ae2fd52756469a13337d392abd74b96115
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Mon May 2 12:08:50 2022 -0700
jwt: add control points for test binaries
This hooks the configurations of the MiniOidc into the Java test binary,
allowing test code to add statically-defined JWKS associated with
specific account IDs, to be fetched via OIDC Discovery Endpoint.
Change-Id: I489a67e93610467c5a2caabcf3e5603cbb49d118
Reviewed-on: http://gerrit.cloudera.org:8080/18476
Tested-by: Kudu Jenkins
Reviewed-by: Attila Bukor <ab...@apache.org>
(cherry picked from commit 3d56a26b962d83f3f2789b2f2c7c2967322e98e9)
Reviewed-on: http://gerrit.cloudera.org:8080/19970
Reviewed-by: Yingchun Lai <la...@apache.org>
---
.../org/apache/kudu/backup/TestKuduBackup.scala | 6 +--
.../apache/kudu/test/cluster/MiniKuduCluster.java | 45 ++++++++++++++++++++--
.../org/apache/kudu/test/TestMiniKuduCluster.java | 17 ++++++++
src/kudu/tools/tool.proto | 10 ++++-
src/kudu/tools/tool_action_test.cc | 24 ++++++++++++
5 files changed, 94 insertions(+), 8 deletions(-)
diff --git a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
index f1421b5ff..0a61f989c 100644
--- a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
+++ b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
@@ -609,7 +609,7 @@ class TestKuduBackup extends KuduTestSuite {
// Check the range bounds and the hash schema of each range of the restored table.
val restoredTable = kuduClient.openTable(s"$tableName-restore")
assertEquals(
- "[0 <= VALUES < 100 HASH(key) PARTITIONS 2, " +
+ "[0 <= VALUES < 100 HASH(key) PARTITIONS 2, " +
"100 <= VALUES < 200 HASH(key) PARTITIONS 3]",
restoredTable.getFormattedRangePartitionsWithHashSchema(10000).toString
)
@@ -635,7 +635,7 @@ class TestKuduBackup extends KuduTestSuite {
// Check the range bounds and the hash schema of each range of the restored table.
val restoredTable = kuduClient.openTable(s"$tableName-restore")
assertEquals(
- "[0 <= VALUES < 100 HASH(key) PARTITIONS 2, " +
+ "[0 <= VALUES < 100 HASH(key) PARTITIONS 2, " +
"100 <= VALUES < 200 HASH(key) PARTITIONS 3, " +
"200 <= VALUES < 300 HASH(key) PARTITIONS 4]",
restoredTable.getFormattedRangePartitionsWithHashSchema(10000).toString
@@ -694,7 +694,7 @@ class TestKuduBackup extends KuduTestSuite {
// Check the range bounds and the hash schema of each range of the restored table.
val restoredTable = kuduClient.openTable(s"$tableName-restore")
assertEquals(
- "[0 <= VALUES < 100 HASH(key) PARTITIONS 2, " +
+ "[0 <= VALUES < 100 HASH(key) PARTITIONS 2, " +
"100 <= VALUES < 200 HASH(key) PARTITIONS 3, " +
"200 <= VALUES < 300 HASH(key) PARTITIONS 5, " +
"300 <= VALUES < 400 HASH(key) PARTITIONS 6]",
diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
index cd5eb2acb..31bcdc6f9 100644
--- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
+++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
@@ -50,7 +50,10 @@ import org.apache.kudu.test.TempDirUtils;
import org.apache.kudu.tools.Tool.ControlShellRequestPB;
import org.apache.kudu.tools.Tool.ControlShellResponsePB;
import org.apache.kudu.tools.Tool.CreateClusterRequestPB;
+import org.apache.kudu.tools.Tool.CreateClusterRequestPB.JwksOptionsPB;
import org.apache.kudu.tools.Tool.CreateClusterRequestPB.MiniKdcOptionsPB;
+import org.apache.kudu.tools.Tool.CreateClusterRequestPB.MiniOidcOptionsPB;
+import org.apache.kudu.tools.Tool.CreateJwtRequestPB;
import org.apache.kudu.tools.Tool.DaemonIdentifierPB;
import org.apache.kudu.tools.Tool.DaemonInfoPB;
import org.apache.kudu.tools.Tool.GetKDCEnvVarsRequestPB;
@@ -110,9 +113,11 @@ public final class MiniKuduCluster implements AutoCloseable {
private final ImmutableList<String> locationInfo;
private final String clusterRoot;
private final String principal;
+ private final boolean enableClientJwt;
private MiniKdcOptionsPB kdcOptionsPb;
private final Common.HmsMode hmsMode;
+ private MiniOidcOptionsPB oidcOptionsPb;
private MiniKuduCluster(boolean enableKerberos,
int numMasters,
@@ -123,7 +128,9 @@ public final class MiniKuduCluster implements AutoCloseable {
MiniKdcOptionsPB kdcOptionsPb,
String clusterRoot,
Common.HmsMode hmsMode,
- String principal) {
+ String principal,
+ boolean enableClientJwt,
+ MiniOidcOptionsPB oidcOptionsPb) {
this.enableKerberos = enableKerberos;
this.numMasters = numMasters;
this.numTservers = numTservers;
@@ -133,6 +140,8 @@ public final class MiniKuduCluster implements AutoCloseable {
this.kdcOptionsPb = kdcOptionsPb;
this.principal = principal;
this.hmsMode = hmsMode;
+ this.enableClientJwt = enableClientJwt;
+ this.oidcOptionsPb = oidcOptionsPb;
if (clusterRoot == null) {
// If a cluster root was not set, create a unique temp directory to use.
@@ -227,7 +236,8 @@ public final class MiniKuduCluster implements AutoCloseable {
.addAllExtraTserverFlags(extraTserverFlags)
.setMiniKdcOptions(kdcOptionsPb)
.setClusterRoot(clusterRoot)
- .setPrincipal(principal);
+ .setPrincipal(principal)
+ .setMiniOidcOptions(oidcOptionsPb);
// Set up the location mapping command flag if there is location info.
if (!locationInfo.isEmpty()) {
@@ -323,6 +333,18 @@ public final class MiniKuduCluster implements AutoCloseable {
return principal;
}
+ public String createJwtFor(String accountId, String subject, boolean isValid) throws IOException {
+ ControlShellResponsePB resp = sendRequestToCluster(ControlShellRequestPB.newBuilder()
+ .setCreateJwt(CreateJwtRequestPB
+ .newBuilder()
+ .setAccountId(accountId)
+ .setSubject(subject)
+ .setIsValidKey(isValid)
+ .build())
+ .build());
+ return resp.getCreateJwt().getJwt();
+ }
+
/**
* Starts a master identified by a host and port.
* Does nothing if the server was already running.
@@ -703,8 +725,10 @@ public final class MiniKuduCluster implements AutoCloseable {
private final List<String> locationInfo = new ArrayList<>();
private String clusterRoot = null;
private String principal = "kudu";
+ private boolean enableClientJwt = false;
private MiniKdcOptionsPB.Builder kdcOptionsPb = MiniKdcOptionsPB.newBuilder();
+ private MiniOidcOptionsPB.Builder oidcOptionsPb = MiniOidcOptionsPB.newBuilder();
private Common.HmsMode hmsMode = Common.HmsMode.NONE;
public MiniKuduClusterBuilder numMasterServers(int numMasterServers) {
@@ -726,6 +750,11 @@ public final class MiniKuduCluster implements AutoCloseable {
return this;
}
+ public MiniKuduClusterBuilder enableClientJwt() {
+ enableClientJwt = true;
+ return this;
+ }
+
public MiniKuduClusterBuilder enableHiveMetastoreIntegration() {
hmsMode = Common.HmsMode.ENABLE_METASTORE_INTEGRATION;
return this;
@@ -788,6 +817,15 @@ public final class MiniKuduCluster implements AutoCloseable {
return this;
}
+ public MiniKuduClusterBuilder addJwks(String accountId, boolean isValid) {
+ this.oidcOptionsPb.addJwksOptions(
+ JwksOptionsPB.newBuilder()
+ .setAccountId(accountId)
+ .setIsValidKey(isValid)
+ .build());
+ return this;
+ }
+
/**
* Builds and starts a new {@link MiniKuduCluster} using builder state.
* @return the newly started {@link MiniKuduCluster}
@@ -798,7 +836,8 @@ public final class MiniKuduCluster implements AutoCloseable {
new MiniKuduCluster(enableKerberos,
numMasterServers, numTabletServers,
extraTabletServerFlags, extraMasterServerFlags, locationInfo,
- kdcOptionsPb.build(), clusterRoot, hmsMode, principal);
+ kdcOptionsPb.build(), clusterRoot, hmsMode, principal, enableClientJwt,
+ oidcOptionsPb.build());
try {
cluster.start();
} catch (IOException e) {
diff --git a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/TestMiniKuduCluster.java b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/TestMiniKuduCluster.java
index 6844fcdd4..a36f38d0a 100644
--- a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/TestMiniKuduCluster.java
+++ b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/TestMiniKuduCluster.java
@@ -37,6 +37,8 @@ import org.apache.kudu.client.TimeoutTracker;
import org.apache.kudu.test.cluster.FakeDNS;
import org.apache.kudu.test.cluster.MiniKuduCluster;
import org.apache.kudu.test.junit.RetryRule;
+import org.apache.kudu.tools.Tool.CreateClusterRequestPB.JwksOptionsPB;
+import org.apache.kudu.tools.Tool.CreateClusterRequestPB.MiniOidcOptionsPB;
public class TestMiniKuduCluster {
@@ -107,6 +109,21 @@ public class TestMiniKuduCluster {
}
}
+ @Test(timeout = 50000)
+ public void testJwt() throws Exception {
+ try (MiniKuduCluster cluster = new MiniKuduCluster.MiniKuduClusterBuilder()
+ .numMasterServers(NUM_MASTERS)
+ .numTabletServers(0)
+ .enableClientJwt()
+ .addJwks("account-id", true)
+ .build();
+ KuduClient client = new KuduClientBuilder(cluster.getMasterAddressesAsString()).build()) {
+ String jwt = cluster.createJwtFor("account-id", "subject", true);
+
+ assertNotNull(jwt);
+ }
+ }
+
@Test(timeout = 50000)
public void testKerberos() throws Exception {
FakeDNS.getInstance().install();
diff --git a/src/kudu/tools/tool.proto b/src/kudu/tools/tool.proto
index b98345bba..4851d877c 100644
--- a/src/kudu/tools/tool.proto
+++ b/src/kudu/tools/tool.proto
@@ -72,7 +72,7 @@ message CreateClusterRequestPB {
// Options pertaining to a single JWKS.
message JwksOptionsPB {
optional string account_id = 1;
- optional bool is_valid_key = 2;
+ optional bool is_valid_key = 2 [ default = true ];
}
message MiniOidcOptionsPB {
@@ -226,6 +226,7 @@ message ControlShellResponsePB {
GetMastersResponsePB get_masters = 2;
GetTServersResponsePB get_tservers = 3;
GetKDCEnvVarsResponsePB get_kdc_env_vars = 4;
+ CreateJwtResponsePB create_jwt = 5;
}
}
@@ -252,6 +253,7 @@ message ControlShellRequestPB {
SetDaemonFlagRequestPB set_daemon_flag = 12;
PauseDaemonRequestPB pause_daemon = 13;
ResumeDaemonRequestPB resume_daemon = 14;
+ CreateJwtRequestPB create_jwt = 15;
}
}
@@ -551,7 +553,7 @@ message TablesInfoPB {
repeated TableInfoPB tables = 1;
}
-message CreateJwtPB {
+message CreateJwtRequestPB {
// The account ID with which a JWT will be created.
optional string account_id = 1;
@@ -561,3 +563,7 @@ message CreateJwtPB {
// Whether or not the returned token should supply a valid key ID.
optional bool is_valid_key = 3;
}
+
+message CreateJwtResponsePB {
+ optional string jwt = 1;
+}
diff --git a/src/kudu/tools/tool_action_test.cc b/src/kudu/tools/tool_action_test.cc
index 25f6787b0..7a06b3835 100644
--- a/src/kudu/tools/tool_action_test.cc
+++ b/src/kudu/tools/tool_action_test.cc
@@ -43,6 +43,7 @@
#include "kudu/tools/tool.pb.h"
#include "kudu/util/env.h"
#include "kudu/util/env_util.h"
+#include "kudu/util/mini_oidc.h"
#include "kudu/util/path_util.h"
#include "kudu/util/pb_util.h"
#include "kudu/util/status.h"
@@ -179,6 +180,14 @@ Status ProcessRequest(const ControlShellRequestPB& req,
opts.principal = cc.principal();
}
}
+ if (cc.has_mini_oidc_options()) {
+ opts.enable_client_jwt = true;
+ const auto& mini_oidc_opts = cc.mini_oidc_options();
+ for (const auto& jwks_opt : mini_oidc_opts.jwks_options()) {
+ opts.mini_oidc_options.account_ids.emplace(
+ jwks_opt.account_id(), jwks_opt.is_valid_key());
+ }
+ }
cluster->reset(new ExternalMiniCluster(std::move(opts)));
break;
@@ -343,6 +352,21 @@ Status ProcessRequest(const ControlShellRequestPB& req,
RETURN_NOT_OK(daemon->Resume());
break;
}
+ case ControlShellRequestPB::kCreateJwt:
+ {
+ const auto& r = req.create_jwt();
+ if (!r.has_account_id() || !r.has_subject()) {
+ return Status::InvalidArgument("JWT creation requires account ID and subject");
+ }
+ RETURN_NOT_OK(CheckClusterExists(*cluster));
+ auto* oidc = (*cluster)->oidc();
+ if (!oidc) {
+ return Status::ConfigurationError("mini-cluster must be configured with MiniOidc");
+ }
+ auto jwt = oidc->CreateJwt(r.account_id(), r.subject(), r.is_valid_key());
+ *resp->mutable_create_jwt()->mutable_jwt() = jwt;
+ break;
+ }
default:
RETURN_NOT_OK(Status::InvalidArgument("unknown cluster control request"));
}