Skip to content

Commit

Permalink
Warn if backing storage cannot be accessed (#473)
Browse files Browse the repository at this point in the history
Also, warn if AWS auth is being used and machine identity is disabled
  • Loading branch information
qc00 committed Aug 21, 2023
1 parent c328afa commit a8bd7f6
Show file tree
Hide file tree
Showing 21 changed files with 325 additions and 25 deletions.
62 changes: 56 additions & 6 deletions cpp/arcticdb/storage/azure/azure_storage-inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <google/protobuf/io/zero_copy_stream_impl_lite.h>
#include <folly/gen/Base.h>
#include <arcticdb/async/task_scheduler.hpp>
#include <arcticdb/storage/azure/azure_storage.hpp>
#include <arcticdb/storage/storage_utils.hpp>
#include <arcticdb/storage/object_store_utils.hpp>
Expand Down Expand Up @@ -293,32 +294,81 @@ bool do_key_exists_impl(
}
} //namespace detail

struct CheckAccessibilityTask : async::BaseTask {
// This ref assumes the `AzureStorage` will out live the TaskScheduler
std::shared_ptr<Azure::Storage::Blobs::BlobContainerClient> container_client_;

void operator()() {
container_client_->GetProperties();
}
};

CheckAccessibilityResult AzureStorage::check_accessibility() {
using namespace spdlog::level;
using HttpStatus = Azure::Core::Http::HttpStatusCode;

auto wait_ms = ConfigsMap::instance()->get_int("AzureStorage.CheckContainerMaxWaitMs", 1000);
auto wait_duration = std::chrono::duration_cast<folly::HighResDuration>(std::chrono::milliseconds(wait_ms));
auto fut = async::TaskScheduler::instance()->submit_cpu_task(CheckAccessibilityTask{{}, container_client_});
std::string details;
try {
std::move(fut).get(wait_duration);
return {debug, "Container access check successful"};
} catch (const folly::FutureTimeout&) {
return {info, "Unable to determine bucket accessibility within the allotted time"};
} catch (const Azure::Storage::StorageException& e) {
details = fmt::format("HTTP {}. {} {}.",
static_cast<std::underlying_type_t<HttpStatus>>(e.StatusCode), e.ErrorCode, e.ReasonPhrase);

switch (e.StatusCode) {
case HttpStatus::Unauthorized:
case HttpStatus::Forbidden:
return {warn, "No permission to access the container", std::move(details)};
case HttpStatus::NotFound:
case HttpStatus::Conflict:
return {err, "The container is not found or being deleted", std::move(details)};
case HttpStatus::BadRequest:
if ("InvalidResourceName" == e.ErrorCode) {
return {err, "The container name is invalid", std::move(details)};
}
break;
default:
break;
}
} catch (const std::exception& e) {
details = e.what();
}
return {warn,
"Unexpected error while checking for storage access. Please report an issue with below details:",
std::move(details)};
}

inline void AzureStorage::do_write(Composite<KeySegmentPair>&& kvs) {
detail::do_write_impl(std::move(kvs), root_folder_, container_client_, FlatBucketizer{}, upload_option_, request_timeout_);
detail::do_write_impl(std::move(kvs), root_folder_, *container_client_, FlatBucketizer{}, upload_option_, request_timeout_);
}

inline void AzureStorage::do_update(Composite<KeySegmentPair>&& kvs, UpdateOpts) {
detail::do_update_impl(std::move(kvs), root_folder_, container_client_, FlatBucketizer{}, upload_option_, request_timeout_);
detail::do_update_impl(std::move(kvs), root_folder_, *container_client_, FlatBucketizer{}, upload_option_, request_timeout_);
}

inline void AzureStorage::do_read(Composite<VariantKey>&& ks, const ReadVisitor& visitor, ReadKeyOpts opts) {
detail::do_read_impl(std::move(ks), visitor, root_folder_, container_client_, FlatBucketizer{}, opts, download_option_, request_timeout_);
detail::do_read_impl(std::move(ks), visitor, root_folder_, *container_client_, FlatBucketizer{}, opts, download_option_, request_timeout_);
}

inline void AzureStorage::do_remove(Composite<VariantKey>&& ks, RemoveOpts) {
detail::do_remove_impl(std::move(ks), root_folder_, container_client_, FlatBucketizer{}, request_timeout_);
detail::do_remove_impl(std::move(ks), root_folder_, *container_client_, FlatBucketizer{}, request_timeout_);
}

