)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"00a1318979f0641f8790887b2ed73d609a2b3975","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"02be5dec_b98a9e68","updated":"2022-11-07 18:04:39.000000000","message":"Overall the change looks pretty clean. Have asked a few questions so I can better understand some of implementation decisions.","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":29543,"name":"Scott Solkhon","email":"scott.solkhon@gresearch.co.uk","username":"scott.solkhon"},"change_message_id":"b0795c76e3405e5cf105d46b46f1b24f525cf59d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ec1e0d4e_aeb6470d","updated":"2022-09-20 12:13:58.000000000","message":"bump","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":29543,"name":"Scott Solkhon","email":"scott.solkhon@gresearch.co.uk","username":"scott.solkhon"},"change_message_id":"2da2389bf36f02f77b6135cdbf21e8ca7be9e87a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7b8470c0_bdfb94c2","in_reply_to":"02be5dec_b98a9e68","updated":"2022-11-09 12:59:27.000000000","message":"Ack","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"1d0524e79e5ff0a8c2f190bf726d7ff54b647b6e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3fcca6db_e7570395","updated":"2022-12-09 14:08:25.000000000","message":"LGTM - needs a rebase though to fix the merge conflict","commit_id":"b5c1151a6ca4f9020690fbcd038e3ad6dbc95c5b"},{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"1f538266d4b5e06ccbc7dff4de8ba09f147cb2f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3dc6776a_cab3d8a1","updated":"2023-01-11 14:16:43.000000000","message":"LGTM, need test it","commit_id":"08bd6815bdf58e384b29c38d4a256439a36f3571"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"10cbca964560041a37f31ccf31fa73b01f47d3b8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3e6f5162_c4d59425","updated":"2023-01-23 17:40:17.000000000","message":"recheck (rocky9 job networking issue?)","commit_id":"08bd6815bdf58e384b29c38d4a256439a36f3571"}],"ansible/group_vars/all/kolla":[{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"23bddd9912ab1b23c7cfe3688fc1568e1c9d41a8","unresolved":true,"context_lines":[{"line_number":472,"context_line":"kolla_ansible_vault_password: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_PASSWORD\u0027) | default }}\""},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"# Hashi Vault"},{"line_number":475,"context_line":"kolla_ansbible_vault_addr: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_ADDR\u0027) | default }}\""},{"line_number":476,"context_line":"kolla_ansbible_vault_mount_point: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_MOUNT_POINT\u0027) | default }}\""},{"line_number":477,"context_line":"kolla_ansbible_vault_kv_path: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_KV_PATH\u0027) | default }}\""},{"line_number":478,"context_line":"kolla_ansbible_vault_namespace: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_NAMESPACE\u0027) | default }}\""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"30f47bff_291d164a","line":475,"range":{"start_line":475,"start_character":0,"end_line":475,"end_character":25},"updated":"2022-09-08 12:38:59.000000000","message":"You meant ansible rather than ansbible, right?","commit_id":"aaa5bbc2dc979ea006a697014db1664e69edd5b5"},{"author":{"_account_id":29543,"name":"Scott Solkhon","email":"scott.solkhon@gresearch.co.uk","username":"scott.solkhon"},"change_message_id":"259da63ded758f6a89d1689142ea34355e28adee","unresolved":false,"context_lines":[{"line_number":472,"context_line":"kolla_ansible_vault_password: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_PASSWORD\u0027) | default }}\""},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"# Hashi Vault"},{"line_number":475,"context_line":"kolla_ansbible_vault_addr: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_ADDR\u0027) | default }}\""},{"line_number":476,"context_line":"kolla_ansbible_vault_mount_point: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_MOUNT_POINT\u0027) | default }}\""},{"line_number":477,"context_line":"kolla_ansbible_vault_kv_path: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_KV_PATH\u0027) | default }}\""},{"line_number":478,"context_line":"kolla_ansbible_vault_namespace: \"{{ lookup(\u0027env\u0027, \u0027KAYOBE_VAULT_NAMESPACE\u0027) | default }}\""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"a59a5f9c_59fe7210","line":475,"range":{"start_line":475,"start_character":0,"end_line":475,"end_character":25},"in_reply_to":"30f47bff_291d164a","updated":"2022-09-08 16:28:48.000000000","message":"good catch - that would be a typo :). Fixed in my latest patch.","commit_id":"aaa5bbc2dc979ea006a697014db1664e69edd5b5"}],"ansible/roles/kolla-ansible/library/kolla_passwords.py":[{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"804ed2fefaee86778587ea9d4aa78f0075f8f7db","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            finally:"},{"line_number":168,"context_line":"                os.unlink(src_path)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        if module.params[\u0027vault_addr\u0027] and module.params[\u0027vault_addr\u0027] !\u003d \u0027\u0027:"},{"line_number":171,"context_line":"            src_path \u003d create_named_tempfile()"},{"line_number":172,"context_line":"            try:"},{"line_number":173,"context_line":"                shutil.copyfile(module.params[\u0027src\u0027], src_path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"70aa5bfd_e6bded98","line":170,"range":{"start_line":170,"start_character":8,"end_line":170,"end_character":76},"updated":"2022-11-07 18:09:19.000000000","message":"if module.params[\u0027vault_addr\u0027]\n\nThink you can remove the second part of the conditional. If `module.params[\u0027vault_addr\u0027] `  evaluates to true, then vault_addr can not be \u0027\u0027.","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"11d4a8f11b97dabd26ef6dacb88986582c4325c4","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            finally:"},{"line_number":168,"context_line":"                os.unlink(src_path)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        if module.params[\u0027vault_addr\u0027] and module.params[\u0027vault_addr\u0027] !\u003d \u0027\u0027:"},{"line_number":171,"context_line":"            src_path \u003d create_named_tempfile()"},{"line_number":172,"context_line":"            try:"},{"line_number":173,"context_line":"                shutil.copyfile(module.params[\u0027src\u0027], src_path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"90edfe4e_5581e4b2","line":170,"range":{"start_line":170,"start_character":8,"end_line":170,"end_character":76},"in_reply_to":"24542916_25dc2679","updated":"2022-12-09 12:52:34.000000000","message":"I was worried someone might hose their system with different passwords, but thinking about it again, you can always set `kolla_ansible_vault_addr` in the yaml config instead of using an environment variable. That would prevent this issue.","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"782b84058529f81783dc8059bfce9da78a627d6f","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            finally:"},{"line_number":168,"context_line":"                os.unlink(src_path)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        if module.params[\u0027vault_addr\u0027] and module.params[\u0027vault_addr\u0027] !\u003d \u0027\u0027:"},{"line_number":171,"context_line":"            src_path \u003d create_named_tempfile()"},{"line_number":172,"context_line":"            try:"},{"line_number":173,"context_line":"                shutil.copyfile(module.params[\u0027src\u0027], src_path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a2715b72_8bd3238a","line":170,"range":{"start_line":170,"start_character":8,"end_line":170,"end_character":76},"in_reply_to":"70aa5bfd_e6bded98","updated":"2022-11-07 18:21:00.000000000","message":"Just checking vault_addr does have the potential to silently fallback to use the file based password generation. The issue is that you may forget to set the environment variable: `KAYOBE_VAULT_ADDR` and then kayobe will generate you a new set of passwords and potentially redeploy the services using these instead of the values in the vault. I wonder if we need a flag to explicitly enable the vault integration?","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":29543,"name":"Scott Solkhon","email":"scott.solkhon@gresearch.co.uk","username":"scott.solkhon"},"change_message_id":"2da2389bf36f02f77b6135cdbf21e8ca7be9e87a","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            finally:"},{"line_number":168,"context_line":"                os.unlink(src_path)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        if module.params[\u0027vault_addr\u0027] and module.params[\u0027vault_addr\u0027] !\u003d \u0027\u0027:"},{"line_number":171,"context_line":"            src_path \u003d create_named_tempfile()"},{"line_number":172,"context_line":"            try:"},{"line_number":173,"context_line":"                shutil.copyfile(module.params[\u0027src\u0027], src_path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"24542916_25dc2679","line":170,"range":{"start_line":170,"start_character":8,"end_line":170,"end_character":76},"in_reply_to":"a2715b72_8bd3238a","updated":"2022-11-09 12:59:27.000000000","message":"\u003e Think you can remove the second part of the conditional. If `module.params[\u0027vault_addr\u0027] `  evaluates to true, then vault_addr can not be \u0027\u0027.\n\nGood point. I\u0027ll remove 👍\n\n\u003e Just checking vault_addr does have the potential to silently fallback to use the file based password generation. The issue is that you may forget to set the environment variable: `KAYOBE_VAULT_ADDR` and then kayobe will generate you a new set of passwords and potentially redeploy the services using these instead of the values in the vault. I wonder if we need a flag to explicitly enable the vault integration?\n\nYeh if the env var is missing then it will still run because the file is put into etc/kolla for you in previous runs, but obviously if it was the first time running then you would have to have a copy of that file in etc/kolla/passwords.yml that you previously pulled from Vault otherwise Kayobe will go and generate you some. This is how it is at the moment so nothing is changing and also without that logic you wouldn\u0027t be able to bootstrap new environments.\n\nI\u0027m not sure I share the same enthusiasm for more options - if the user forgets to set KAYOBE_VAULT_ADDR they\u0027re not going to remember to set one to enable the integration either. It also makes it a little clunky IMO. I just see Vault as an additional backend for Kolla passwords so it shouldn\u0027t need anything special other than a few params to connect to Vault.","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"00a1318979f0641f8790887b2ed73d609a2b3975","unresolved":true,"context_lines":[{"line_number":181,"context_line":"                              module.params[\u0027vault_token\u0027],"},{"line_number":182,"context_line":"                              module.params[\u0027vault_cacert\u0027])"},{"line_number":183,"context_line":"                kolla_mergepwd(module, src_path, temp_file_path, temp_file_path)"},{"line_number":184,"context_line":"                kolla_genpwd(module, temp_file_path)"},{"line_number":185,"context_line":"                kolla_writepwd(module, temp_file_path,"},{"line_number":186,"context_line":"                              module.params[\u0027vault_addr\u0027],"},{"line_number":187,"context_line":"                              module.params[\u0027vault_mount_point\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a94347b_fe9b121b","line":184,"updated":"2022-11-07 18:04:39.000000000","message":"When using this code path, we end up calling genpwd again on line 210. This is not necessarily an issue if it keeps the logic clean, as running genpwd a second time will essentially be a no-op. It does mean we parse the file for a second time, but I don\u0027t think this particularly expensive.","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":29543,"name":"Scott Solkhon","email":"scott.solkhon@gresearch.co.uk","username":"scott.solkhon"},"change_message_id":"2da2389bf36f02f77b6135cdbf21e8ca7be9e87a","unresolved":true,"context_lines":[{"line_number":181,"context_line":"                              module.params[\u0027vault_token\u0027],"},{"line_number":182,"context_line":"                              module.params[\u0027vault_cacert\u0027])"},{"line_number":183,"context_line":"                kolla_mergepwd(module, src_path, temp_file_path, temp_file_path)"},{"line_number":184,"context_line":"                kolla_genpwd(module, temp_file_path)"},{"line_number":185,"context_line":"                kolla_writepwd(module, temp_file_path,"},{"line_number":186,"context_line":"                              module.params[\u0027vault_addr\u0027],"},{"line_number":187,"context_line":"                              module.params[\u0027vault_mount_point\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"d6d37edd_b9d6ef9d","line":184,"in_reply_to":"1a94347b_fe9b121b","updated":"2022-11-09 12:59:27.000000000","message":"The logic is as follows:\n\n- Create a tempfile containing the existing values in etc/kolla/passwords.yml\n- kolla_readpwd - Read values that exist in Vault\n- kolla_mergepwd - Merge the two files\n- kolla_genpwd - Generate any missing values (keys may have been added)\n- kolla_writepwd - Write the passwords back into Vault to add the missing values\n\nSo the additional genpwd is needed to catch edge cases where a new key is added to the passwords file that needs a value generating.","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"11d4a8f11b97dabd26ef6dacb88986582c4325c4","unresolved":false,"context_lines":[{"line_number":181,"context_line":"                              module.params[\u0027vault_token\u0027],"},{"line_number":182,"context_line":"                              module.params[\u0027vault_cacert\u0027])"},{"line_number":183,"context_line":"                kolla_mergepwd(module, src_path, temp_file_path, temp_file_path)"},{"line_number":184,"context_line":"                kolla_genpwd(module, temp_file_path)"},{"line_number":185,"context_line":"                kolla_writepwd(module, temp_file_path,"},{"line_number":186,"context_line":"                              module.params[\u0027vault_addr\u0027],"},{"line_number":187,"context_line":"                              module.params[\u0027vault_mount_point\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"53ef1cb6_05ce5831","line":184,"in_reply_to":"d6d37edd_b9d6ef9d","updated":"2022-12-09 12:52:34.000000000","message":"Thanks for the clarification.","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"00a1318979f0641f8790887b2ed73d609a2b3975","unresolved":true,"context_lines":[{"line_number":195,"context_line":"                os.unlink(src_path)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        # Merge in overrides."},{"line_number":198,"context_line":"        if module.params[\u0027overrides\u0027]:"},{"line_number":199,"context_line":"            with tempfile.NamedTemporaryFile(delete\u003dFalse) as f:"},{"line_number":200,"context_line":"                # NOTE(mgoddard): Temporary files are opened in binary mode, so"},{"line_number":201,"context_line":"                # specify an encoding."},{"line_number":202,"context_line":"                yaml.dump(module.params[\u0027overrides\u0027], f, encoding\u003d\u0027utf-8\u0027)"},{"line_number":203,"context_line":"                overrides_path \u003d f.name"},{"line_number":204,"context_line":"            try:"},{"line_number":205,"context_line":"                kolla_mergepwd(module, overrides_path, temp_file_path, temp_file_path)"},{"line_number":206,"context_line":"            finally:"},{"line_number":207,"context_line":"                os.unlink(overrides_path)"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        # Generate null passwords."},{"line_number":210,"context_line":"        kolla_genpwd(module, temp_file_path)"},{"line_number":211,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d8bbe8dd_f32699ef","line":208,"range":{"start_line":198,"start_character":0,"end_line":208,"end_character":0},"updated":"2022-11-07 18:04:39.000000000","message":"Was there a particular reason for not uploading the custom passwords? I can see the potential for confusion if someone is using a custom password to override an entry in kolla passwords e.g libvirt_sasl_password. I think we\u0027d end up uploading ` libvirt_sasl_password` with a password that we essentially never use (as this value gets overridden by the custom password but never uploaded).","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"11d4a8f11b97dabd26ef6dacb88986582c4325c4","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                os.unlink(src_path)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        # Merge in overrides."},{"line_number":198,"context_line":"        if module.params[\u0027overrides\u0027]:"},{"line_number":199,"context_line":"            with tempfile.NamedTemporaryFile(delete\u003dFalse) as f:"},{"line_number":200,"context_line":"                # NOTE(mgoddard): Temporary files are opened in binary mode, so"},{"line_number":201,"context_line":"                # specify an encoding."},{"line_number":202,"context_line":"                yaml.dump(module.params[\u0027overrides\u0027], f, encoding\u003d\u0027utf-8\u0027)"},{"line_number":203,"context_line":"                overrides_path \u003d f.name"},{"line_number":204,"context_line":"            try:"},{"line_number":205,"context_line":"                kolla_mergepwd(module, overrides_path, temp_file_path, temp_file_path)"},{"line_number":206,"context_line":"            finally:"},{"line_number":207,"context_line":"                os.unlink(overrides_path)"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        # Generate null passwords."},{"line_number":210,"context_line":"        kolla_genpwd(module, temp_file_path)"},{"line_number":211,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"f8ae703b_2332b5dd","line":208,"range":{"start_line":198,"start_character":0,"end_line":208,"end_character":0},"in_reply_to":"cbf9f53c_df40afde","updated":"2022-12-09 12:52:34.000000000","message":"Done","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"},{"author":{"_account_id":29543,"name":"Scott Solkhon","email":"scott.solkhon@gresearch.co.uk","username":"scott.solkhon"},"change_message_id":"2da2389bf36f02f77b6135cdbf21e8ca7be9e87a","unresolved":true,"context_lines":[{"line_number":195,"context_line":"                os.unlink(src_path)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        # Merge in overrides."},{"line_number":198,"context_line":"        if module.params[\u0027overrides\u0027]:"},{"line_number":199,"context_line":"            with tempfile.NamedTemporaryFile(delete\u003dFalse) as f:"},{"line_number":200,"context_line":"                # NOTE(mgoddard): Temporary files are opened in binary mode, so"},{"line_number":201,"context_line":"                # specify an encoding."},{"line_number":202,"context_line":"                yaml.dump(module.params[\u0027overrides\u0027], f, encoding\u003d\u0027utf-8\u0027)"},{"line_number":203,"context_line":"                overrides_path \u003d f.name"},{"line_number":204,"context_line":"            try:"},{"line_number":205,"context_line":"                kolla_mergepwd(module, overrides_path, temp_file_path, temp_file_path)"},{"line_number":206,"context_line":"            finally:"},{"line_number":207,"context_line":"                os.unlink(overrides_path)"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        # Generate null passwords."},{"line_number":210,"context_line":"        kolla_genpwd(module, temp_file_path)"},{"line_number":211,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"cbf9f53c_df40afde","line":208,"range":{"start_line":198,"start_character":0,"end_line":208,"end_character":0},"in_reply_to":"d8bbe8dd_f32699ef","updated":"2022-11-09 12:59:27.000000000","message":"That\u0027s a really good catch I hadn\u0027t tested that edge case. I will update this in my latest patch to write back the changes into Vault 👍","commit_id":"a07fff5ba9ac21e579bcff42c958cc796e23a2e6"}]}