Fixes #14232 - make generic helpers available for global PXE template #3335
Conversation
| @@ -121,7 +121,8 @@ def self.build_pxe_default(renderer) | |||
| if error_msg.empty? | |||
| begin | |||
| @profiles = pxe_default_combos | |||
| menu = renderer.render_safe(default_template.template, [:default_template_url], {:profiles => @profiles}) | |||
| allowed_helpers = ALLOWED_GENERIC_HELPERS + [ :default_template_url ] | |||
There was a problem hiding this comment.
I receive an error building the default PXE menu, I think from here: failed to process template: uninitialized constant ProvisioningTemplate::ALLOWED_GENERIC_HELPERS
There was a problem hiding this comment.
I'm sorry, last second change which I somehow didn't test
|
This doesn't appear to fix the described bug, it only permits safemode access to the existing method and doesn't give the "name" (FQDN, presumably). Should this also address #7828? |
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
In the linked BZ reporter was ok with just using foreman_url in his use-case. But you're right, since we're touching this we can add new method returning just the FQDN of foreman.
added foreman_server_fqdn, if it get's in, I'll document it at http://projects.theforeman.org/projects/foreman/wiki/templatewriting
partly yes, #7828 seems to be solved by #11723 already, it just doesn't work with safe mode enabled, I've marked it as duplicate |
|
I will test #7828 as it's important feature for Discovery, thanks! |
| @@ -57,6 +59,11 @@ def foreman_url(action = "built") | |||
| :token => (@host.token.value unless @host.try(:token).nil?), :kind => action | |||
| end | |||
|
|
|||
| def foreman_server_fqdn | |||
| config = URI.parse(Setting[:unattended_url]) | |||
| config.host || request.host | |||
There was a problem hiding this comment.
Using the host only is perhaps not quite enough to help with #7828, where it's needed to build a URL for say, Discovery - the protocol, port and path specified in :unattended_url are relevant too.
While a plethora of helpers is a bad thing, I can see a case for having helpers for either the FQDN and the base URL, or the base URL only. If the former, we can merge this as-is to provide the FQDN and re-open #7828 - I'm just unsure this fully solves it as-is.
There was a problem hiding this comment.
Yeah, FQDN is not enough, but it's a good start. Scheme is always https in our case and port is either 443 or 8443 depending on connection type. I filed a ticket that could provide the rest via settings: http://projects.theforeman.org/issues/14245. And if we can get full URL with scheme and port, at least for Discovery case we can detect what we need.
There was a problem hiding this comment.
I'd much prefer to provide a helper that simply returns the URL rather than have users write against specific setting names, because it provides a good layer of abstraction against internal changes and features. (For example, foreman_url supports template proxies, which is more complex than just checking the value of a setting.)
There was a problem hiding this comment.
ok, I can add foreman_server_url that will add protocol and port too (no path, no template proxies in this one), would that help?
There was a problem hiding this comment.
Templating port is irrelevant for Discovery as it always uses HTTPS port and HTTP port (which is utilized for templating). Therefore this won't solve the discovery case at all. Maybe if you could add a param force_https that could be used in Discovery global PXELinux section to force it.
There was a problem hiding this comment.
It sounds incorrect to force HTTPS, but what you're actually describing is that you prefer the value of :foreman_url setting and not the :unattended_url.
There was a problem hiding this comment.
Which seems to make sense for both new helpers. The only thing that makes it confusing is that our existing foreman_url helper uses unattended_url setting while the new one would use foreman_url setting. Anyway I've added two new helpers, please take a look.
EDIT: and asked @lzap for his comments after short discussion
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
After short call with @ares I confirm what's implemented now is enough for discovery. Since we don't support multiple PXELinux default templates per subnet, the only sane default for discovery is to use foreman URL. And for this, |
|
tests failures do not seem to be related |
|
Merged as e4ed4a0, thanks @ares. Please update http://projects.theforeman.org/projects/foreman/wiki/TemplateWriting with the new features and feel free to use them in the community-templates develop branch. |
|
Thanks, I'll do it later today. |
|
Created patch for discovery global PXELinux default one. |
|
updated TemplateWriting docs with other changes, link for diff http://projects.theforeman.org/projects/foreman/wiki/TemplateWriting/diff?utf8=%E2%9C%93&version=84&version_from=83&commit=View+differences |
No description provided.