inline void AzureStorage::do_iterate_type(KeyType key_type, const IterateTypeVisitor& visitor, const std::string &prefix) {
auto prefix_handler = [] (const std::string& prefix, const std::string& key_type_dir, const KeyDescriptor key_descriptor, KeyType) {
return !prefix.empty() ? fmt::format("{}/{}*{}", key_type_dir, key_descriptor, prefix) : key_type_dir;
};

detail::do_iterate_type_impl(key_type, std::move(visitor), root_folder_, container_client_, FlatBucketizer{}, std::move(prefix_handler), prefix);
detail::do_iterate_type_impl(key_type, std::move(visitor), root_folder_, *container_client_, FlatBucketizer{}, std::move(prefix_handler), prefix);
}

inline bool AzureStorage::do_key_exists(const VariantKey& key) {
return detail::do_key_exists_impl(key, root_folder_, container_client_, FlatBucketizer{});
return detail::do_key_exists_impl(key, root_folder_, *container_client_, FlatBucketizer{});
}

} // namespace azure
Expand Down
13 changes: 8 additions & 5 deletions cpp/arcticdb/storage/azure/azure_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ using namespace Azure::Storage;
using namespace Azure::Storage::Blobs;


AzureStorage::AzureStorage(const LibraryPath &library_path, OpenMode mode, const Config &conf) :
AzureStorage::AzureStorage(const LibraryPath &library_path, OpenMode mode, Config&& conf) :
Storage(library_path, mode),
container_client_(BlobContainerClient::CreateFromConnectionString(conf.endpoint(), conf.container_name(), get_client_options(conf))),
container_client_(std::make_shared<BlobContainerClient>(BlobContainerClient::CreateFromConnectionString(
conf.endpoint(), conf.container_name(), get_client_options(conf)))),
root_folder_(object_store_utils::get_root_folder(library_path)),
request_timeout_(conf.request_timeout() == 0 ? 60000 : conf.request_timeout()){
if (conf.ca_cert_path().empty())
Expand All @@ -41,10 +42,12 @@ AzureStorage::AzureStorage(const LibraryPath &library_path, OpenMode mode, const
}

Azure::Storage::Blobs::BlobClientOptions AzureStorage::get_client_options(const Config &conf) {
Azure::Core::Http::CurlTransportOptions curl_transport_options;
curl_transport_options.CAInfo = conf.ca_cert_path();
BlobClientOptions client_options;
client_options.Transport.Transport = std::make_shared<Azure::Core::Http::CurlTransport>(curl_transport_options);
if (!conf.ca_cert_path().empty()) {
Azure::Core::Http::CurlTransportOptions curl_transport_options;
curl_transport_options.CAInfo = conf.ca_cert_path();
client_options.Transport.Transport = std::make_shared<Azure::Core::Http::CurlTransport>(curl_transport_options);
}
return client_options;
}

Expand Down
8 changes: 6 additions & 2 deletions cpp/arcticdb/storage/azure/azure_storage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ class AzureStorage final : public Storage {
// friend class AzureTestClientAccessor<AzureStorage>;
using Config = arcticdb::proto::azure_storage::Config;

AzureStorage(const LibraryPath &lib, OpenMode mode, const Config &conf);
AzureStorage(const LibraryPath &library_path, OpenMode mode, Config&& conf);

/**
* Full object path in Azure bucket.
*/
std::string get_key_path(const VariantKey& key) const;

CheckAccessibilityResult check_accessibility() override;

protected:
void do_write(Composite<KeySegmentPair>&& kvs) final;

Expand All @@ -59,7 +61,9 @@ class AzureStorage final : public Storage {
std::string do_key_path(const VariantKey&) const final { return {}; };

private:
Azure::Storage::Blobs::BlobContainerClient container_client_;
// The check_accessibility() starts an async task internally with a timeout, so the client code could be still
// running this AzureStorage is disposed, so must share:
std::shared_ptr<Azure::Storage::Blobs::BlobContainerClient> container_client_;
std::string root_folder_;
unsigned int request_timeout_;
Azure::Storage::Blobs::UploadBlockBlobFromOptions upload_option_;
Expand Down
7 changes: 6 additions & 1 deletion cpp/arcticdb/storage/library.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ class Library {
Library& operator=(const Library&) = delete;
Library& operator=(Library&&) = delete;

/** Calls check_accessibility() on the primary Storage. */
CheckAccessibilityResult check_accessibility_of_primary_storage() {
return storages_->check_accessibility_of_primary_storage();
}

/**
* Tries to get every key of the given type (and prefix if not empty). Please assume this can skip keys sometimes
* and code defensively.
Expand Down Expand Up @@ -122,7 +127,7 @@ class Library {
return res;
}

/** Calls VariantStorage::do_key_path on the primary storage */
/** Calls Storage::do_key_path on the primary storage */
std::string key_path(const VariantKey& key) const {
return storages_->key_path(key);
}
Expand Down
4 changes: 4 additions & 0 deletions cpp/arcticdb/storage/lmdb/lmdb_storage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ class LmdbStorage final : public Storage {

LmdbStorage(const LibraryPath &lib, OpenMode mode, const Config &conf);

CheckAccessibilityResult check_accessibility() override {
return {spdlog::level::trace, "No storage access check necessary"}; // The constructor throws on invalid path
}

private:
void do_write(Composite<KeySegmentPair>&& kvs) final;

Expand Down
4 changes: 4 additions & 0 deletions cpp/arcticdb/storage/memory/memory_storage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ namespace arcticdb::storage::memory {

MemoryStorage(const LibraryPath &lib, OpenMode mode, const Config &conf);

CheckAccessibilityResult check_accessibility() override {
return {spdlog::level::trace, "No storage access check necessary"};
}

private:
void do_write(Composite<KeySegmentPair>&& kvs) final;

Expand Down
3 changes: 3 additions & 0 deletions cpp/arcticdb/storage/mongo/mongo_storage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class MongoStorage final : public Storage {

MongoStorage(const LibraryPath &lib, OpenMode mode, const Config &conf);

CheckAccessibilityResult check_accessibility() override {
return {spdlog::level::debug, "Storage access check not yet implemented for MongoDB"};
}
private:
void do_write(Composite<KeySegmentPair>&& kvs) final;

Expand Down
13 changes: 13 additions & 0 deletions cpp/arcticdb/storage/python_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,19 @@ void register_bindings(py::module& storage, py::exception<arcticdb::ArcticExcept
return none.cast<py::object>();
});
})
.def("check_accessibility_of_primary_storage", &Library::check_accessibility_of_primary_storage)
;

py::class_<CheckAccessibilityResult, std::shared_ptr<CheckAccessibilityResult>>(storage, "CheckAccessibilityResult")
.def_readonly("log_level", &CheckAccessibilityResult::log_level_)
.def_readonly("user_friendly_description", &CheckAccessibilityResult::user_friendly_description_)
.def_readonly("technical_details", &CheckAccessibilityResult::technical_details_)
.def("__str__", [](CheckAccessibilityResult self) {
return fmt::format("CheckAccessibilityResult({}, {}, {})",
self.log_level_,
self.user_friendly_description_,
self.technical_details_);
})
;

py::class_<S3CredentialsOverride>(storage, "S3CredentialsOverride")
Expand Down
4 changes: 4 additions & 0 deletions cpp/arcticdb/storage/s3/nfs_backed_storage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ class NfsBackedStorage final : public Storage {

NfsBackedStorage(const LibraryPath &lib, OpenMode mode, const Config &conf);

CheckAccessibilityResult check_accessibility() override {
return arcticdb::storage::s3::do_check_accessibility(s3_client_, bucket_name_);
}

private:
void do_write(Composite<KeySegmentPair>&& kvs) final;

Expand Down
1 change: 1 addition & 0 deletions cpp/arcticdb/storage/s3/s3_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ S3ApiInstance::S3ApiInstance(Aws::Utils::Logging::LogLevel log_level) :
return;
ARCTICDB_RUNTIME_DEBUG(log::Loggers::instance()->storage(),
"Does not appear to be using AWS. Will set AWS_EC2_METADATA_DISABLED");
ec2_metadata_disabled_by_us_ = true;
#ifdef WIN32
_putenv_s("AWS_EC2_METADATA_DISABLED", "true");
#else
Expand Down
4 changes: 4 additions & 0 deletions cpp/arcticdb/storage/s3/s3_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@ class S3ApiInstance {
static std::shared_ptr<S3ApiInstance> instance();
static void destroy_instance();

bool is_ec2_metadata_disabled_by_us() const {
return ec2_metadata_disabled_by_us_;
}
private:
Aws::Utils::Logging::LogLevel log_level_;
Aws::SDKOptions options_;
bool ec2_metadata_disabled_by_us_;
};

} //namespace arcticdb::storage::s3
37 changes: 37 additions & 0 deletions cpp/arcticdb/storage/s3/s3_storage-inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <aws/s3/model/DeleteObjectsRequest.h>
#include <aws/s3/model/ListObjectsV2Request.h>
#include <aws/s3/model/HeadObjectRequest.h>
#include <aws/s3/model/HeadBucketRequest.h>
#include <aws/s3/model/Object.h>
#include <aws/s3/model/Delete.h>
#include <aws/s3/model/ObjectIdentifier.h>
Expand All @@ -50,6 +51,42 @@ namespace s3 {

namespace fg = folly::gen;

CheckAccessibilityResult do_check_accessibility(const Aws::S3::S3Client& s3_client, const std::string& bucket_name) {
using namespace Aws::S3;
using namespace spdlog::level;
// We cannot easily change the timeouts of the s3_client_, so use async:
auto future = s3_client.HeadBucketCallable(Model::HeadBucketRequest().WithBucket(bucket_name.c_str()));
auto wait = std::chrono::milliseconds(ConfigsMap::instance()->get_int("S3Storage.CheckBucketMaxWait", 1000));
if (future.wait_for(wait) == std::future_status::ready) {
auto outcome = future.get();
if (!outcome.IsSuccess()) {
auto& error = outcome.GetError();
auto details = fmt::format("HTTP Status: {}. Server response: {}",
int(error.GetResponseCode()), error.GetMessage().c_str());

// HEAD request can't return the error details, so can't use the more detailed error codes.
switch (error.GetResponseCode()) {
case Aws::Http::HttpResponseCode::NOT_FOUND:
return {err, fmt::format("The specified bucket [{}] does not exist", bucket_name), std::move(details)};
case Aws::Http::HttpResponseCode::UNAUTHORIZED:
case Aws::Http::HttpResponseCode::FORBIDDEN:
// This is not an error because AWS's ACL scheme might be able to block HEAD'ing the bucket, but
// allow accessing the blobs
return {warn, "No permission to access the bucket", std::move(details)};
case Aws::Http::HttpResponseCode::REQUEST_NOT_MADE:
return {warn, "Cannot connect to the given server", std::move(details)};
default:
return {warn,
"Unexpected error while checking for storage access. "
"Please report an issue with below details:",
std::move(details)};
}
}
return {debug, "Bucket access check successful"};
} else {
return {info, "Unable to determine bucket accessibility within the allotted time"};
}
}

inline std::string S3Storage::get_key_path(const VariantKey& key) const {
auto b = FlatBucketizer{};
Expand Down
14 changes: 13 additions & 1 deletion cpp/arcticdb/storage/s3/s3_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#define ARCTICDB_S3_STORAGE_H_
#include <arcticdb/storage/s3/s3_storage-inl.hpp>
#include <aws/core/auth/AWSCredentialsProvider.h>
#include <aws/core/platform/Environment.h>
#include <arcticdb/storage/s3/s3_api.hpp>
#include <arcticdb/log/log.hpp>
#include <locale>
Expand All @@ -33,6 +34,16 @@ std::streamsize S3StreamBuffer::xsputn(const char_type* s, std::streamsize n) {
}
}

void check_ec2_metadata_endpoint(const S3ApiInstance& s3_api) {
auto disabled_env = Aws::Environment::GetEnv("AWS_EC2_METADATA_DISABLED");
if (Aws::Utils::StringUtils::ToLower(disabled_env.c_str()) == "true") {
const char* who_disabled = s3_api.is_ec2_metadata_disabled_by_us() ?
"EC2 metadata endpoint did not respond in time so was bypassed":
"AWS_EC2_METADATA_DISABLED environment variable is set to true";
log::storage().warn("{}. This means machine identity authentication will not work.", who_disabled);
}
}

S3Storage::S3Storage(const LibraryPath &library_path, OpenMode mode, const Config &conf) :
Storage(library_path, mode),
s3_api_(S3ApiInstance::instance()),
Expand All @@ -43,6 +54,7 @@ S3Storage::S3Storage(const LibraryPath &library_path, OpenMode mode, const Confi

if (creds.GetAWSAccessKeyId() == USE_AWS_CRED_PROVIDERS_TOKEN && creds.GetAWSSecretKey() == USE_AWS_CRED_PROVIDERS_TOKEN){
ARCTICDB_RUNTIME_DEBUG(log::storage(), "Using AWS auth mechanisms");
check_ec2_metadata_endpoint(*s3_api_);
s3_client_ = Aws::S3::S3Client(get_s3_config(conf), Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never, conf.use_virtual_addressing());
} else {
ARCTICDB_RUNTIME_DEBUG(log::storage(), "Using provided auth credentials");
Expand All @@ -64,4 +76,4 @@ S3Storage::S3Storage(const LibraryPath &library_path, OpenMode mode, const Confi
s3_api_.reset();
}

} // namespace arcticdb::storage::s3
} // namespace arcticdb::storage::s3
Loading

0 comments on commit a8bd7f6

Please sign in to comment.