delfer / docker-alpine-ftp-server

Small and flexible docker image with vsftpd server
189 stars 130 forks source link

Set configuration parameters by variables #45

Open damwiw opened 2 years ago

damwiw commented 2 years ago

It's a proposal to set parameters from vsftp.conf file by variables. This can be used to solve issues https://github.com/delfer/docker-alpine-ftp-server/issues/39 and https://github.com/delfer/docker-alpine-ftp-server/issues/19. Feel free to comment.

delfer commented 12 months ago

Hello @damwiw ! Thank you very much for this PR! You made a great work! You implement something like "scripting", where we can perform actions like "comment out", "uncomment" and "set" different values by configuration file. This is not the way how this project made. It wold be better to have definitions instead of actions. For example: "Parameter A should have value B" instead of "Set parameter A to B" "Parameter A should be commented out" instead of "Comment out parameter A" This is can bring the idempotency to the project. I personally want to make algorithm where we can add env value with prefix like CONF_ and the the name of the configuration parameter and then in the startvsftpd.sh execute env command, grep all variables with `CONF` prefix, take parameter name and value, then find specified parameter in vsftpd.conf, uncomment if this value was commented out, and change value. This will be idempotent because this action can be repeated multiple times with the same resulting file. But I have not enough time for this. Is it possible to change the logic?

damwiw commented 12 months ago

Hi @delfer,

Thanks for those positive feedbacks.

It's doable to change the logic for setting parameter values, with things like CONF_FTPD_BANNER=my specific banner.

Now, we must consider parameters we just want to comment/uncomment, without no change to the default value (this is one of my requirements).

I can see 3 options: 1) environment variables such as COMMENT_PASV_ENABLE or UNCOMMENT_CHROOT_LIST_FILE 2) CONF_PASV_ENABLE=COMMENT|UNCOMMENT|value, but I don't like much the idea to mix actions and values. 3) CONF_PASV_ENABLE=COMMENT|UNCOMMENT|VALUE:

This is open to discussion!

delfer commented 11 months ago

@damwiw hi! I'm very glad to see you reply! Thank you! Nice to know that we found same opinion about setting values (CONF_FTPD_BANNER). But I'm scare a little bit about comment/uncomment options, because it can cause unpredictable behavior. For example:

  1. We write docker-compose file to deploy alpine-ftp-server:latest with the variable UNCOMMENT_CHROOT_LIST_FILE=yes
  2. The image was build 2023-11-08 with line #chroot_list_file=/etc/vsftpd.chroot_list in vsftpd.conf inside the image
  3. In new image 2023-11-09 vsftpd.conf was changed and now this line become #chroot_list_file=/etc/vsftpd/vsftpd.chroot_list
  4. Deployment broken

I thins if someone want to change the value he should provide exact value, instead of asking to use default value. @damwiw is it possible to split this PR for to part, first about setting values and second about comment/uncomment values. I very like your idea to change configuration using enviroment variables, but idea with commenting/uncommenting needs additional discussion.