Re-enable database TLS#2245
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables internal and backend database TLS by upgrading ProxySQL to version 3.0.x and adding the necessary certificate generation tasks. A review comment identifies a hardcoded path in the root CA copy task that should be parameterized with the stackhpc_ca_secret_store variable to support different secret store backends.
b34e018 to
6a816f6
Compare
984f1bb to
217e3a1
Compare
|
@seunghun1ee what's the intention here? This PR has been stuck in draft for a while. Is it waiting for something else to merge? |
|
This is currently actively worked on and dependent on https://review.opendev.org/c/openstack/kolla-ansible/+/986381 |
|
Should we just include this in 2026.1? |
|
Happy Friday @seunghun1ee, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
6 similar comments
|
Happy Friday @seunghun1ee, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
|
Happy Friday @seunghun1ee, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
|
Happy Friday @seunghun1ee, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
|
Happy Friday @seunghun1ee, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
|
Happy Friday @seunghun1ee, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
|
Happy Friday @seunghun1ee, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
etc/kayobe/ansible/secret-store/secret-store-generate-internal-tls.yml (1)
62-62: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNormalize the feature flag in the conditional.
If this value is supplied as the string
"false", Ansible treats the non-empty string as truthy and still generates the ProxySQL private key. Use| bool; this is harmless for actual booleans and prevents false-positive generation. (docs.ansible.com)Proposed fix
- when: kolla_enable_proxysql + when: kolla_enable_proxysql | bool🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@etc/kayobe/ansible/secret-store/secret-store-generate-internal-tls.yml` at line 62, Update the conditional associated with kolla_enable_proxysql to normalize the feature flag through Ansible’s bool filter, preserving key generation for enabled values while treating the string "false" as disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@etc/kayobe/ansible/secret-store/secret-store-generate-internal-tls.yml`:
- Line 62: Update the conditional associated with kolla_enable_proxysql to
normalize the feature flag through Ansible’s bool filter, preserving key
generation for enabled values while treating the string "false" as disabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4bd407f0-b800-4cb6-bc04-139fbed8c001
📒 Files selected for processing (2)
etc/kayobe/ansible/secret-store/secret-store-generate-internal-tls.ymletc/kayobe/kolla/globals.yml
💤 Files with no reviewable changes (1)
- etc/kayobe/kolla/globals.yml
|
@seunghun1ee the bot has raised a good nitpick about slapping |
Slapped |
Kolla-Ansible bumped ProxySQL to 3.0.x and merged [1]. These fix the problem of using database TLS with intermediate certificate. [1] https://review.opendev.org/c/openstack/kolla-ansible/+/994258
2 participants
Upstream Kolla-Ansible 2026.1 brought ProxySQL 3.0.x.
It has a fix for the bug sysown/proxysql#4877 which prevented using TLS between database and ProxySQL.
Re-enable Database TLS and add tasks for creating certificates for ProxySQL.
Note: Two variables
database_enable_tls_internalanddatabase_enable_tls_backendhave default valuetruebut these are still explicitly set totrueto ensure existing systems can also get effect.Summary by CodeRabbit
New Features
Configuration