foundationdb icon indicating copy to clipboard operation
foundationdb copied to clipboard

Complete the proxy support for backup to S3.

Open RenxuanW opened this issue 3 years ago • 10 comments

After this PR, FDB can backup to S3 (https) via proxy. The connection to proxy cannot be SSL though.

20220725-232951-renxuan-9f1df6cc57c9f273

Code-Reviewer Section

The general guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

  • [ ] The PR has a description, explaining both the problem and the solution.
  • [ ] The description mentions which forms of testing were done and the testing seems reasonable.
  • [ ] Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

  • [ ] This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
  • [ ] There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

RenxuanW avatar Jul 26 '22 00:07 RenxuanW

Doxense CI Report for Windows 10

  • Commit ID: a6abcc1b00907281549aeec408348cc2aa6c6db2
  • Result: :heavy_check_mark: SUCCEEDED
  • Build Logs (available for 30 days)

fdb-windows-ci avatar Jul 26 '22 00:07 fdb-windows-ci

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: a6abcc1b00907281549aeec408348cc2aa6c6db2
  • Duration 1:03:45
  • Result: :white_check_mark: SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

foundationdb-ci avatar Jul 26 '22 02:07 foundationdb-ci

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: a6abcc1b00907281549aeec408348cc2aa6c6db2
  • Duration 4:15:42
  • Result: :x: FAILED
  • Error: Build has timed out.
  • Build Logs (available for 30 days)

foundationdb-ci avatar Jul 26 '22 04:07 foundationdb-ci

What happens when the proxy only support SSL connection from the client? Will an error be raised and visible?

Are you plan to implement the SSL connection to the proxy server as a separate PR?

I was only focusing on our use case, where the proxy supports both SSL and non SSL conection. I don't have a SSL only proxy to test, but there will definitely be some issues. I can make it throw an warning for now. What's your preference?

RenxuanW avatar Jul 26 '22 05:07 RenxuanW

Doxense CI Report for Windows 10

  • Commit ID: 6851b15ed5d62e2b674a52ff751f4d2fd694a4cd
  • Result: :heavy_check_mark: SUCCEEDED
  • Build Logs (available for 30 days)

fdb-windows-ci avatar Jul 26 '22 05:07 fdb-windows-ci

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 6851b15ed5d62e2b674a52ff751f4d2fd694a4cd
  • Duration 1:04:31
  • Result: :white_check_mark: SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

foundationdb-ci avatar Jul 26 '22 06:07 foundationdb-ci

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: 6851b15ed5d62e2b674a52ff751f4d2fd694a4cd
  • Duration 1:43:52
  • Result: :x: FAILED
  • Error: Error while executing command: make -C tests test_ycsb.run. Reason: exit status 2
  • Build Logs (available for 30 days)

foundationdb-ci avatar Jul 26 '22 07:07 foundationdb-ci

What happens when the proxy only support SSL connection from the client? Will an error be raised and visible? Are you plan to implement the SSL connection to the proxy server as a separate PR?

I was only focusing on our use case, where the proxy supports both SSL and non SSL conection. I don't have a SSL only proxy to test, but there will definitely be some issues. I can make it throw an warning for now. What's your preference?

I'd prefer to throw an error for the SSL only proxy. Can you make that change?

jzhou77 avatar Jul 26 '22 18:07 jzhou77

What happens when the proxy only support SSL connection from the client? Will an error be raised and visible? Are you plan to implement the SSL connection to the proxy server as a separate PR?

I was only focusing on our use case, where the proxy supports both SSL and non SSL conection. I don't have a SSL only proxy to test, but there will definitely be some issues. I can make it throw an warning for now. What's your preference?

I'd prefer to throw an error for the SSL only proxy. Can you make that change?

I don't know how to detect if a proxy is SSL only, but I can throw an error if the proxy port is 443. What do you think?

RenxuanW avatar Jul 26 '22 18:07 RenxuanW

What happens when the proxy only support SSL connection from the client? Will an error be raised and visible? Are you plan to implement the SSL connection to the proxy server as a separate PR?

I was only focusing on our use case, where the proxy supports both SSL and non SSL conection. I don't have a SSL only proxy to test, but there will definitely be some issues. I can make it throw an warning for now. What's your preference?

I'd prefer to throw an error for the SSL only proxy. Can you make that change?

I don't know how to detect if a proxy is SSL only, but I can throw an error if the proxy port is 443. What do you think?

Relying on port number is not ideal. It should be something similar to openssl s_client, i.e., verifying if an SSL connection can be established, probably via the handshake protocol of SSL.

jzhou77 avatar Aug 04 '22 17:08 jzhou77

Doxense CI Report for Windows 10

  • Commit ID: 2d78468cc0425466121ad938ea46945415a860ca
  • Result: :heavy_check_mark: SUCCEEDED
  • Build Logs (available for 30 days)

fdb-windows-ci avatar Aug 22 '22 19:08 fdb-windows-ci

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 2d78468cc0425466121ad938ea46945415a860ca
  • Duration 1:06:24
  • Result: :white_check_mark: SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)
  • Build Artifact (available for 30 days)

foundationdb-ci avatar Aug 22 '22 19:08 foundationdb-ci

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: 2d78468cc0425466121ad938ea46945415a860ca
  • Duration 4:17:43
  • Result: :x: FAILED
  • Error: Build has timed out.
  • Build Logs (available for 30 days)
  • Build Artifact (available for 30 days)

foundationdb-ci avatar Aug 22 '22 22:08 foundationdb-ci