cloud-init
cloud-init copied to clipboard
Add ftp support to NoCloud
Proposed Commit Message
feat: Add support for FTP and FTP over TLS
Use Python's ftplib to connect to and read instance configurations from a
remove server. Like the current http support, nocloud's configuration URI
is defined via kernel command line or dmi product serial.
Notes on FTP and FTP over TLS
=============================
FTP is insecure, use at your own risk.
The FTP over TLS implementation follows library best practices[1][2].
From the ssl documentation regarding create_default_context():
> It will load the system’s trusted CA certificates, enable certificate
> validation and hostname checking, and try to choose reasonably secure
> protocol and cipher settings.
Test Changes
============
tests/i/util.py
---------------
- new helper function verify_clean_boot() to verify cloud-init state
- acquire override_kernel_cmdline() for reuse in other tests
- acquire restart_cloud_init() for reuse in other tests
tests/i/datasources/test_nocloud.py
-----------------------------------
- new test class for FTP/FTPS uses pyftpdlib for server, mkcert for test certs
- 2 test additions for FTP
- 2 test additions for FTPS
Implements via ftplib
=====================
RFC 959 - File Transfer Protocol
RFC 2640 - FTP Internationalization (UTF-8)
RFC 4217 - FTP over TLS
[1] https://docs.python.org/3/library/ftplib.html#ftp-tls-objects
[2] https://docs.python.org/3/library/ssl.html#ssl-security
Additional Context
Test Steps
PLATFORM="lxd_vm" CLOUD_INIT_SOURCE="./cloud-init_all.deb" python -m
pytest -vv --log-cli-level=INFO --durations 10 tests/integration_tests/datasources/test_nocloud.py::TestFTP
Merge type
- [x] Squash merge using "Proposed Commit Message"
- [ ] Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)
@holmanb this still WIP?
@holmanb this still WIP?
I'm working on an integration test for it currently, but I think it is ready for review.
@TheRealFalcon I think I've addressed your comments. I've added integration tests and updated the commit message.
Ready for re-review.
@TheRealFalcon I believe that I've addressed all of your comments.
I have one more review item left to address, which is the request to include a manually generated cert rather than installing mkcert from source for focal. I won't be able to work on that request until Friday at the earliest, so if you'd like to re-review the non-test code, I don't have any more planned changes there.
Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.
If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.
(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)
rebased and force pushed - addressed remaining comment from @TheRealFalcon and merging