)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7130,"name":"David Hill","email":"davidchill@hotmail.com","username":"dhill"},"change_message_id":"f5296d7e8e0a9ed3b1da198f937f80950f49244e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"27a411a7_5702a86b","updated":"2022-02-22 21:44:28.000000000","message":"recheck","commit_id":"9cb551201b2e0c68ad0868c9f1f524c51226ba4c"}],"deployment/keystone/keystone-container-puppet.yaml":[{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"714d94e5f9f1e9a6575ec56e46280ace0a4b7aaf","unresolved":true,"context_lines":[{"line_number":945,"context_line":"      host_prep_tasks:"},{"line_number":946,"context_line":"        list_concat:"},{"line_number":947,"context_line":"          - {get_attr: [KeystoneLogging, host_prep_tasks]}"},{"line_number":948,"context_line":"          - - name: Locating all files in /etc/openldap/certs"},{"line_number":949,"context_line":"              find:"},{"line_number":950,"context_line":"                path: \"/etc/openldap/certs\""},{"line_number":951,"context_line":"              register: cleanup"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"f5d0ed1f_7cea7912","line":948,"updated":"2022-02-03 11:00:48.000000000","message":"this doesn\u0027t look idempotent. Was the expectation to purge all certs for each deployment command run?","commit_id":"b99aad9e14b39665819a6818cf2d3bd509e2a51f"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"8b675a2f65bc85fb9edd01ad343d4a2eb3767b53","unresolved":true,"context_lines":[{"line_number":945,"context_line":"      host_prep_tasks:"},{"line_number":946,"context_line":"        list_concat:"},{"line_number":947,"context_line":"          - {get_attr: [KeystoneLogging, host_prep_tasks]}"},{"line_number":948,"context_line":"          - - name: Locating all files in /etc/openldap/certs"},{"line_number":949,"context_line":"              find:"},{"line_number":950,"context_line":"                path: \"/etc/openldap/certs\""},{"line_number":951,"context_line":"              register: cleanup"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"2d3ffad2_bf37b35b","line":948,"in_reply_to":"3fdb4690_01ae9d6e","updated":"2022-02-15 21:19:58.000000000","message":"failure seems like the best scenario to me. and when we fail, the message should include how to proceed (manual cleanup).","commit_id":"b99aad9e14b39665819a6818cf2d3bd509e2a51f"},{"author":{"_account_id":7130,"name":"David Hill","email":"davidchill@hotmail.com","username":"dhill"},"change_message_id":"a0dcaec32846e60581c5ced2bd5e8465a7b340ca","unresolved":true,"context_lines":[{"line_number":945,"context_line":"      host_prep_tasks:"},{"line_number":946,"context_line":"        list_concat:"},{"line_number":947,"context_line":"          - {get_attr: [KeystoneLogging, host_prep_tasks]}"},{"line_number":948,"context_line":"          - - name: Locating all files in /etc/openldap/certs"},{"line_number":949,"context_line":"              find:"},{"line_number":950,"context_line":"                path: \"/etc/openldap/certs\""},{"line_number":951,"context_line":"              register: cleanup"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"3fdb4690_01ae9d6e","line":948,"in_reply_to":"5d4d7f3d_843c0763","updated":"2022-02-15 12:07:31.000000000","message":"Well, that really depends how it\u0027s configured ... is it only an empty DB that was created or was a TLS certificate manually configured in opendlap.conf ?\n\nIE:\n\nTLSCACertificateFile /etc/openldap/certs/ldapserver-cacerts.pem\nTLSCertificateFile /etc/openldap/certs/ldapserver-cert.pem\nTLSCertificateKeyFile /etc/openldap/certs/ldapserver-key.pem\n\nor just /etc/openldap/certs/*.db ... which case should we validate ?  Both ?  Only the latter ?","commit_id":"b99aad9e14b39665819a6818cf2d3bd509e2a51f"},{"author":{"_account_id":7130,"name":"David Hill","email":"davidchill@hotmail.com","username":"dhill"},"change_message_id":"32e2252d455b81850b816f77fe3b6104d669b11b","unresolved":true,"context_lines":[{"line_number":945,"context_line":"      host_prep_tasks:"},{"line_number":946,"context_line":"        list_concat:"},{"line_number":947,"context_line":"          - {get_attr: [KeystoneLogging, host_prep_tasks]}"},{"line_number":948,"context_line":"          - - name: Locating all files in /etc/openldap/certs"},{"line_number":949,"context_line":"              find:"},{"line_number":950,"context_line":"                path: \"/etc/openldap/certs\""},{"line_number":951,"context_line":"              register: cleanup"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"8dbf8b63_c72db915","line":948,"in_reply_to":"6b8eba21_931a0f8e","updated":"2022-02-09 21:01:26.000000000","message":"We had an issue that took a few hours/days to fix during a FFU because there was a database in /etc/openldab/certs that was created by the operator or something like that.   It did override the /etc/pki/tls/ certs and broke openldap certificate validations ...  we can run it once, or every time we do an overcloud operation just to make sure someone didn\u0027t add it .   It would fall in the same scope as the libvirtd process being started and conflicting with ironic dnsmasq process, there shouldn\u0027t be certificates in that folder .   So we can move it elsewhere, leave it here or abandon this patch if you want.  I\u0027ve seen this issue so far only once but it did take us a bit of time to figure out what was happening and this would\u0027ve avoided that situation.","commit_id":"b99aad9e14b39665819a6818cf2d3bd509e2a51f"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"a85c1e07146f0a201fded936d67e3bd92a16caf6","unresolved":true,"context_lines":[{"line_number":945,"context_line":"      host_prep_tasks:"},{"line_number":946,"context_line":"        list_concat:"},{"line_number":947,"context_line":"          - {get_attr: [KeystoneLogging, host_prep_tasks]}"},{"line_number":948,"context_line":"          - - name: Locating all files in /etc/openldap/certs"},{"line_number":949,"context_line":"              find:"},{"line_number":950,"context_line":"                path: \"/etc/openldap/certs\""},{"line_number":951,"context_line":"              register: cleanup"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"c72b96ea_1e1c6d7c","line":948,"in_reply_to":"8dbf8b63_c72db915","updated":"2022-02-10 17:52:25.000000000","message":"So the operator broke their environment, and this task is meant to prevent that in the future. That\u0027s important context, and explains why we would just delete a directory on every deployment.\n\nI also think it\u0027s likely that in the case where the operator intentionally configured something, and then we go out of our way to delete it, could be very bad user experience. Even in this case, could we delete critical secret data (cert db)? Even if it\u0027s the right technical fix so we can proceed, that doesn\u0027t sound like good UX.\n\nThis feels more like a validation, or perhaps we fail if there are files in this directory instead of deleting them.","commit_id":"b99aad9e14b39665819a6818cf2d3bd509e2a51f"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"7d433e158ca294a1e60b25ef39fa88b7f7a28e1d","unresolved":true,"context_lines":[{"line_number":945,"context_line":"      host_prep_tasks:"},{"line_number":946,"context_line":"        list_concat:"},{"line_number":947,"context_line":"          - {get_attr: [KeystoneLogging, host_prep_tasks]}"},{"line_number":948,"context_line":"          - - name: Locating all files in /etc/openldap/certs"},{"line_number":949,"context_line":"              find:"},{"line_number":950,"context_line":"                path: \"/etc/openldap/certs\""},{"line_number":951,"context_line":"              register: cleanup"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"5d4d7f3d_843c0763","line":948,"in_reply_to":"927afc5d_bd89660c","updated":"2022-02-15 10:53:53.000000000","message":"We could check if the cert database is empty, and delete it only in that case.","commit_id":"b99aad9e14b39665819a6818cf2d3bd509e2a51f"},{"author":{"_account_id":7130,"name":"David Hill","email":"davidchill@hotmail.com","username":"dhill"},"change_message_id":"c83bc93f2ce26152e887e0a84397d90acf7f3628","unresolved":true,"context_lines":[{"line_number":945,"context_line":"      host_prep_tasks:"},{"line_number":946,"context_line":"        list_concat:"},{"line_number":947,"context_line":"          - {get_attr: [KeystoneLogging, host_prep_tasks]}"},{"line_number":948,"context_line":"          - - name: Locating all files in /etc/openldap/certs"},{"line_number":949,"context_line":"              find:"},{"line_number":950,"context_line":"                path: \"/etc/openldap/certs\""},{"line_number":951,"context_line":"              register: cleanup"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"927afc5d_bd89660c","line":948,"in_reply_to":"c72b96ea_1e1c6d7c","updated":"2022-02-10 17:57:48.000000000","message":"Well if we just fail, it\u0027ll still be a problem because the operator will be stuck there because of the certificates that shouldn\u0027t be there in a first place and if they put certificates there it should be done through tht and not any other ways ... imo.   I feel better with the deletion than the failure here tbh.","commit_id":"b99aad9e14b39665819a6818cf2d3bd509e2a51f"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"61fb2bae792b880f86dac026b6f527e11f294d59","unresolved":true,"context_lines":[{"line_number":945,"context_line":"      host_prep_tasks:"},{"line_number":946,"context_line":"        list_concat:"},{"line_number":947,"context_line":"          - {get_attr: [KeystoneLogging, host_prep_tasks]}"},{"line_number":948,"context_line":"          - - name: Locating all files in /etc/openldap/certs"},{"line_number":949,"context_line":"              find:"},{"line_number":950,"context_line":"                path: \"/etc/openldap/certs\""},{"line_number":951,"context_line":"              register: cleanup"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"6b8eba21_931a0f8e","line":948,"in_reply_to":"f5d0ed1f_7cea7912","updated":"2022-02-09 18:57:03.000000000","message":"well, it\u0027s certainly idempotent. It\u0027s going to rm /etc/openldap/certs/* every time. but I definitely agree that the intent seems wrong. why would we need to delete these files every time?","commit_id":"b99aad9e14b39665819a6818cf2d3bd509e2a51f"}]}