Project

General

Profile

Bug #14479

Function foreman_url check for tokens

Added by Lukas Zapletal almost 5 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Category:
Templates
Target version:
Difficulty:
Triaged:
Bugzilla link:
Fixed in Releases:
Found in Releases:

Description

In function foreman_url we explicitly check for tokens and I can't find a reason why we do that:

      url = if @template_url && @host.try(:token).present?
              @template_url
            elsif proxy.present? && proxy.has_feature?('Templates') && @host.try(:token).present?
              temp_url = ProxyAPI::Template.new(:url => proxy.url).template_url
              if temp_url.nil?
                logger.warn("unable to obtain template url set by proxy #{proxy.url}. falling back on proxy url.")
                temp_url = proxy.url
              end
              temp_url
            end

This simply blocks provisioning when using Templating Smart Proxy plugin. Not sure about the check on the first line, if that's appropriate or not.


Related issues

Related to Smart Proxy - Bug #10259: Template proxy does not lookup by MAC addressClosed2015-04-24
Related to Foreman - Bug #17636: Template preview requires token to be present for template proxyingClosed2016-12-12

Associated revisions

Revision 6a9f57a6 (diff)
Added by Lukas Zapletal almost 5 years ago

Fixes #14479 - removed unwanted check for token param

History

#1 Updated by Stephen Benjamin almost 5 years ago

With the token checks, you won't see the proxied url in templates when the host isn't in build mode, or at all when tokens are disabled. Surprised we didn't notice until now.

Anyway, that first one is used to pull the proxy url from params to avoid requesting it multiple times from the Proxy, I think the check can be removed there too. Here's where it's set: https://github.com/theforeman/foreman/blob/develop/lib/foreman/renderer.rb#L141-L144

#2 Updated by The Foreman Bot almost 5 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/3404 added

#3 Updated by Dominic Cleal almost 5 years ago

  • Category set to Templates
  • Legacy Backlogs Release (now unused) set to 136

This simply blocks provisioning when using Templating Smart Proxy plugin

What does this mean precisely? Tokens are enabled by default, it should typically evaluate to true.

With the token checks, you won't see the proxied url in templates when the host isn't in build mode, or at all when tokens are disabled. Surprised we didn't notice until now.

Indeed, I see the point about previewing when not in build mode. The template support always required tokens to identify the client, which I believe is why the check was there. (If there's no token, the feature won't work.)

#4 Updated by Lukas Zapletal almost 5 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

#5 Updated by Lukas Zapletal almost 5 years ago

This simply blocks provisioning when using Templating Smart Proxy plugin

What does this mean precisely? Tokens are enabled by default, it should typically evaluate to true.

I mean when you disable tokens and enable template proxying, it will never work.

Indeed, I see the point about previewing when not in build mode. The template support always required tokens to identify the client, which I believe is why the check was there. (If there's no token, the feature won't work.)

Yes, sure, I see that too, but we should either check only for appropriate cases, or remove the check. Kickstarts work without tokens just fine (MAC address by Anaconda via headers) and there is the client IP address fallback which should work for others.

#6 Updated by Lukas Zapletal over 4 years ago

  • Related to Bug #10259: Template proxy does not lookup by MAC address added

#7 Updated by Lukas Zapletal over 4 years ago

  • Bugzilla link set to 1292421

#8 Updated by Lukas Zapletal about 4 years ago

  • Related to Bug #17636: Template preview requires token to be present for template proxying added

Also available in: Atom PDF