github-actions[bot] commented on code in PR #68117:
URL: https://github.com/apache/doris/pull/68117#discussion_r4036980720
##########
be/src/util/s3_util.cpp:
##########
@@ -165,8 +234,370 @@ constexpr char S3_NEED_OVERRIDE_ENDPOINT[] =
"AWS_NEED_OVERRIDE_ENDPOINT";
constexpr char S3_ROLE_ARN[] = "AWS_ROLE_ARN";
constexpr char S3_EXTERNAL_ID[] = "AWS_EXTERNAL_ID";
constexpr char S3_CREDENTIALS_PROVIDER_TYPE[] =
"AWS_CREDENTIALS_PROVIDER_TYPE";
+
+// Native Azure binding keys. The AWS_* aliases above remain accepted for
+// existing object-storage callers, but Azure scans use these provider-owned
+// names so their meaning does not depend on the S3 adapter.
+constexpr char AZURE_AUTH_TYPE[] = "AZURE_AUTH_TYPE";
+constexpr char AZURE_ENDPOINT[] = "AZURE_ENDPOINT";
+constexpr char AZURE_ACCOUNT_NAME[] = "AZURE_ACCOUNT_NAME";
+constexpr char AZURE_ACCOUNT_KEY[] = "AZURE_ACCOUNT_KEY";
+constexpr char AZURE_CONTAINER[] = "AZURE_CONTAINER";
+constexpr char AZURE_SAS_TOKEN[] = "AZURE_SAS_TOKEN";
+constexpr char AZURE_SAS_EXPIRY_MS[] = "AZURE_SAS_EXPIRY_MS";
+constexpr char AZURE_CLIENT_ID[] = "AZURE_CLIENT_ID";
+constexpr char AZURE_CLIENT_SECRET[] = "AZURE_CLIENT_SECRET";
+constexpr char AZURE_TENANT_ID[] = "AZURE_TENANT_ID";
+constexpr char AZURE_OAUTH_SERVER_URI[] = "AZURE_OAUTH_SERVER_URI";
+
+const std::string* find_property(const StringCaseMap<std::string>& properties,
+ std::initializer_list<const char*> names) {
+ for (const auto* name : names) {
+ auto it = properties.find(name);
+ if (it != properties.end()) {
+ return &it->second;
+ }
+ }
+ return nullptr;
+}
+
+bool has_property(const StringCaseMap<std::string>& properties,
+ std::initializer_list<const char*> names) {
+ return find_property(properties, names) != nullptr;
+}
+
+std::string normalize_azure_endpoint(std::string endpoint) {
+ if (endpoint.empty()) {
+ return endpoint;
+ }
+ const bool has_scheme = endpoint.find("://") != std::string::npos;
+ if (!has_scheme) {
+ endpoint = "https://"; + endpoint;
+ }
+ const auto scheme_end = endpoint.find("://");
+ endpoint.replace(0, scheme_end, to_lower(endpoint.substr(0, scheme_end)));
+ const auto authority_begin = scheme_end == std::string::npos ? 0 :
scheme_end + 3;
+ const auto authority_end = endpoint.find('/', authority_begin);
+ const auto authority_length = authority_end == std::string::npos
+ ? endpoint.size() - authority_begin
+ : authority_end - authority_begin;
+ const auto authority = endpoint.substr(authority_begin, authority_length);
+ if (authority.empty()) {
+ return endpoint;
+ }
+
+ auto lower_authority = to_lower(authority);
+ endpoint.replace(authority_begin, authority_length, lower_authority);
+ // Match the host, not host:port, so explicit transport ports do not
disable
+ // the official DFS-to-Blob conversion. Custom proxy hosts stay unchanged.
+ const auto host = lower_authority.substr(0, lower_authority.find(':'));
+ const auto dfs_pos = host.find(".dfs.");
+ const bool official_dfs =
+ dfs_pos != std::string::npos &&
(host.ends_with(".dfs.core.windows.net") ||
+
host.ends_with(".dfs.core.chinacloudapi.cn") ||
+
host.ends_with(".dfs.core.usgovcloudapi.net") ||
+
host.ends_with(".dfs.core.cloudapi.de"));
+ if (official_dfs) {
+ endpoint.replace(authority_begin + dfs_pos, 5, ".blob.");
+ } else if (!has_scheme && authority.find('.') == std::string::npos &&
Review Comment:
[P1] Preserve single-label hosts for legacy Azure endpoints
This shorthand rule also runs after `convert_legacy_azure_properties()` and
both persisted storage-vault adapters. An existing legacy config with
`AWS_ENDPOINT=storage-proxy` used to create `https://storage-proxy/container`;
after this change it targets
`https://storage-proxy.blob.core.windows.net/container`. The legacy endpoint
contract does not require a scheme or dotted host, so internal
proxies/emulators using one-label DNS names stop working on upgrade (and
requests are redirected to an unrelated public origin). Please keep
account-name suffix inference native-only, or otherwise distinguish legacy
literal endpoints, and cover the property/PB/Thrift paths with an unschemed
single-label endpoint.
##########
be/src/util/s3_util.cpp:
##########
@@ -165,8 +234,370 @@ constexpr char S3_NEED_OVERRIDE_ENDPOINT[] =
"AWS_NEED_OVERRIDE_ENDPOINT";
constexpr char S3_ROLE_ARN[] = "AWS_ROLE_ARN";
constexpr char S3_EXTERNAL_ID[] = "AWS_EXTERNAL_ID";
constexpr char S3_CREDENTIALS_PROVIDER_TYPE[] =
"AWS_CREDENTIALS_PROVIDER_TYPE";
+
+// Native Azure binding keys. The AWS_* aliases above remain accepted for
+// existing object-storage callers, but Azure scans use these provider-owned
+// names so their meaning does not depend on the S3 adapter.
+constexpr char AZURE_AUTH_TYPE[] = "AZURE_AUTH_TYPE";
+constexpr char AZURE_ENDPOINT[] = "AZURE_ENDPOINT";
+constexpr char AZURE_ACCOUNT_NAME[] = "AZURE_ACCOUNT_NAME";
+constexpr char AZURE_ACCOUNT_KEY[] = "AZURE_ACCOUNT_KEY";
+constexpr char AZURE_CONTAINER[] = "AZURE_CONTAINER";
+constexpr char AZURE_SAS_TOKEN[] = "AZURE_SAS_TOKEN";
+constexpr char AZURE_SAS_EXPIRY_MS[] = "AZURE_SAS_EXPIRY_MS";
+constexpr char AZURE_CLIENT_ID[] = "AZURE_CLIENT_ID";
+constexpr char AZURE_CLIENT_SECRET[] = "AZURE_CLIENT_SECRET";
+constexpr char AZURE_TENANT_ID[] = "AZURE_TENANT_ID";
+constexpr char AZURE_OAUTH_SERVER_URI[] = "AZURE_OAUTH_SERVER_URI";
+
+const std::string* find_property(const StringCaseMap<std::string>& properties,
+ std::initializer_list<const char*> names) {
+ for (const auto* name : names) {
+ auto it = properties.find(name);
+ if (it != properties.end()) {
+ return &it->second;
+ }
+ }
+ return nullptr;
+}
+
+bool has_property(const StringCaseMap<std::string>& properties,
+ std::initializer_list<const char*> names) {
+ return find_property(properties, names) != nullptr;
+}
+
+std::string normalize_azure_endpoint(std::string endpoint) {
+ if (endpoint.empty()) {
+ return endpoint;
+ }
+ const bool has_scheme = endpoint.find("://") != std::string::npos;
+ if (!has_scheme) {
+ endpoint = "https://"; + endpoint;
+ }
+ const auto scheme_end = endpoint.find("://");
+ endpoint.replace(0, scheme_end, to_lower(endpoint.substr(0, scheme_end)));
+ const auto authority_begin = scheme_end == std::string::npos ? 0 :
scheme_end + 3;
+ const auto authority_end = endpoint.find('/', authority_begin);
+ const auto authority_length = authority_end == std::string::npos
+ ? endpoint.size() - authority_begin
+ : authority_end - authority_begin;
+ const auto authority = endpoint.substr(authority_begin, authority_length);
+ if (authority.empty()) {
+ return endpoint;
+ }
+
+ auto lower_authority = to_lower(authority);
+ endpoint.replace(authority_begin, authority_length, lower_authority);
+ // Match the host, not host:port, so explicit transport ports do not
disable
+ // the official DFS-to-Blob conversion. Custom proxy hosts stay unchanged.
+ const auto host = lower_authority.substr(0, lower_authority.find(':'));
+ const auto dfs_pos = host.find(".dfs.");
+ const bool official_dfs =
+ dfs_pos != std::string::npos &&
(host.ends_with(".dfs.core.windows.net") ||
+
host.ends_with(".dfs.core.chinacloudapi.cn") ||
+
host.ends_with(".dfs.core.usgovcloudapi.net") ||
+
host.ends_with(".dfs.core.cloudapi.de"));
+ if (official_dfs) {
+ endpoint.replace(authority_begin + dfs_pos, 5, ".blob.");
+ } else if (!has_scheme && authority.find('.') == std::string::npos &&
+ authority.find(':') == std::string::npos) {
+ endpoint.insert(authority_begin + authority.size(),
".blob.core.windows.net");
+ }
+ while (endpoint.ends_with('/')) {
+ endpoint.pop_back();
+ }
+ return endpoint;
+}
+
+std::string endpoint_authority(const std::string& endpoint) {
+ const auto begin = endpoint.find("://") + 3;
+ auto authority = endpoint.substr(begin, endpoint.find('/', begin) - begin);
+ if (endpoint.starts_with("https://";) && authority.ends_with(":443")) {
+ authority.resize(authority.size() - 4);
+ } else if (endpoint.starts_with("http://";) && authority.ends_with(":80")) {
+ authority.resize(authority.size() - 3);
+ }
+ return authority;
+}
+
+// Only established SharedKey wire producers use AWS fields for Azure. Once
+// translated here the native factory never inspects these fields again.
+void import_legacy_azure_shared_key(S3ClientConf* conf) {
+ // Keep the established SharedKey endpoint normalization at the legacy
+ // boundary. Native endpoints and object keys retain their internal
slashes.
+ conf->endpoint = normalize_http_uri(conf->endpoint);
Review Comment:
[P2] Keep legacy custom endpoint paths byte-preserving
Calling `normalize_http_uri()` here changes existing legacy endpoints, not
just their credential representation. For example, current FE maps preserve
`https://proxy.example/gateway//tenant`; the old factory passed
`https://proxy.example/gateway//tenant/container` to the SDK, while this import
rewrites it to `/gateway/tenant/container`. Repeated or trailing separators can
be meaningful reverse-proxy routes, so those persisted endpoints silently
target a different path after upgrade. Please normalize only the
endpoint/container join boundary (or limit full normalization to the native
protocol) and add property/PB/Thrift coverage for an exact custom base path.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
