)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a19ba495b101183cd9903459a263e00167382b4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"be56a9cf_ae0aa2e1","updated":"2022-10-11 07:27:49.000000000","message":"I had to look into the old git histories but it seems we selected the wrong value when we were forced to set the ssl_verify_client parameter[1][2][3] though we didn\u0027t intend to enable client auth.\n\n[1] https://review.opendev.org/c/openstack/tripleo-heat-templates/+/759285\n[2] https://review.opendev.org/c/openstack/tripleo-heat-templates/+/763172\n[3] https://review.opendev.org/c/openstack/puppet-horizon/+/758041/\n\nWe better consider enabling Horizon in IPA job and trigger it by the change in horizon template so that we ensure horizon is working in tls-e setup, but in the mean time the change itself looks reasonable to me.","commit_id":"1788cbac8540a101cc113b16044db91943bf2b5b"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"dc0a445266ed33b9b123ff7c30dd9935917ac7c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"30352b1e_aef771e3","updated":"2022-10-11 06:56:46.000000000","message":"adding Takashi wdyt please see inline comment","commit_id":"1788cbac8540a101cc113b16044db91943bf2b5b"}],"deployment/horizon/horizon-container-puppet.yaml":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"dc0a445266ed33b9b123ff7c30dd9935917ac7c1","unresolved":true,"context_lines":[{"line_number":253,"context_line":"        - if:"},{"line_number":254,"context_line":"          - {get_param: EnableInternalTLS}"},{"line_number":255,"context_line":"          - horizon::horizon_ca: {get_param: InternalTLSCAFile}"},{"line_number":256,"context_line":"            horizon::ssl_verify_client: none"},{"line_number":257,"context_line":"        - if:"},{"line_number":258,"context_line":"          - {get_param: WebSSOEnable}"},{"line_number":259,"context_line":"          - horizon::websso_enabled:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"f4707506_44903e45","line":256,"updated":"2022-10-11 06:56:46.000000000","message":"i think we need someone more familiar with the puppet modules to weigh  in here\n\nbut i think this is wrong.\n\nlooking at https://opendev.org/openstack/puppet-horizon/src/commit/dac3cece5198e9cd76c7419dc697a34d85b62d08/manifests/wsgi/apache.pp#L188 for example\n\nit seems we cannot set the cert and then set verify_client none?\n\n\nas an aside to this patch however, the horizon_ca param seems to be deprecated per https://opendev.org/openstack/puppet-horizon/commit/b725778cd2d2350b86e8eedf7e09bb24b0c11ed4  and should be updated \n\nperhaps this is the reason for the bug you\u0027re hitting ?","commit_id":"1788cbac8540a101cc113b16044db91943bf2b5b"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a19ba495b101183cd9903459a263e00167382b4b","unresolved":true,"context_lines":[{"line_number":253,"context_line":"        - if:"},{"line_number":254,"context_line":"          - {get_param: EnableInternalTLS}"},{"line_number":255,"context_line":"          - horizon::horizon_ca: {get_param: InternalTLSCAFile}"},{"line_number":256,"context_line":"            horizon::ssl_verify_client: none"},{"line_number":257,"context_line":"        - if:"},{"line_number":258,"context_line":"          - {get_param: WebSSOEnable}"},{"line_number":259,"context_line":"          - horizon::websso_enabled:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"5f57cde7_5431bb2b","line":256,"in_reply_to":"006aaf77_7b5aa160","updated":"2022-10-11 07:27:49.000000000","message":"The implementation looks correct. The problem here is that the SSLVerifyCleint is set only when TLS-e is used but ipa job is not triggered by this change.","commit_id":"1788cbac8540a101cc113b16044db91943bf2b5b"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"32ce7d4ee7a4861666f0213faae21956b83624d6","unresolved":true,"context_lines":[{"line_number":253,"context_line":"        - if:"},{"line_number":254,"context_line":"          - {get_param: EnableInternalTLS}"},{"line_number":255,"context_line":"          - horizon::horizon_ca: {get_param: InternalTLSCAFile}"},{"line_number":256,"context_line":"            horizon::ssl_verify_client: none"},{"line_number":257,"context_line":"        - if:"},{"line_number":258,"context_line":"          - {get_param: WebSSOEnable}"},{"line_number":259,"context_line":"          - horizon::websso_enabled:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"810886c2_09cf7e53","line":256,"in_reply_to":"5f57cde7_5431bb2b","updated":"2022-10-11 07:31:20.000000000","message":"Sorry I missed one good point Marios raised. the horizon_ca parameter was deprecated in favor of the new ssl_ca parameter and it no longer exists in master. The new ssl_ca parameter is present in stable/wallaby so we better fix the ca parameter name as well.","commit_id":"1788cbac8540a101cc113b16044db91943bf2b5b"},{"author":{"_account_id":34754,"name":"Cristian Le","email":"opendev@lecris.me","username":"lecris"},"change_message_id":"ac98d3e0bc7ee7ec3208d2b80ff74991b97d5329","unresolved":true,"context_lines":[{"line_number":253,"context_line":"        - if:"},{"line_number":254,"context_line":"          - {get_param: EnableInternalTLS}"},{"line_number":255,"context_line":"          - horizon::horizon_ca: {get_param: InternalTLSCAFile}"},{"line_number":256,"context_line":"            horizon::ssl_verify_client: none"},{"line_number":257,"context_line":"        - if:"},{"line_number":258,"context_line":"          - {get_param: WebSSOEnable}"},{"line_number":259,"context_line":"          - horizon::websso_enabled:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"006aaf77_7b5aa160","line":256,"in_reply_to":"f4707506_44903e45","updated":"2022-10-11 07:11:09.000000000","message":"None here is string \u0027none\u0027, not null, that why zuul passes.\n\nThanks for pointing out the deprecated values, I\u0027ll try to patch those here too.\n\nAs for the bug, I am fairly confident that it is the lack of client certificate support on haproxy side. For reference here is the haproxy config that it generates.\n```\nbackend horizon_be\n  mode http\n  cookie SERVERID insert indirect nocache\n  option httpchk\n  server controller-0.internalapi.openstack.lab 192.168.2.54:443 ca-file /etc/pki/CA/certs/ca.crt check cookie controller-0.internalapi.openstack.lab fall 5 inter 2000 rise 2 ssl verify required verifyhost controller-0.internalapi.openstack.lab\n```\nNotice the lack of crt parameter according to the guide [1]. Maybe I am missing somewhere else that these are being added to, so I\u0027ll check if fixing the deprecated values has any effect.\n\n[1] https://www.haproxy.com/documentation/hapee/latest/security/authentication/client-certificate-authentication/","commit_id":"1788cbac8540a101cc113b16044db91943bf2b5b"}]}