zjw1111 commented on code in PR #244: URL: https://github.com/apache/paimon-cpp/pull/244#discussion_r3849221823
########## 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: We avoid default values for function parameters (e.g., `= std::nullopt`); instead, explicitly pass all arguments at the call site, including `std::nullopt`. ########## 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: Just curious, why were the URLs in these two patches changed? ########## 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: ```suggestion find_package(OpenSSL 1.1.0 REQUIRED) ``` ########## 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: Could you reuse with `src/paimon/common/utils/options_utils.h`, If the semantics of some of these functions still fall short of the requirements (e.g., RequiredNonEmptyOption), you can extract new utility functions. -- 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]
