-
Notifications
You must be signed in to change notification settings - Fork 43
TFTP feature #533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TFTP feature #533
Changes from all commits
16990ad
0db63fe
625bc50
03f7f1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| --- | ||
| help: | | ||
| Setup TFTP server | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| --- | ||
| - name: TFTP | ||
| hosts: | ||
| - quadlet | ||
| become: true | ||
| tasks: | ||
| - name: Install packages | ||
| ansible.builtin.package: | ||
| name: | ||
| - tftp-server | ||
| - tftp | ||
| state: present | ||
|
|
||
| - name: Start TFTP services | ||
| ansible.builtin.systemd: | ||
| name: "{{ item }}" | ||
| state: started | ||
| enabled: true | ||
| with_items: | ||
| - tftp.service |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,10 +56,21 @@ variables: | |
| parameter: --bmc-redfish-verify-ssl | ||
| help: Verify SSL certificates for Redfish BMC connections. | ||
| type: Boolean | ||
| foreman_proxy_tftp_root: | ||
| parameter: --tftp-root | ||
| help: Directory to serve TFTP files from, defaults to /var/lib/tftpboot if not specified. | ||
| type: AbsolutePath | ||
| foreman_proxy_tftp_servername: | ||
| parameter: --tftp-servername | ||
| help: Server name or IP address for TFTP. TFTP clients typically do not have DNS available, so an IP address is recommended. | ||
|
|
||
| constraints: | ||
| required_together: | ||
| - [certificates_custom_server_certificate, certificates_custom_server_key, certificates_custom_server_ca_certificate] | ||
| forbidden_if: | ||
| - [certificates_source, installer, [certificates_custom_server_certificate, certificates_custom_server_key, certificates_custom_server_ca_certificate]] | ||
| required_in_list: | ||
| - [[[features, tftp]], [foreman_proxy_tftp_servername]] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add a foreman-proxy feature as a requirement here as well? I tried to enable tftp without foreman-proxy and it was enabled without any errors, but I know ideally it wouldn't work, so should we add a better validation here to check if tftp is present in required_in_list or do we want to enable foreman-proxy feature as dependancy to TFTP?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ekohl What is the correct behavior in this scenario: User runs What's the expected behavior?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think This will also apply to all other Smart Proxy features, like DNS and DHCP. I lean to tracking that as a follow up task because I doubt we fixed in other places.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| include: | ||
| - _certificate_source | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| --- | ||
| - name: TFTP root directory | ||
| notify: | ||
| - Restart Foreman Proxy | ||
| - Refresh Foreman Proxy | ||
| block: | ||
| - name: Create TFTP root directory | ||
| ansible.builtin.file: | ||
| path: "{{ foreman_proxy_tftp_root }}" | ||
| state: directory | ||
| mode: "0755" | ||
| owner: 998 | ||
| group: 998 | ||
|
|
||
| - name: Mount the directory into foreman-proxy container | ||
| ansible.builtin.copy: | ||
| dest: /etc/containers/systemd/foreman-proxy.container.d/tftp-root.conf | ||
| content: | | ||
| [Container] | ||
| Volume={{ foreman_proxy_tftp_root }}:/var/lib/tftpboot:rw | ||
| mode: '0644' | ||
| owner: root | ||
| group: root |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ | |
| sdnotify: true | ||
| network: host | ||
| hostname: "{{ ansible_facts['hostname'] }}.local" | ||
| security_opt: | ||
| - "label=disable" | ||
|
Comment on lines
+19
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the only way to make the container read, write, and list data in the directory. |
||
| secrets: | ||
| - 'foreman-proxy-settings-yml,type=mount,target=/etc/foreman-proxy/settings.yml' | ||
| - 'foreman-proxy-ssl-ca,type=mount,target=/etc/foreman-proxy/ssl_ca.pem' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| :enabled: {{ feature_enabled }} | ||
| :tftproot: /var/lib/tftpboot | ||
| :tftp_servername: {{ foreman_proxy_tftp_servername }} | ||
|
stejskalleos marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| SSH_CONFIG = './.tmp/ssh-config' | ||
| OBSAH_STATE = os.environ.get('OBSAH_STATE', '.var/lib/foremanctl') | ||
| PARAMETERS_FILE = os.path.join(OBSAH_STATE, 'parameters.yaml') | ||
| FOREMAN_PROXY_PORT = 8443 | ||
|
|
||
|
|
||
| class UserParameters: | ||
|
|
@@ -271,6 +272,26 @@ def get_connection_with_tls_context(self, request, verify, proxies=None, cert=No | |
| return conn | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def proxy_request(server, certificates, server_fqdn): | ||
|
stejskalleos marked this conversation as resolved.
|
||
| def _request(path, method=None, data=None, return_body=False): | ||
| curl_opts = ( | ||
| f"--cacert {certificates['server_ca_certificate']} " | ||
| f"--cert {certificates['client_certificate']} " | ||
| f"--key {certificates['client_key']} " | ||
| f"--silent " | ||
| ) | ||
| if not return_body: | ||
| curl_opts += "--output /dev/null --write-out '%{http_code}' " | ||
| if method: | ||
| curl_opts += f"-X {method} " | ||
| if data: | ||
| curl_opts += f"-d '{data}' " | ||
| return server.run(f"curl {curl_opts}https://{server_fqdn}:{FOREMAN_PROXY_PORT}/{path}") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried this manually, and whenever I invoke this API, it creates the directory structure on the Foreman side rather than on the TFTP server. Additionally, when I run the Could you elaborate on the expected workflow here and how this test is supposed to work? Am I missing a step that synchronizes or publishes the generated content to the TFTP server?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is nothing else needed; just replicate the test steps, and it should work. The CI is passing, so it's working.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The core design is that it will be created on the Foreman Proxy side (which may also run a Foreman). It's expected that the admin makes sure the TFTP server also uses the same directory. For example, using NFS as done in https://docs.theforeman.org/3.19/Integrating_Provisioning_Infrastructure_Services/index-foreman-el.html#integrating-a-generic-tftp-server. Ideally we'd have an accompanying docs PR that explains the end-to-end workflow. |
||
|
|
||
| return _request | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def local_request(ssh_config, server_fqdn): | ||
| session = requests.Session() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.