)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"533c8fbdac8725e7767efa2941245e5fc56ee794","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Michal Arbet \u003cmichal.arbet@ultimum.io\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-09-11 20:01:37 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Do not copy TLS files for backend if turned off"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch fixes an issue where backend related"},{"line_number":10,"context_line":"certificates are attempted to be copied when"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"d3a537cf_3e58478b","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":2},"updated":"2024-09-12 07:58:20.000000000","message":"nit: \"Do not copy TLS files for backend if kolla_enable_tls_backend is turned off\"","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"74b0333070858e3aa79a5c2980b69bb06c0d8ec3","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Michal Arbet \u003cmichal.arbet@ultimum.io\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-09-11 20:01:37 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Do not copy TLS files for backend if turned off"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch fixes an issue where backend related"},{"line_number":10,"context_line":"certificates are attempted to be copied when"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fc5755db_63f8dfe3","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":2},"in_reply_to":"d3a537cf_3e58478b","updated":"2024-10-02 10:18:32.000000000","message":"Grammatically, the sentence \"Do not copy TLS files for backend if turned off\" is mostly clear.\n\nMoreover, this is the header of the commit message, and the details are specified in the body of the commit message. Additionally, the most important thing that the operator will care about is the release note, where it is also very clearly specified. Personally, I do not like putting variable names directly into the header. I consider it unnecessary to change this patch for a \u0027nit\u0027 and lose the +2, which has been so hard to get lately.","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"ed86e6d9fceef4d6c4c4950a0cc63f90aa245a77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"619da254_2a787e2a","updated":"2024-09-11 14:51:14.000000000","message":"I will add RENO and amend commit, just let\u0027s wait for first pass 😊","commit_id":"ab43646ffb11ddd6dc59566e2c8ee38777ea32b7"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"08a5c2639a0a176933558b17ce410cfd874d26af","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1b98091b_1cb07122","updated":"2024-09-11 18:02:43.000000000","message":"Ready.","commit_id":"ab43646ffb11ddd6dc59566e2c8ee38777ea32b7"},{"author":{"_account_id":37013,"name":"Ivan Vnučko","display_name":"Ivan Vnucko","email":"ivan@vnucko.com","username":"ivnucko"},"change_message_id":"f334b6400224abde579f79b56e4c1568724e7271","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e4c311c1_00171fa2","updated":"2024-09-12 12:43:34.000000000","message":"I would propose to make the conditional service dependent - `{{ service }}_enable_tls_backend`, but there are some CRs open for additional \"backend\" TLS of services like RabbitMQ management UI, or MariaDB replication traffic which do not adhere to this naming convention, e.g. MariaDB would have multiple backend TLS flags. \n\nAnother option would be to implement service-cert-copy as lenient (skip\u003dtrue) and implement prechecks if we want the deployment to fail when certificates aren\u0027t present.","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":37013,"name":"Ivan Vnučko","display_name":"Ivan Vnucko","email":"ivan@vnucko.com","username":"ivnucko"},"change_message_id":"f6e83994c510f2c058e73687d33d1ef249b62f6d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8b645a6d_b13c8f4b","updated":"2024-09-12 12:20:03.000000000","message":"There is the option to enable backend TLS per service - e.g. `keystone_enable_tls_backend` - in which case this wouldn\u0027t work as certificate files wouldn\u0027t be copied even for keystone which needs them.","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"533c8fbdac8725e7767efa2941245e5fc56ee794","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d3cc80b2_ee80a2b3","updated":"2024-09-12 07:58:20.000000000","message":"trivial","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":37013,"name":"Ivan Vnučko","display_name":"Ivan Vnucko","email":"ivan@vnucko.com","username":"ivnucko"},"change_message_id":"e1de4a9119d5c62e9806bf62f2911b5c1756a534","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7f2a294a_0b5abe37","in_reply_to":"2167d216_af250d97","updated":"2024-09-28 10:54:22.000000000","message":"I do agree with the principle to keep it simpler. \n\nI see the per-service flags to be potentially useful to turn off TLS for specific service if technical issues ensue with it. In that case this fix would be ok.\n\nMaybe the solution here would be just to update the documentation chapter on backend TLS to present the per-service flags as a way to turn it off for specific service and not the other way around as is the case explicitly now.","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"8ca639e1b982b1934e3cd97154d82a53c6b61947","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b7bdc833_9327bf6c","in_reply_to":"7f2a294a_0b5abe37","updated":"2024-09-29 11:13:19.000000000","message":"Yes, why not. But this should be discussed in the review where that implementation will be tracked. The fact is that the currently merged code does not work, and this patch fixes it. Any improvement to this role is, of course, welcome, but I think only after this is merged – moreover, this is blocking the fix for Let\u0027s Encrypt.\n\nMarking discussion as done - please propose what you were talking about and rebase onto https://review.opendev.org/c/openstack/kolla-ansible/+/925971","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"74b0333070858e3aa79a5c2980b69bb06c0d8ec3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"62ac840d_ce506405","in_reply_to":"8b645a6d_b13c8f4b","updated":"2024-10-02 10:18:32.000000000","message":"This is half true because documentation for enabling TLS on the backend exists here: https://docs.openstack.org/kolla-ansible/latest/admin/tls.html. It is clearly specified that if you want to enable TLS on the backend, you must enable the option kolla_enable_tls_backend.\n\nThe code is designed this way – meaning that service roles inherit from the parent kolla_enable_tls_backend.\n\n```\nansible/group_vars/all.yml:etcd_enable_tls: \"{{ kolla_enable_tls_backend }}\"\nansible/group_vars/all.yml:glance_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/group_vars/all.yml:neutron_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/group_vars/all.yml:horizon_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/barbican/defaults/main.yml:barbican_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/cinder/defaults/main.yml:cinder_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/heat/defaults/main.yml:heat_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/ironic/defaults/main.yml:ironic_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/keystone/defaults/main.yml:keystone_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/nova/defaults/main.yml:nova_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/octavia/defaults/main.yml:octavia_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/placement/defaults/main.yml:placement_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/skyline/defaults/main.yml:skyline_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\nansible/roles/trove/defaults/main.yml:trove_enable_tls_backend: \"{{ kolla_enable_tls_backend }}\"\n```\n\nThis means that if you want to, for example, run TLS backend only for Keystone, you enable kolla_tls_backend: true and disable all others.\n\nI think there’s no need to complicate things. Marking this as resolved. Please open a new comment if you know there might be an issue somewhere.","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"6cb832083f8743d4e4650b698188680bede88bf4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2167d216_af250d97","in_reply_to":"9563155d_44600d04","updated":"2024-09-28 09:22:50.000000000","message":"I get a feeling that we are on a trajectory to overcomplicate the TLS setup.\n\nI\u0027m not sure it\u0027s worth it to configure per service different TLS settings(on|off).\n\nI can\u0027t imagine a scenario where I want, I don\u0027t know encrypted cinder but non encrypted keystone?\n\nhas someone a usecase for that (not: \"It\u0027s theoretically possible to do this\" but: \"I really need this because $foo in my deployment doesn\u0027t work without it\")?","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"83b166d9a5a5d5b56b30bc3081568a252795a3fe","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9563155d_44600d04","in_reply_to":"e4c311c1_00171fa2","updated":"2024-09-27 07:42:50.000000000","message":"Why not, but can we firstly merge this and then we can work on it as followup ? Because it\u0027s just not working now with current code.","commit_id":"924b2aa041b6c83b5cbb6a601ad9b4556ac68155"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"27148a67c8d57dde550a887781830084cda5fe07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f94bb6c7_5233110d","updated":"2024-10-02 14:27:19.000000000","message":"recheck 504 timeout on download collections","commit_id":"1540618f00bc3742a4befb9547a740f44a6bda95"}]}