fixes #10286 - ensure default PXE menu can use templates proxy #2327
Conversation
stbenjam
changed the title
|
Ok will review later /CC |
| conditions[:puppet_ca_proxy_id] = id | ||
| end | ||
| conditions | ||
| end | ||
|
|
||
| # Helper methods for features - e.g. has_puppet? has_dns? |
There was a problem hiding this comment.
this appears to be squashed in from #2329, which should be reviewed first
There was a problem hiding this comment.
Yes, I broke apart a single PR into several because there was too much going on, when the other two are merged, I'll fix this one
stbenjam
force-pushed
the
10286
branch
2 times, most recently
from
bbce40f
to
7734c27
Compare
stbenjam
changed the title
|
@lzap Updated and ready to review @ your convenience. Much clearer as a separate commit. |
| Setting[:unattended_url] = "http://foreman.unattended.url" | ||
| @request.env['HTTP_REFERER'] = config_templates_path | ||
|
|
||
| # With Templates Proxy |
There was a problem hiding this comment.
Split this into a new test if you can, to save this one covering multiple things. Move the general initialisation bits into a context + setup block if necessary.
There was a problem hiding this comment.
The problem I had was there's a TFTP proxy in fixtures, so if it's not in the same test, ProxyAPI::Template gets an unexpected invocation for create_default. Any way to avoid that?
Edit: fixtures, not features
There was a problem hiding this comment.
If we can delete the fixture, that'd be fantastic. If not, .delete_all in setup...
|
Everything else reviews well for me, untested. |
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
stbenjam
force-pushed
the
10286
branch
8 times, most recently
from
4c3211f
to
ccb0385
Compare
| setup do | ||
| proxy = smart_proxies(:two) | ||
| proxy.features = [] | ||
| proxy.save! |
There was a problem hiding this comment.
Only way to avoid having this fixture TFTP proxy in the tests. It'd be nice if the feature Smart Proxies became factories, but they're used in Taxonomy tests and not touching those 👻 😨
|
@domcleal Updated |
|
This PR's fine, works perfectly. However, am I missing something here, as the smart proxy's templates module doesn't support URLs of this type as far as I can tell. Requests such as this simply 404, as there's no sinatra API request handler for it. |
|
Oh, d'oh. I fixed this on my smart proxy months ago and forgot to open a PR... will clean it up! Sorry 😁 |
|
Any update on this one? Having to manually update the |
|
@stbenjam can help testing and reviewing! Discovery 😋 😋 😍 |
|
I had patched my Smart Proxy to support the template URL's but it didn't capture all cases. That would need fixing. I don't have the bandwidth to keep fixing the endless issues with host group provisioning. If it needs fixing, it should be a feature team and allocated resources, otherwise we rely on fact-based discovery. |
No description provided.