Skip to content

Commit

Permalink
Snapshot
Browse files Browse the repository at this point in the history
  • Loading branch information
phoebusm committed Mar 11, 2024
1 parent e935c80 commit cd0d937
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 8 deletions.
3 changes: 2 additions & 1 deletion cpp/arcticdb/storage/python_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ void register_bindings(py::module& storage, py::exception<arcticdb::ArcticExcept
"use_virtual_addressing", &S3Override::use_virtual_addressing, &S3Override::set_use_virtual_addressing)
.def_property("ca_cert_path", &S3Override::ca_cert_path, &S3Override::set_ca_cert_path)
.def_property("ca_cert_dir", &S3Override::ca_cert_dir, &S3Override::set_ca_cert_dir)
.def_property("https", &S3Override::https, &S3Override::set_https);
.def_property("https", &S3Override::https, &S3Override::set_https)
.def_property("ssl", &S3Override::ssl, &S3Override::set_ssl);

py::class_<AzureOverride>(storage, "AzureOverride")
.def(py::init<>())
Expand Down
10 changes: 10 additions & 0 deletions cpp/arcticdb/storage/storage_override.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class S3Override {
std::string ca_cert_dir_;
bool use_virtual_addressing_ = false;
bool https_;
bool ssl_;

public:
std::string credential_name() const {
Expand Down Expand Up @@ -93,6 +94,14 @@ class S3Override {
https_ = https;
}

bool ssl() const {
return ssl_;
}

void set_ssl(bool ssl){
ssl_ = ssl;
}

void modify_storage_config(arcticdb::proto::storage::VariantStorage& storage) const {
if(storage.config().Is<arcticdb::proto::s3_storage::Config>()) {
arcticdb::proto::s3_storage::Config s3_storage;
Expand All @@ -107,6 +116,7 @@ class S3Override {
s3_storage.set_ca_cert_path(ca_cert_path_);
s3_storage.set_ca_cert_dir(ca_cert_dir_);
s3_storage.set_https(https_);
s3_storage.set_ssl(ssl_);

util::pack_to_any(s3_storage, *storage.mutable_config());
}
Expand Down
4 changes: 2 additions & 2 deletions python/arcticdb/storage_fixtures/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def __init__(self, factory: "BaseS3StorageFixtureFactory", bucket: str):
if platform.system() is not "Windows":
if factory.client_cert_file:
self.arctic_uri += f"&CA_cert_path={self.factory.client_cert_file}"
# client_cert_dir is skipped on purpose; It will be test manually in other test
# client_cert_dir is skipped on purpose; It will be test manually in other tests
self.arctic_uri += f"&CA_cert_dir="

def __exit__(self, exc_type, exc_value, traceback):
Expand Down Expand Up @@ -285,7 +285,7 @@ def _start_server(self):
server_cert.private_key_pem.write_to_path(self.key_file)
server_cert.cert_chain_pems[0].write_to_path(self.cert_file)
ca.cert_pem.write_to_path(self.client_cert_file)
self.client_cert_dir = "" # client_cert_dir is skipped on purpose; It will be test manually in other test
self.client_cert_dir = self.working_dir
# Create the sym link for curl CURLOPT_CAPATH option; rehash only available on openssl >=1.1.1
subprocess.run(
f'ln -s "{self.client_cert_file}" "$(openssl x509 -hash -noout -in "{self.client_cert_file}")".0',
Expand Down
18 changes: 15 additions & 3 deletions python/tests/integration/arcticdb/test_arctic.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
ArcticInvalidApiUsageException,
)

from tests.util.mark import AZURE_TESTS_MARK, MONGO_TESTS_MARK, REAL_S3_TESTS_MARK, DEDICATED_S3_NON_SSL_TEST_MARK
from tests.util.mark import AZURE_TESTS_MARK, MONGO_TESTS_MARK, REAL_S3_TESTS_MARK, DEDICATED_S3_TEST_MARK_IF_SSL_ON

def test_library_creation_deletion(arctic_client):
ac = arctic_client
Expand Down Expand Up @@ -173,7 +173,7 @@ class ParameterDisplayStatus(Enum):

parameter_display_status = [ParameterDisplayStatus.NOT_SHOW, ParameterDisplayStatus.DISABLE, ParameterDisplayStatus.ENABLE]

@DEDICATED_S3_NON_SSL_TEST_MARK
@DEDICATED_S3_TEST_MARK_IF_SSL_ON
@pytest.mark.parametrize('client_cert_file', parameter_display_status)
@pytest.mark.parametrize('ssl', parameter_display_status)
def test_s3_no_ssl_verification(s3_no_ssl_storage, client_cert_file, ssl):
Expand All @@ -190,7 +190,8 @@ def test_s3_no_ssl_verification(s3_no_ssl_storage, client_cert_file, ssl):
lib = ac.create_library("test")
lib.write("sym", pd.DataFrame())

@DEDICATED_S3_NON_SSL_TEST_MARK

@DEDICATED_S3_TEST_MARK_IF_SSL_ON
def test_s3_ca_directory_ssl_verification(s3_storage):
uri = f"s3s://{s3_storage.factory.host}:{s3_storage.bucket}?access={s3_storage.key.id}&secret={s3_storage.key.secret}&CA_cert_dir={s3_storage.factory.client_cert_dir}&ssl=True"
if s3_storage.factory.port:
Expand All @@ -200,6 +201,16 @@ def test_s3_ca_directory_ssl_verification(s3_storage):
lib.write("sym", pd.DataFrame())


@DEDICATED_S3_TEST_MARK_IF_SSL_ON
def test_s3_https_backend_without_ssl_verification(s3_storage):
uri = f"s3s://{s3_storage.factory.host}:{s3_storage.bucket}?access={s3_storage.key.id}&secret={s3_storage.key.secret}&ssl=False"
if s3_storage.factory.port:
uri += f"&port={s3_storage.factory.port}"
ac = Arctic(uri)
lib = ac.create_library("test")
lib.write("sym", pd.DataFrame())


def test_library_options(arctic_client):
ac = arctic_client
assert ac.list_libraries() == []
Expand Down Expand Up @@ -236,6 +247,7 @@ def test_separation_between_libraries(arctic_client):
# issue #520 then re-implemented in issue #889
"""Validate that symbols in one library are not exposed in another."""
ac = arctic_client
print(ac._uri)
assert ac.list_libraries() == []

ac.create_library("pytest_test_lib")
Expand Down
4 changes: 2 additions & 2 deletions python/tests/util/mark.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@
S3_SSL_TEST_ENABLED = sys.platform == "linux"


DEDICATED_S3_NON_SSL_TEST_MARK = pytest.mark.skipif(
DEDICATED_S3_TEST_MARK_IF_SSL_ON = pytest.mark.skipif(
not S3_SSL_TEST_ENABLED,
reason="S3 tests are non SSL already. No need to have specific non SSL tests",
reason="Additional S3 test only when SSL is ON",
)


Expand Down

0 comments on commit cd0d937

Please sign in to comment.