-
Notifications
You must be signed in to change notification settings - Fork 38
Expose /pub directory #316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,53 @@ | ||
| HTTP_HOST = 'localhost' | ||
| HTTP_PORT = 80 | ||
| HTTPS_PORT = 443 | ||
|
|
||
| HTTPD_PUB_DIR = '/var/www/html/pub' | ||
|
|
||
| def test_httpd_service(server): | ||
| httpd = server.service("httpd") | ||
| assert httpd.is_running | ||
| assert httpd.is_enabled | ||
|
|
||
|
|
||
| def test_http_port(server): | ||
| httpd = server.addr(HTTP_HOST) | ||
| assert httpd.port(HTTP_PORT).is_reachable | ||
|
|
||
|
|
||
| def test_https_port(server): | ||
| httpd = server.addr(HTTP_HOST) | ||
| assert httpd.port(HTTPS_PORT).is_reachable | ||
|
|
||
|
|
||
| def test_https_foreman_ping(server, certificates, server_fqdn): | ||
| cmd = server.run(f"curl --cacert {certificates['ca_certificate']} --silent --output /dev/null --write-out '%{{http_code}}' https://{server_fqdn}/api/v2/ping") | ||
| assert cmd.succeeded | ||
| assert cmd.stdout == '200' | ||
|
|
||
|
|
||
| def test_https_pulp_status(server, certificates, server_fqdn): | ||
| cmd = server.run(f"curl --cacert {certificates['ca_certificate']} --silent --output /dev/null --write-out '%{{http_code}}' https://{server_fqdn}/pulp/api/v3/status/") | ||
| assert cmd.succeeded | ||
| assert cmd.stdout == '200' | ||
|
|
||
|
|
||
| def test_https_pulp_content(server, certificates, server_fqdn): | ||
| cmd = server.run(f"curl --cacert {certificates['ca_certificate']} --silent --output /dev/null --write-out '%{{http_code}}' https://{server_fqdn}/pulp/content/") | ||
| assert cmd.succeeded | ||
| assert cmd.stdout == '200' | ||
|
|
||
|
|
||
| def test_https_pulp_auth(server, certificates, server_fqdn): | ||
| cmd = server.run(f"curl --cacert {certificates['ca_certificate']} --silent --write-out '%{{stderr}}%{{http_code}}' --cert {certificates['client_certificate']} --key {certificates['client_key']} https://{server_fqdn}/pulp/api/v3/users/") | ||
| assert cmd.succeeded | ||
| assert cmd.stderr == '200' | ||
|
|
||
| def test_pub_directory_exists(server): | ||
| pub_dir = server.file(HTTPD_PUB_DIR) | ||
| assert pub_dir.exists | ||
| assert pub_dir.is_directory | ||
| assert pub_dir.mode == 0o755 | ||
|
|
||
| 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/") | ||
|
Comment on lines
+45
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if we validating a CA certificate is downloadable which would cover this test already
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We deploy the config with
Comment on lines
+45
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest one more test here which checks the pub dir accessibility from http url
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not until #293 is done. |
||
| assert cmd.succeeded | ||
| assert cmd.stdout == '200' | ||
|
|
||
| def test_pub_ca_certificate_downloadable(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/katello-server-ca.crt") | ||
| assert cmd.succeeded | ||
| assert cmd.stdout == '200' | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically this is also available under
/unattended/public/foreman_raw_cawithout/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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to configure Apache to proxy that specific request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I ask is that we also know we need to support
/pub/katello-consumer-cabecause 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/pubpaths to Foreman to render it dynamically. Thepublictemplates 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see a list of those plugins, because no plugin should count on
/pubexisting. Katello and RH cloud might, but others shouldn't.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the description of SAT-40189, it is mentioned
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/pub(but the way tests in robotello are written for it might abuse `/pub)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i see that the
katello-ca-consumer-latest.noarch.rpmstill stays inpub, is that not being used now for registration(doc says(Deprecated) Katello CA Consumermethod for registeration is deprecated)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct. it's still there (for users who have non-default workflows), but unused by the default workflow we offer.