CrateDB SSL: Add client programs using sslmode=require and enable testing on CI
#898
Conversation
WalkthroughThis update introduces a new SSL-enabled Docker Compose environment for CrateDB under Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Test Runner
participant CrateDB (SSL Compose)
participant Client Scripts
Developer->>GitHub Actions: Push/PR triggers workflow
GitHub Actions->>Test Runner: Start test job (setup env)
Test Runner->>CrateDB (SSL Compose): docker compose up
Test Runner->>Client Scripts: Run client_crash.sh / client_dbapi.py / etc.
Client Scripts->>CrateDB (SSL Compose): Connect via SSL, run SELECT 42
CrateDB (SSL Compose)-->>Client Scripts: Return query result
Client Scripts-->>Test Runner: Output results
Test Runner->>CrateDB (SSL Compose): docker compose down
Test Runner-->>GitHub Actions: Report test results
Possibly related issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
amotl
force-pushed
the
cratedb-ssl-sslmode
branch
3 times, most recently
from
6b26ac4 to
9c37fb3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
operation/compose/ssl/client_psycopg.py (1)
12-19: Consider adding error handling and removing commented-out code.The script is clear and works as a minimal example. For robustness, consider wrapping the connection and query execution in a try/except block to handle connection or query errors gracefully. Also, remove the commented-out async line unless you plan to provide an async example.
operation/compose/ssl/client_sqlalchemy.py (1)
12-16: Consider adding error handling for robustness.The script is clear and idiomatic for a minimal example. For production or more robust usage, consider adding try/except blocks to handle connection or query errors gracefully.
operation/compose/ssl/README.md (3)
15-17: Specify language for code blocks.For better readability and markdown compliance, specify the language (e.g.,
shell) for all fenced code blocks.Apply this diff for the code blocks:
-```shell +```shellAlso applies to: 30-32, 38-41
21-22: Reduce wordiness in client examples section.Consider simplifying the sentence to:
"Client examples disable certificate validation to allow connections to the server using self-signed certificates."🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Consider a shorter alternative to avoid wordiness.
Context: ...sted to disable certificate validation, in order to satisfy connecting to the server which ...(IN_ORDER_TO_PREMIUM)
46-50: Convert bare URLs to markdown links for style compliance.Replace bare URLs with markdown links for improved readability and markdown compliance.
Example:
- - https://cratedb.com/docs/guide/install/container/ - - https://cratedb.com/docs/guide/install/container/docker.html - - https://cratedb.com/docs/crate/reference/en/latest/admin/ssl.html + - [Install with containers](https://cratedb.com/docs/guide/install/container/) + - [Docker install guide](https://cratedb.com/docs/guide/install/container/docker.html) + - [SSL Reference](https://cratedb.com/docs/crate/reference/en/latest/admin/ssl.html)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
46-46: Bare URL used
null(MD034, no-bare-urls)
47-47: Bare URL used
null(MD034, no-bare-urls)
48-48: Bare URL used
null(MD034, no-bare-urls)
50-50: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/dependabot.yml(1 hunks).github/workflows/operation-compose-ssl.yml(1 hunks)operation/compose/ssl/Makefile(1 hunks)operation/compose/ssl/README.md(1 hunks)operation/compose/ssl/client_crash.sh(1 hunks)operation/compose/ssl/client_dbapi.py(1 hunks)operation/compose/ssl/client_pgsql.sh(1 hunks)operation/compose/ssl/client_psycopg.py(1 hunks)operation/compose/ssl/client_sqlalchemy.py(1 hunks)operation/compose/ssl/compose.yml(2 hunks)operation/docker/ssl/README.md(0 hunks)
💤 Files with no reviewable changes (1)
- operation/docker/ssl/README.md
🧰 Additional context used
🧬 Code Graph Analysis (3)
operation/compose/ssl/client_dbapi.py (2)
operation/compose/ssl/client_psycopg.py (1)
main(12-19)operation/compose/ssl/client_sqlalchemy.py (1)
main(12-16)
operation/compose/ssl/client_psycopg.py (2)
operation/compose/ssl/client_dbapi.py (1)
main(12-20)operation/compose/ssl/client_sqlalchemy.py (1)
main(12-16)
operation/compose/ssl/client_sqlalchemy.py (2)
operation/compose/ssl/client_dbapi.py (1)
main(12-20)operation/compose/ssl/client_psycopg.py (1)
main(12-19)
🪛 LanguageTool
operation/compose/ssl/README.md
[style] ~21-~21: Consider a shorter alternative to avoid wordiness.
Context: ...sted to disable certificate validation, in order to satisfy connecting to the server which ...
(IN_ORDER_TO_PREMIUM)
🪛 markdownlint-cli2 (0.17.2)
operation/compose/ssl/README.md
38-38: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
46-46: Bare URL used
null
(MD034, no-bare-urls)
47-47: Bare URL used
null
(MD034, no-bare-urls)
48-48: Bare URL used
null
(MD034, no-bare-urls)
50-50: Bare URL used
null
(MD034, no-bare-urls)
🪛 actionlint (1.7.4)
.github/workflows/operation-compose-ssl.yml
5-5: character '~' is invalid for branch and tag names. ref name cannot contain spaces, ~, ^, :, [, ?, *. see man git-check-ref-format for more details. note that regular expression is unavailable. note: filter pattern syntax is explained at https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet
(glob)
🔇 Additional comments (6)
.github/dependabot.yml (1)
175-181: Dependabot configuration for SSL compose directory looks good.The new entry for
/operation/compose/sslis correct and follows the existing structure.operation/compose/ssl/client_dbapi.py (1)
1-25: Minimal DB-API SSL client example is clear and correct.The script demonstrates the intended usage and is consistent with the other client examples.
operation/compose/ssl/client_pgsql.sh (1)
1-4: PostgreSQL SSL client example is correct and clear.The script is minimal and demonstrates the intended usage.
operation/compose/ssl/client_crash.sh (1)
1-8: uvx crash SSL client example is correct and clear.The script is minimal and demonstrates the intended usage.
operation/compose/ssl/Makefile (1)
1-20: Makefile for SSL compose integration testing is well-structured and correct.The targets are clear and the test orchestration is robust.
operation/compose/ssl/compose.yml (1)
1-32: Compose file is well-structured for SSL-enabled CrateDB.The configuration is clear, exposes the necessary ports, and uses appropriate health checks for self-signed certificates. Comments are helpful and the service naming is clear.
amotl
force-pushed
the
cratedb-ssl-sslmode
branch
from
9c37fb3 to
66db2de
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
operation/compose/ssl/README.md (2)
21-22: Consider rewording for conciseness.The phrase "in order to satisfy connecting to the server which is using self-signed certificates" can be shortened for clarity. For example:
"Client examples adjusted to disable certificate validation, allowing connections to the server with self-signed certificates."
🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Consider a shorter alternative to avoid wordiness.
Context: ...sted to disable certificate validation, in order to satisfy connecting to the server which ...(IN_ORDER_TO_PREMIUM)
38-41: Add a language specifier to the code block for better syntax highlighting.The code block showing the SSL error message should specify the language (e.g.,
text) for improved readability and to satisfy markdown linting tools.-``` +[text][SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed
certificate in certificate chain (_ssl.c:1010).<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 38-38: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 9c37fb341949398e441a912bba8a3284617ec7f7 and 66db2de9bb3cf7f3dff5122c8d64de543f650360. </details> <details> <summary>📒 Files selected for processing (11)</summary> * `.github/dependabot.yml` (1 hunks) * `.github/workflows/operation-compose-ssl.yml` (1 hunks) * `operation/compose/ssl/Makefile` (1 hunks) * `operation/compose/ssl/README.md` (1 hunks) * `operation/compose/ssl/client_crash.sh` (1 hunks) * `operation/compose/ssl/client_dbapi.py` (1 hunks) * `operation/compose/ssl/client_pgsql.sh` (1 hunks) * `operation/compose/ssl/client_psycopg.py` (1 hunks) * `operation/compose/ssl/client_sqlalchemy.py` (1 hunks) * `operation/compose/ssl/compose.yml` (2 hunks) * `operation/docker/ssl/README.md` (0 hunks) </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * operation/docker/ssl/README.md </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (9)</summary> * .github/dependabot.yml * operation/compose/ssl/client_pgsql.sh * operation/compose/ssl/client_crash.sh * operation/compose/ssl/client_psycopg.py * operation/compose/ssl/client_dbapi.py * operation/compose/ssl/client_sqlalchemy.py * operation/compose/ssl/Makefile * operation/compose/ssl/compose.yml * .github/workflows/operation-compose-ssl.yml </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>operation/compose/ssl/README.md</summary> [style] ~21-~21: Consider a shorter alternative to avoid wordiness. Context: ...sted to disable certificate validation, in order to satisfy connecting to the server which ... (IN_ORDER_TO_PREMIUM) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>operation/compose/ssl/README.md</summary> 38-38: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>operation/compose/ssl/README.md (1)</summary> `1-59`: **Documentation is clear and well-structured.** The README provides thorough instructions, rationale, and references for running CrateDB with SSL in Docker Compose. All sections are logically organized and easy to follow. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [style] ~21-~21: Consider a shorter alternative to avoid wordiness. Context: ...sted to disable certificate validation, in order to satisfy connecting to the server which ... (IN_ORDER_TO_PREMIUM) </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 38-38: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
amotl
force-pushed
the
cratedb-ssl-sslmode
branch
2 times, most recently
from
dd53f2f to
3b79d44
Compare
There was a problem hiding this comment.
Looks good as an improvement on the original PR which I had not seen. I have a comment however which maybe you could address in the README? unless I am missing something this example is not generating fresh self signed certificates but using the ones uploaded in the repo so anyone spinning an environment from this example will be using the same encryption keys.
amotl
force-pushed
the
cratedb-ssl-sslmode
branch
from
3b79d44 to
6dbc3e5
Compare
|
Thanks. I've added a new prominent section to the README:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
operation/compose/ssl/README.md (1)
13-14: Minor language nitpick: Add a comma for clarity.Consider adding a comma after "Caveat" to improve readability:
-## Caveat +## Caveat,Or, if the intention is to keep it as a section heading, no change is needed. If you want to clarify the transition between sections, you could also add a blank line or rephrase the following sentence for smoother flow.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: Possible missing comma found.
Context: ... or?sslmode=verify-full. ## Caveat Note this example is not generating fresh se...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/dependabot.yml(1 hunks).github/workflows/operation-compose-ssl.yml(1 hunks)operation/compose/ssl/Makefile(1 hunks)operation/compose/ssl/README.md(1 hunks)operation/compose/ssl/client_crash.sh(1 hunks)operation/compose/ssl/client_dbapi.py(1 hunks)operation/compose/ssl/client_pgsql.sh(1 hunks)operation/compose/ssl/client_psycopg.py(1 hunks)operation/compose/ssl/client_sqlalchemy.py(1 hunks)operation/compose/ssl/compose.yml(2 hunks)operation/docker/ssl/README.md(0 hunks)
💤 Files with no reviewable changes (1)
- operation/docker/ssl/README.md
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/operation-compose-ssl.yml
🚧 Files skipped from review as they are similar to previous changes (8)
- operation/compose/ssl/client_pgsql.sh
- .github/dependabot.yml
- operation/compose/ssl/client_crash.sh
- operation/compose/ssl/client_sqlalchemy.py
- operation/compose/ssl/client_psycopg.py
- operation/compose/ssl/client_dbapi.py
- operation/compose/ssl/Makefile
- operation/compose/ssl/compose.yml
🧰 Additional context used
🪛 LanguageTool
operation/compose/ssl/README.md
[uncategorized] ~14-~14: Possible missing comma found.
Context: ... or ?sslmode=verify-full. ## Caveat Note this example is not generating fresh se...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (1)
operation/compose/ssl/README.md (1)
1-68: Documentation is clear and comprehensive.The README provides thorough instructions, context, and rationale for the SSL-enabled CrateDB Docker Compose setup. All references and usage notes are accurate and actionable.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: Possible missing comma found.
Context: ... or?sslmode=verify-full. ## Caveat Note this example is not generating fresh se...(AI_HYDRA_LEO_MISSING_COMMA)
About
Provide example client programs using the canonical
?sslmode=requirequery parameter, which means to connect to SSL-based servers, but to turn off certificate validation.References
sslmodesqlalchemy-cratedb#202sslmodeURI parameter cratedb-toolkit#408/cc @WalBeh