Skip to content

Adding basic ssl server testflows suite.#35949

Closed
vzakaznikov wants to merge 6 commits intoClickHouse:masterfrom
vzakaznikov:testflows_ssl_server_tests
Closed

Adding basic ssl server testflows suite.#35949
vzakaznikov wants to merge 6 commits intoClickHouse:masterfrom
vzakaznikov:testflows_ssl_server_tests

Conversation

@vzakaznikov
Copy link
Copy Markdown
Contributor

@vzakaznikov vzakaznikov commented Apr 5, 2022

Changelog category (leave one):

  • Build/Testing/Packaging Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
NA

Descrtiption:

Adding tests/testflows/ssl_server module to demonstrate the following issues:

  1. Adding SSL configuration when server key has a passphrase fails
  2. Dynamic SSL context configuration without server restart fails with or without passphrase present for server key
/tests/testflows/ssl_server$ ./regression.py --local --clickhouse-binary-path docker://clickhouse/clickhouse-server:22.3.2.2-alpine --test-to-end -l test.log -o classic
➤ Apr 04,2022 20:04:31 /ssl server
➤ Apr 04,2022 20:05:03 /ssl server/sanity
➤ Apr 04,2022 20:05:03 /ssl server/sanity/check non secure connection
✔ 46ms      [   OK   ] /ssl server/sanity/check non secure connection
✔ 48ms      [   OK   ] /ssl server/sanity
➤ Apr 04,2022 20:05:03 /ssl server/ssl context
➤ Apr 04,2022 20:05:03 /ssl server/ssl context/enable ssl no server key passphrase
✔ 28s 291ms [   OK   ] /ssl server/ssl context/enable ssl no server key passphrase
➤ Apr 04,2022 20:05:31 /ssl server/ssl context/enable ssl no server key passphrase dynamically
✘ 19s 796ms [  Fail  ] /ssl server/ssl context/enable ssl no server key passphrase dynamically
    ConfigReloader: Error updating configuration
➤ Apr 04,2022 20:05:51 /ssl server/ssl context/enable ssl with server key passphrase
✘ 14s 992ms [  Fail  ] /ssl server/ssl context/enable ssl with server key passphrase
    ConfigReloader: Error updating configuration
➤ Apr 04,2022 20:06:06 /ssl server/ssl context/enable ssl with server key passphrase dynamically
✘ 15s 20ms  [  Fail  ] /ssl server/ssl context/enable ssl with server key passphrase dynamically
    ConfigReloader: Error updating configuration
✘ 1m 18s    [  Fail  ] /ssl server/ssl context
    ConfigReloader: Error updating configuration
✘ 1m 51s    [  Fail  ] /ssl server
    ConfigReloader: Error updating configuration

Passing

✔ [ OK ] /ssl server/sanity/check non secure connection
✔ [ OK ] /ssl server/sanity
✔ [ OK ] /ssl server/ssl context/enable ssl no server key passphrase

Failing

✘ [ Fail ] /ssl server/ssl context/enable ssl no server key passphrase dynamically
✘ [ Fail ] /ssl server/ssl context/enable ssl with server key passphrase
✘ [ Fail ] /ssl server/ssl context/enable ssl with server key passphrase dynamically
✘ [ Fail ] /ssl server/ssl context
✘ [ Fail ] /ssl server

@vzakaznikov
Copy link
Copy Markdown
Contributor Author

vzakaznikov commented Apr 5, 2022

Full test log https://gist.github.com/vzakaznikov/ed57c00a8d616a7f71f805ee62839b1b.

@robot-ch-test-poll robot-ch-test-poll added the pr-build Pull request with build/testing/packaging improvement label Apr 5, 2022
@vzakaznikov vzakaznikov mentioned this pull request Apr 5, 2022
Closed
@vzakaznikov
Copy link
Copy Markdown
Contributor Author

vzakaznikov commented Apr 5, 2022

Tests reproduce issue in #35950.

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Apr 5, 2022

We need to add TestFlows to CI before merging this PR.
Otherwise it is a dead code.

@alexey-milovidov alexey-milovidov marked this pull request as draft April 7, 2022 13:42
@vzakaznikov vzakaznikov marked this pull request as ready for review April 11, 2022 18:43
@vzakaznikov
Copy link
Copy Markdown
Contributor Author

vzakaznikov commented Apr 11, 2022

I will be using this PR to try to enable TestFlows in CI.

@alexey-milovidov alexey-milovidov added the can be tested Allows running workflows for external contributors label Apr 11, 2022
@vzakaznikov vzakaznikov force-pushed the testflows_ssl_server_tests branch from 02f1a71 to 152d10a Compare April 12, 2022 14:17
@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented May 5, 2022

This PR looks a bit stale, let's merge as is and enable the tests in subsequent PR.

@alexey-milovidov alexey-milovidov self-assigned this May 5, 2022
@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented May 5, 2022

@Mergifyio update

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented May 5, 2022 •
edited
Loading

PS. Testflows still look as an overengineered technology that no one will be able to support.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented May 5, 2022

update

✅ Branch has been successfully updated

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented May 5, 2022

@Felixoid said that the "workflow" file is corrupted.

@Felixoid
Copy link
Copy Markdown
Member

Felixoid commented May 5, 2022

https://github.com/ClickHouse/ClickHouse/actions/runs/2161228036

@Felixoid
Copy link
Copy Markdown
Member

Felixoid commented May 6, 2022

Once the workflow file is fixed, the PR can be reopened

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@alexey-milovidov alexey-milovidov

Labels

can be tested Allows running workflows for external contributors pr-build Pull request with build/testing/packaging improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vzakaznikov @alexey-milovidov @Felixoid @filimonov @robot-ch-test-poll