Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config support for S3 ca_path, ca_file, and verify_ssl options … #1376

Merged
merged 3 commits into from
Sep 5, 2019

Conversation

ihnorton
Copy link
Member

@ihnorton ihnorton commented Sep 3, 2019

@ihnorton ihnorton force-pushed the ihn/aws_conf_ca_verifyssl branch from e511f1d to b8f0624 Compare September 3, 2019 18:51
@ihnorton ihnorton requested a review from tdenniston September 3, 2019 20:02
@ihnorton
Copy link
Member Author

ihnorton commented Sep 3, 2019

Not sure why Travis S3 failed. May be due to https://github.com/travis-ci/travis-ci/issues/8891 (binding of localhost for ip4 vs ip6).

Copy link
Contributor

@tdenniston tdenniston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM mostly -- could you also add the new config params to the https://docs.tiledb.io/en/stable/tutorials/config.html page? We really need to consolidate where these params are listed/documented (but we'll do this in a separate PR at some point).

tiledb/sm/filesystem/s3.cc Outdated Show resolved Hide resolved
tiledb/sm/c_api/tiledb.h Outdated Show resolved Hide resolved
@ihnorton ihnorton force-pushed the ihn/aws_conf_ca_verifyssl branch from b8f0624 to 2220b6e Compare September 3, 2019 21:12
@ihnorton
Copy link
Member Author

ihnorton commented Sep 3, 2019

add the new config params to the https://docs.tiledb.io/en/stable/tutorials/config.html page

done

@ihnorton ihnorton force-pushed the ihn/aws_conf_ca_verifyssl branch from 2220b6e to f33611f Compare September 4, 2019 12:51
@ihnorton ihnorton force-pushed the ihn/aws_conf_ca_verifyssl branch 4 times, most recently from 4b1799b to 85b4e13 Compare September 4, 2019 20:19
tiledb/sm/storage_manager/config.h Outdated Show resolved Hide resolved
tiledb/sm/cpp_api/config.h Outdated Show resolved Hide resolved
tiledb/sm/c_api/tiledb.h Outdated Show resolved Hide resolved
Corresponding to Aws::Client::ClientConfiguration caPath, caFile,
  and verifySSL struct fields.
- create and utilize certificate files as documented in:
  https://docs.min.io/docs/how-to-secure-access-to-minio-server-with-tls

- use ECDSA cert instead of RSA
  ref: github.com/minio/minio/issues/6611

- add test certificates to repository

- adjust tests to use certs (one each for ca_file and ca_path) or to
  pass `verify_ssl = false`.
@ihnorton ihnorton force-pushed the ihn/aws_conf_ca_verifyssl branch from 85b4e13 to 7d23cd6 Compare September 4, 2019 21:17
@ihnorton ihnorton merged commit b72f595 into dev Sep 5, 2019
@ihnorton ihnorton deleted the ihn/aws_conf_ca_verifyssl branch September 5, 2019 13:35
@tdenniston tdenniston added the backport tags commits to backport to release branch label Sep 5, 2019
@tdenniston tdenniston added this to the 1.6.3 milestone Sep 5, 2019
tdenniston pushed a commit that referenced this pull request Sep 30, 2019
Corresponding to Aws::Client::ClientConfiguration caPath, caFile,
  and verifySSL struct fields.

PR #1376

(cherry picked from commit ae32198)
tdenniston pushed a commit that referenced this pull request Sep 30, 2019
- create and utilize certificate files as documented in:
  https://docs.min.io/docs/how-to-secure-access-to-minio-server-with-tls

- use ECDSA cert instead of RSA
  ref: github.com/minio/minio/issues/6611

- add test certificates to repository

- adjust tests to use certs (one each for ca_file and ca_path) or to
  pass `verify_ssl = false`.

PR #1376

(cherry picked from commit aa5be3a)
tdenniston pushed a commit that referenced this pull request Sep 30, 2019
@tdenniston tdenniston removed the backport tags commits to backport to release branch label Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants