Skip to content
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

Set configuration parameters by variables #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

damwiw
Copy link

@damwiw damwiw commented Sep 12, 2022

It's a proposal to set parameters from vsftp.conf file by variables.
This can be used to solve issues #39 and #19.
Feel free to comment.

@delfer
Copy link
Owner

delfer commented Oct 30, 2023

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 start_vsftpd.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
Copy link
Author

damwiw commented Oct 31, 2023

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
Copy link
Owner

delfer commented Nov 8, 2023

@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.

@damwiw
Copy link
Author

damwiw commented Dec 1, 2024

Hi @delfer,

As my last commit was really old, I rebased the branch and updated the behaviour of my patch.

Now you can only set a configuration parameter with something like CONF_=foo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants