JingsongLi commented on code in PR #244: URL: https://github.com/apache/paimon-cpp/pull/244#discussion_r3849458470
########## src/paimon/rest/dlf_auth.h: ########## @@ -0,0 +1,210 @@ +/* + * 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 <chrono> +#include <cstdint> +#include <functional> +#include <map> +#include <memory> +#include <mutex> +#include <optional> +#include <string> +#include <vector> + +#include "paimon/common/utils/http_client.h" +#include "paimon/rest/rest_auth.h" + +namespace paimon { + +/// Access key credentials used to sign DLF REST requests. +class DlfToken { + public: + DlfToken(const std::string& access_key_id, const std::string& access_key_secret, Review Comment: Fixed in c3ad6d4. Removed the default arguments from all newly added DLF auth APIs and updated every call site to pass `std::nullopt`, retry settings, and the clock explicitly. ########## cmake_modules/arrow.diff: ########## @@ -15,6 +15,13 @@ diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/Thi index 8cb3ec83f5..0765df8fa8 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake +@@ -814,5 +814,6 @@ if(DEFINED ENV{ARROW_THRIFT_URL}) + set(THRIFT_SOURCE_URL "$ENV{ARROW_THRIFT_URL}") + else() + set_urls(THRIFT_SOURCE_URL ++ "https://archive.apache.org/dist/thrift/${ARROW_THRIFT_BUILD_VERSION}/thrift-${ARROW_THRIFT_BUILD_VERSION}.tar.gz"; Review Comment: These URL changes fixed the dependency-download failures seen in the first CI run: ORC format 1.0.0 is no longer available from the active Apache mirror selected by the dynamic closer, so the build failed while downloading it. The Apache Archive is the canonical location for that retired release. The Arrow patch only prepends the Archive URL as a fallback for the older Thrift artifact and retains the existing mirror URLs. After bb1f37d, the affected CI builds passed. ########## CMakeLists.txt: ########## @@ -79,6 +80,9 @@ endif() if(PAIMON_ENABLE_S3 OR PAIMON_ENABLE_REST) find_package(CURL REQUIRED) endif() +if(PAIMON_ENABLE_REST) + find_package(OpenSSL REQUIRED) Review Comment: Fixed in c3ad6d4. `find_package` now explicitly requires OpenSSL 1.1.0; the EVP APIs used by the DLF signers are available from that version. ########## src/paimon/rest/dlf_auth.cpp: ########## @@ -0,0 +1,797 @@ +/* + * 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 "paimon/rest/dlf_auth.h" + +#include <openssl/evp.h> + +#include <array> +#include <cctype> +#include <climits> +#include <ctime> +#include <fstream> +#include <iomanip> +#include <limits> +#include <regex> +#include <set> +#include <sstream> +#include <string_view> +#include <thread> +#include <utility> + +#include "fmt/format.h" +#include "paimon/catalog_options.h" +#include "paimon/common/utils/string_utils.h" +#include "paimon/common/utils/url_utils.h" +#include "paimon/common/utils/uuid.h" +#include "paimon/rest/rest_http_client.h" +#include "rapidjson/document.h" + +namespace paimon { + +namespace { + +constexpr int32_t kEcsMetadataRequestTimeoutMillis = 3 * 60 * 1000; + +constexpr int64_t kTokenExpirationSafeTimeMillis = 60 * 60 * 1000; +constexpr size_t kMaxTokenResponseBytes = 1024 * 1024; +constexpr const char kDefaultEcsMetadataUrl[] = + "http://100.100.100.200/latest/meta-data/Ram/security-credentials/";; + +constexpr const char kAuthorizationHeader[] = "Authorization"; +constexpr const char kContentMd5Header[] = "Content-MD5"; +constexpr const char kContentTypeHeader[] = "Content-Type"; +constexpr const char kDlfDateHeader[] = "x-dlf-date"; +constexpr const char kDlfSecurityTokenHeader[] = "x-dlf-security-token"; +constexpr const char kDlfVersionHeader[] = "x-dlf-version"; +constexpr const char kDlfContentSha256Header[] = "x-dlf-content-sha256"; +constexpr const char kUnsignedPayload[] = "UNSIGNED-PAYLOAD"; +constexpr const char kJsonMediaType[] = "application/json"; + +constexpr const char kOpenApiDateHeader[] = "Date"; +constexpr const char kOpenApiAcceptHeader[] = "Accept"; +constexpr const char kOpenApiHostHeader[] = "Host"; +constexpr const char kAcsSignatureMethodHeader[] = "x-acs-signature-method"; +constexpr const char kAcsSignatureNonceHeader[] = "x-acs-signature-nonce"; +constexpr const char kAcsSignatureVersionHeader[] = "x-acs-signature-version"; +constexpr const char kAcsVersionHeader[] = "x-acs-version"; +constexpr const char kAcsSecurityTokenHeader[] = "x-acs-security-token"; + +void TrimWhitespace(std::string* value) { + size_t begin = 0; + while (begin < value->size() && std::isspace(static_cast<unsigned char>((*value)[begin]))) { + ++begin; + } + size_t end = value->size(); + while (end > begin && std::isspace(static_cast<unsigned char>((*value)[end - 1]))) { + --end; + } + *value = value->substr(begin, end - begin); +} + +std::optional<std::string> FindOption(const std::map<std::string, std::string>& options, + const std::string& key) { + auto iter = options.find(key); + if (iter == options.end()) { + return std::nullopt; + } + return iter->second; +} + +Result<std::string> RequiredNonEmptyOption(const std::map<std::string, std::string>& options, + const std::string& key) { + std::optional<std::string> value = FindOption(options, key); + if (!value || value->empty()) { + return Status::Invalid(fmt::format("option '{}' must be configured for DLF auth", key)); + } + return value.value(); +} Review Comment: Fixed in c3ad6d4. DLF option parsing now reuses `OptionsUtils`. I added `GetOptionalValueFromMap` because DLF must distinguish a missing option from an explicitly configured empty string, notably for the ECS role-name semantics, and added focused coverage for present, missing, empty, and invalid values. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
