Fixes #38241 - Extend userdata API by MAC address endpoint

[bastian-src]

Extend the proxy API to match the userdata API of foreman: theforeman/foreman#9213

[ekohl]

Refs #34887

That issue has been closed and already a Fixed in release version that went out so you can't use Refs for that. Please open a new issue.

[bastian-src]

Thanks a lot for the fast responses! @ekohl

[bastian-src]

@ekohl do you have an idea why I gotta define the additional parameter here while it is not needed in the unattended API here?

[ekohl]

Looking at the method this feels like an abuse of the API:

smart-proxy/modules/templates/userdata_proxy_request.rb

Line 5 in 6624f2c

def get(kind, env, params)

Technically it may work since it calls flatten, but I really don't like flatten because it ends up being really unpredictable what gets called.

But that's probably not the real issue. If you look at ProxyRequest you can see it copies the parameters while removing a blacklist:

smart-proxy/modules/templates/proxy_request.rb

Lines 30 to 33 in 6624f2c

	opts = params.clone.merge(:url => template_url)
	BLACKLIST_PARAMETERS.each do |blacklisted_parameter|
	opts.delete(blacklisted_parameter)
	end

Looking at that blacklist, we can see kind is in there:

smart-proxy/modules/templates/proxy_request.rb

Line 9 in 6624f2c

BLACKLIST_PARAMETERS = ['path', 'template', 'kind', 'hostgroup', 'splat', 'captures']

I suspect both mac and kind are in params. Not entirely sure what the correct thing to do here, but this whole API feels awkward.

[bastian-src]

Alright, I see why hostgroup and template don't need to be added in the test - thanks!

However, to make this API feel less awkward I see two improvements currently:

a.) Add another function in UserdataProxyRequest to mitigate the flatten here?
Like a def get_mac and then put the mac as mandatory function parameter? But, this makes it less generic..

b.) Overwrite BLACKLIST_PARAMETERS when inheriting ProxyRequest and add mac there?

What do you think?

[sbernhard]

@ekohl thoughts on the comment from Bastian?

[dgoetz]

As I am currently running into this problem and the patch at least works, may I ask if we can get it in as a fix or even as a workaround we can later improve on?

[stejskalleos]

🍏 LGTM

Tested with Foreman and it works, all the comments have been addressed, let's get this in.

Comments have been addressed

[sbernhard]

Thanks @stejskalleos and @bastian-src :-)