| @ldeataid:matrix.org | Looks like the Openstack guys set the certs files to a folder with a name related to the service, for example: | 12:00 |
|---|---|---|
| Nova: https://opendev.org/openstack/openstack-helm/src/branch/master/nova/templates/daemonset-compute.yaml#L449 | ||
| Cinder: https://opendev.org/openstack/openstack-helm/src/branch/master/cinder/templates/deployment-api.yaml#L86 | ||
| Neutron: https://opendev.org/openstack/openstack-helm/src/branch/master/neutron/templates/deployment-server.yaml#L178 | ||
| This might be the reason why on Horizon it's on a folder called "openstack-dashboard", so I don't think they would accept a change on Horizon to move this to "etc/ssl/certs" | ||
| If you notice on [0019-Change-Horizon-CA-Cert-file-path.patch](https://review.opendev.org/c/starlingx/openstack-armada-app/+/892707/4/openstack-helm/debian/deb_folder/patches/0019-Change-Horizon-CA-Cert-file-path.patch) lines 51 52 are actually changing the values inside a huge template: https://opendev.org/openstack/openstack-helm/src/branch/master/horizon/values.yaml#L250, which is why I don't think we should let this be dynamically changed via Helm overrides or plugins, as we'd probably have to override the entire template instead of just a single value. | ||
| Because of that, I think the correct solution would be to change [0019-Change-Horizon-CA-Cert-file-path.patch](https://review.opendev.org/c/starlingx/openstack-armada-app/+/892707/4/openstack-helm/debian/deb_folder/patches/0019-Change-Horizon-CA-Cert-file-path.patch) to something like: | ||
| ``` | ||
| - OPENSTACK_SSL_CACERT = '/etc/openstack-dashboard/certs/ca.crt' | ||
| + OPENSTACK_SSL_CACERT = {{ .Values.ca_cert.path }} | ||
| ``` | ||
| Notice that we'd have to create this variable, as it currently does not exist. | ||
| However, considering that it's not ideal to change the location of this parameter as it can break services functionalities, and that other STX-Openstack services also have the value for this .crt file hard coded, I don't think the change is wrong (keeping it clear that I also don't like hard coded stuff). | ||
| Will leave the decision up to you | ||
| @ldeataid:matrix.org | > <@lutimura:matrix.org> You guys pretty much summed it up 😅 | 12:05 |
| > | ||
| > Having things _helm overridable_ is always desirable from a user's perspective. However, I'm afraid that we already have a solution in place and that we could use to encompass this deployment spec. If you take a look at this patch, [0010-Remove-TLS-from-openstack-services.patch](https://opendev.org/starlingx/openstack-armada-app/src/branch/master/openstack-helm/debian/deb_folder/patches/0010-Remove-TLS-from-openstack-services.patch#L1823-L1827), you will see that it already patches multiple REQUEST_CA_BUNDLEs, just like Lucas de Ataides did in his review. | ||
| > | ||
| > So, since we already have static overrides that change OPENSTACK_SSL_CACERT to our desired value, as tcervi pointed out, I think we only need to update the existing patch to also change Horizon's REQUEST_CA_BUNDLE. | ||
| > | ||
| > What do you guys think? | ||
| Oops, totally overlooked this, but it complements what I said above, this value is hard coded on other services as well | ||
| @fungicide:matrix.org | also note that was a configuration decision taken by the openstack-helm maintainers, it's not necessarily an openstack-wide standard. openstack-helm is one of about half a dozen different deployment alternatives (vs kolla, chef, charms/sunbeam, puppet...) | 12:16 |
| @fungicide:matrix.org | just happens to be the one starlingx chose to build atop | 12:16 |
| @fungicide:matrix.org | if there are things worth improving there, the openstack-helm maintainers may be interested in your suggestions and assistance even | 12:17 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!