CORE-2452 followup for azure managed identities #17840 #17953
Conversation
return .is_hns_enabled iff error code is 400 and x-ms-error-code == HierarchicalNamespaceNotEnabled
per documetnation, Azure Blob Storage requires the x-ms-date header to contain the timestamp of the request. Surprisingly, not having this header seems to work fine on Azurite and Azure Blob Storage, no errors are appearing.
andijcr
changed the title
|
issue is #17847 |
|
Known failure: #17847 |
| @@ -11,6 +11,7 @@ | |||
| #pragma once | |||
|
|
|||
| #include "cloud_roles/apply_credentials.h" | |||
| #include "cloud_roles/signature.h" | |||
There was a problem hiding this comment.
FYI with the new includes work since signature.h is a private header to this module it needs to be a local includes (not rooted from v. I will followup a PR
|
This PR was merged with both release notes linter failures and code linting failures. cc @piyushredpanda |
|
oh hmm, sorry for the oversight. @andijcr and I both chatted that this is good to merge, but I certainly missed it. |
|
all good. the code linter is the real issue here because it causes all other prs to then begin having linter failures. i think tyler has a fix up now. |
|
I didn't notice that there was a failure at the linter stage, sorry about that |
| if (auto error_code_it = headers.find(error_code_name); | ||
| error_code_it->value() | ||
| == hierarchical_namespace_not_enabled_error_code) { |
There was a problem hiding this comment.
why is it not necessary to check if error_code_it != headers.end()
There was a problem hiding this comment.
embarrassing omission. fix here #17969 , running a manual test on azure
abs_client::self_configure:
Use Set Expiry header
x-ms-error-code: HierarchicalNamespaceNotEnabledfor a stricter check.the current behavior is: HTTP 400 → HNS not enabled
with this PR: HTTP 400 &&
x-ms-error-code: HierarchicalNamespaceNotEnabled-> HNS not enabledOtherwise, bubble up the error and let redpanda stop gracefully.
The escape hatch cloud_storage_azure_hierarchical_namespace_enabled can be used if this header response changes without notice.
apply_abs_oauth_credentials:
add required header
x-ms-datehttps://learn.microsoft.com/en-us/rest/api/storageservices/delete-blob?tabs=microsoft-entra-id#request-headers
ABS requires x-ms-date header, in our codebase is set by credential_applier and apply_abs_oauth_credentials does not set it.
Did not trigger errors in CI or Azure because it seems like it’s not actually required by the endpoints. (with signed requests, it’s part of the signature)
Still, better to follow the documentation.
mixed in: comment fixes and better error message in config_multi_property_validation, as a follow-up for #17840
Fixes CORE-2451
Fixes CORE-2452
Backports Required
Release Notes