Expose /pub directory #316
Conversation
src/roles/httpd/defaults/main.yml
Outdated
| httpd_pulp_api_backend: http://localhost:24817 | ||
| httpd_pulp_content_backend: http://localhost:24816 | ||
| httpd_foreman_backend: http://localhost:3000 | ||
| httpd_rpm_server_dir: /var/www/html/pub |
There was a problem hiding this comment.
why "rpm server"? I'd probably just do http_pub_dir or so
There was a problem hiding this comment.
To be honest i went with this as it was used previously https://github.com/theforeman/puppet-foreman_proxy_content/blob/master/manifests/bootstrap_rpm.pp#L38, but you are right httpd_pub_dir is more generalized and readble
arvind4501
force-pushed
the
expose-pub
branch
from
b1183e9 to
3f165ad
Compare
|
Ansible looks good to me. Can you please add a test to https://github.com/theforeman/foremanctl/blob/master/tests/httpd_test.py? |
| - name: Copy CA certificate to pub directory for client trust | ||
| ansible.builtin.copy: | ||
| src: "{{ httpd_server_ca_certificate }}" | ||
| dest: "{{ httpd_pub_dir }}/katello-server-ca.crt" | ||
| remote_src: true | ||
| mode: "0644" |
There was a problem hiding this comment.
technically this is also available under /unattended/public/foreman_raw_ca without /pub, but lots of code relies on the old location and it's a good example of usage for now.
There was a problem hiding this comment.
Does it make sense to configure Apache to proxy that specific request?
There was a problem hiding this comment.
Proxy wouldn't work, as rails doesn't know what /pub is. We'd either need to teach it that route or translate at Apache level. Or just ignore it for now.
There was a problem hiding this comment.
The reason I ask is that we also know we need to support /pub/katello-consumer-ca because Anaconda relies on that. It's in RHEL 9 so we can't easily replace that. So long term I think we need to route specific /pub paths to Foreman to render it dynamically. The public templates mechanism in Foreman could be reused for /pub/katello-consumer-ca.
We also deal with it in a follow up.
There was a problem hiding this comment.
My thoughts on this:-
I think in /pub we have quite a few items from multiple plugins, so ideally it does not make sense to put these things, which is not completely related to foreman, in foreman/rails routes.
There was a problem hiding this comment.
I think in /pub we have quite a few items from multiple plugins, so ideally it does not make sense to put these things, which is not completely related to foreman, in foreman/rails routes.
I'd like to see a list of those plugins, because no plugin should count on /pub existing. Katello and RH cloud might, but others shouldn't.
There was a problem hiding this comment.
In the description of SAT-40189, it is mentioned
- IoP -> For uploading/fetching cvemap.xml
- Discovery -> Uploading FDI image for PXELess Discovery Workflow
- Bootdisk -> Uploading Bootdisk image for Bootdisk Workflow
- katello-ca-consumer -> global registration uses katello-ca-consumer rpm published on /pub
Few other component tests like Repository, ISS, ACS also use /pub during testing.
After looking at these i thought we can keep the pub directory served by apache only.
There was a problem hiding this comment.
- IoP is correct
- Discovery and Bootdisk is not using
/pub(but the way tests in robotello are written for it might abuse `/pub) - global registration does not use /pub, it embedds all certs itself.
There was a problem hiding this comment.
i see that the katello-ca-consumer-latest.noarch.rpm still stays in pub, is that not being used now for registration(doc says (Deprecated) Katello CA Consumer method for registeration is deprecated)?
There was a problem hiding this comment.
correct. it's still there (for users who have non-default workflows), but unused by the default workflow we offer.
|
This PR currently exposes only |
|
I'd keep the scope as is, we can add more later as we need |
arvind4501
force-pushed
the
expose-pub
branch
from
06b0736 to
01a8fb2
Compare
| def test_pub_directory_accessible(server, certificates, server_fqdn): | ||
| cmd = server.run(f"curl --cacert {certificates['ca_certificate']} --silent --output /dev/null --write-out '%{{http_code}}' https://{server_fqdn}/pub/") |
There was a problem hiding this comment.
I think if we validating a CA certificate is downloadable which would cover this test already
There was a problem hiding this comment.
We deploy the config with Indexes, which produces a nice directory index here and this step validates it. Otherwise you'd get a 403 on the /pub/ itself.
| def test_pub_directory_accessible(server, certificates, server_fqdn): | ||
| cmd = server.run(f"curl --cacert {certificates['ca_certificate']} --silent --output /dev/null --write-out '%{{http_code}}' https://{server_fqdn}/pub/") |
There was a problem hiding this comment.
I'd suggest one more test here which checks the pub dir accessibility from http url
evgeni
left a comment
evgeni
left a comment
There was a problem hiding this comment.
tested using foremanctl-1.0.0-0.20251125064410703050.pr316.68.gb6dbef4.el9.noarch.rpm, works as expected.
|
Thanks everyone! |
4 participants
This PR exposes /pub directory. it is a initial work around and currently only exposes
katello-server-ca.crt.