Skip to content
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

Construct foreman_url from servername and drop parameter #987

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Aug 19, 2021

Drops the foreman_url parameter and constructs it from the servername
given to Apache. The foreman_url and servername should match
and this gives one less parameter that needs to be configured to get
a correct installation.

@ekohl I was not sure if this is what you mean tor if there is some nuance of these parameters that I am missing where you would want them to be different. I also realize this is a "breaking" change but if accurate, feels cleaner.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we simplify this, we should also drop support in the Apache config. In particular here:

if $foreman_url {
$suburi_parts = split($foreman_url, '/')
$suburi_parts_count = size($suburi_parts) - 1
if $suburi_parts_count >= 3 {
$suburi_without_slash = join(values_at($suburi_parts, ["3-${suburi_parts_count}"]), '/')
if $suburi_without_slash {
$suburi = "/${suburi_without_slash}"
} else {
$suburi = undef
}
} else {
$suburi = undef
}
} else {
$suburi = undef
}
if $suburi {
$custom_fragment = undef
} else {
$custom_fragment = file('foreman/_assets.conf.erb')
}

Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to prevent an user from having a Foreman URL such as https://mybox.example.com/foreman and providing additional services from mybox.example.com ?

I suppose you could still have foreman.example.com and otherservice.example.com running on the same machine but need to be able to create multiple PTR records for reverse lookups (to my knowledge we require full forward and reverse name resolution).

I also understand that downstream, the application is supported as an appliance that should not share hardware with any other services. I don't see any obvious reason to impose this restriction upstream where the user should be free to pursue whatever configuration they desire, as they are the one supporting it.

@ekohl
Copy link
Member

ekohl commented Aug 27, 2021

Is there any reason to prevent an user from having a Foreman URL such as https://mybox.example.com/foreman and providing additional services from mybox.example.com ?

This used to be supported but the UI broke this a long time ago and we haven't had bug reports from users. Either nobody knew it was there or nobody cares. See https://community.theforeman.org/t/rfc-stop-attempting-to-allow-running-foreman-under-a-sub-folder-in-3-0/24526

@@ -296,6 +293,8 @@
timeout => 0,
}

$foreman_url = "https://${foreman::servername}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified to using a local variable.

Suggested change
$foreman_url = "https://${foreman::servername}"
$foreman_url = "https://${servername}"

@ekohl
Copy link
Member

ekohl commented Feb 20, 2023

@ehelms any plans to revisit this?

@ehelms
Copy link
Member Author

ehelms commented Feb 20, 2023

I do. And I am wondering how to integrate this into a related request. If we look at https://bugzilla.redhat.com/show_bug.cgi?id=2127515 it denotes multiple URLs that the user wishes could be driven off a single installer input to update:

--foreman-foreman-url
--foreman-unattended-url
--foreman-proxy-foreman-base-url
--foreman-proxy-puppet-url
--foreman-proxy-template-url
--puppet-server-foreman-url

@ekohl
Copy link
Member

ekohl commented Feb 20, 2023

--foreman-unattended-url

I started some investigations into if we can actually move to a world where the whole unattended URL is no longer needed. It's going to be a long one because you want kickstart to get the template over HTTPS (which it can do, even today) while establishing a trusted chain (which we haven't solved today).

The main benefit is that we can get rid of serving HTTP on port 80 using Puma altogether and move everything to HTTPS on port 443. Apache can keep a redirect to HTTPS for users.

Given it's going to be long term, we should probably simplify it in the installer in the short term.

--foreman-proxy-foreman-base-url
--foreman-proxy-puppet-url
--foreman-proxy-template-url

These all live in the puppet-foreman_proxy module, so should be considered out of scope here.

--puppet-server-foreman-url

This lives in puppet-puppet, so also out of scope for this PR.

I've been thinking about how to link them up on the same system. You can use Hiera aliases, but for technical reasons that now interferes with how Kafo treats the answer file.

What you can do in those other modules is default to undef and then later in the code use lookup() explicitly, but that requires some thought.

Drops the foreman_url parameter and constructs it from the servername
given to Apache. The foreman_url and servername should match
and this gives one less parameter that needs to be configured to get
a correct installation.
@ehelms ehelms marked this pull request as ready for review February 27, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants