TFTP feature

[stejskalleos]

Why are you introducing these changes? (Problem description, related links)

Support the TFTP Smart Proxy feature.

What are the changes introduced in this pull request?

• foremanctl deploy ---add-feature tftp

How to test this pull request

./foremanctl deploy --help

  --tftp-root FOREMAN_PROXY_TFTP_ROOT
                        Directory to serve TFTP files from. (persisted)
  --tftp-servername FOREMAN_PROXY_TFTP_SERVERNAME
                        Server name to use for TFTP. (persisted)

./foremanctl deploy --add-feature foreman-proxy --add-feature tftp \
  --tftp-root /var/lib/foreman-custom-tftp \
  --tftp-servername something-else.example.com \

Go to https://quadlet.example.com/smart_proxies/2-quadlet-example-com, TFTP should be enabled.

vagrant ssh quadlet
sudo podman exec -it foreman-proxy /bin/bash

cat /etc/foreman-proxy/settings.d/tftp.yml

---
:enabled: false
:tftproot: /var/lib/foreman-custom-tftp
:tftp_servername: something-else.example.com
:tftp_connect_timeout: 10
:verify_server_cert: true

./foremanctl deploy --remove-feature tftp
./foremanctl features
=>

tftp                      available    Enable TFTP feature on Foreman Proxy

cat /etc/foreman-proxy/settings.d/tftp.yml

---
:enabled: false
:tftproot: /var/lib/foreman-custom-tftp
:tftp_servername: something-else.example.com
:tftp_connect_timeout: 10
:verify_server_cert: true

Go to https://quadlet.example.com/smart_proxies/2-quadlet-example-com, TFTP should be disabled.

Checklist

• Tests added/updated (if applicable)
• Documentation updated (if applicable)

[ekohl]

Does this need a default? IIRC the code has a fallback on the Smart Proxy's hostname already so this doesn't add anything. In a remote setup it will also be guaranteed wrong.

[lzap]

Hey, this is completely random (I was just browsing this repository) but:

TFTP clients usually do not have DNS stack available, therefore, this must be IP address if anything. I see fqdn so I assume this would be a hostname. I remember there was some FQDN>IP hack somewhere but using IP was always the best and the safest thing to do.

This has always been terrible name, servername. What it does is sets up next-server which is probably the root cause of all of this terminology. But what users typically need to set is an IP address.

[ekohl]

You're right, that is a better value. Ideally this value defaults to empty and we force users to provide the value when they enable the TFTP feature. Then input validation forces a valid IPv4 address.

[stejskalleos]

We could at least rename the parameter in foremactl.
Change it to tftp-ip and update the description.
WDYT?

[evgeni]

Can we get a test that sets up a tftp-server (on the quadlet machine), instructs the proxy to create a file in the tftp root and uses tftp to fetch that file?

[evgeni]

Ewouds comment in jira made me realize that this description is wrong.
We also use /var/lib/tftpboot for the httpboot module (https://github.com/theforeman/smart-proxy/blob/3e249dd019dd36f41b546fe44c045ab055c1da6c/modules/httpboot/httpboot_plugin.rb#L13) and do not allow users to configure that in Puppet.

It is unclear to me how the HTTPBoot thing obtains the files (seems there is no code for that), so maybe it relies on the TFTP module to place the files and just exposes them over HTTP instead of TFTP?

(This is not blocking the feature here, just food for further thought going forward)

[arvind4501]

I also faced similar doubts while just scaffolding httpboot feature, and i realised its use /var/lib/tftp which is not created by httpboot, which makes me think why we don't add tftp as dependency for httpboot. for full context you can refer #518 (comment)

[ekohl]

which makes me think why we don't add tftp as dependency for httpboot.

You may find theforeman/smart-proxy@db33bc6 interesting.

Perhaps we should call it the netboot directory? I already had https://github.com/theforeman/puppet-foreman_proxy/blob/master/manifests/tftp/netboot.pp in the old installer.

[evgeni]

"interesting" as in "wait, what!?"? ;-)

How is the directory populated, when the TFTP feature is off, but HTTPBoot is on?

[ekohl]

Magic? In theforeman/puppet-foreman_proxy@e4b7763 I wrote:

To get the netboot files, the TFTP feature must still be enabled.

It was a long discussion that was rather tedious. I'm not sure the whole use case of netbooting with HTTPBoot without TFTP was ever really well supported.

[arvind4501]

How is the directory populated, when the TFTP feature is off, but HTTPBoot is on?

AFAIK, no population happens, just configuration, As of today, by default httpboot enablement will add :root_dir: /var/lib/tftpboot in config, and still be indirectly dependent on tftp to create and populate that directory

[stejskalleos]

Can we get a test that sets up a tftp-server (on the quadlet machine), instructs the proxy to create a file in the tftp root and uses tftp to fetch that file?

I'm still working on this test; I will add it in the upcoming commit.

[stejskalleos]

@ehelms applied comments from your review.
Can't add the tests for the feature because of a broken local environment.
I'm trying to figure it out in the meantime.

[ehelms]

Given the validation message below, I'd include this line here in some form:

  This should be the IP address of the TFTP server (TFTP clients typically do not have DNS available).

[ehelms]

Which I will also point out is slightly ironic we call it servername and then state it should be the IP address.

[ekohl]

Technically we're talking about https://www.rfc-editor.org/info/rfc2132/#section-9.4. DHCP option 66 is called TFTP server name. Sadly it doesn't describe at all what the content of it should be at the protocol level.

If we look at what we do for ISC DHCP (https://github.com/theforeman/smart-proxy/blob/3e249dd019dd36f41b546fe44c045ab055c1da6c/modules/dhcp_common/isc/omapi_provider.rb#L205-L217) then that's basically trying to send an IP, but ISC DHCP itself can also be configured with a hostname where the service does a lookup.

Kea's all-options.json actually has an example that is a hostname: https://github.com/isc-projects/kea/blob/f79874a1f22c29b96ee543f5bc905960818991f7/doc/examples/kea4/all-options.json#L787-L798.

So I'm not sure we need to be so strict in only accepting an IP. Apologies for not digging into that deep enough before.

[stejskalleos]

Leftover, will remove