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

Unit: Scripts located in /etc/unit/config.d completely overwrite the Unit configuration #451

Open
jaydrogers opened this issue Oct 2, 2024 · 10 comments
Assignees
Labels
🧐 Bug: Needs Confirmation Something isn't working, but needs to be confirmed by a team member.

Comments

@jaydrogers
Copy link
Member

Steps To Reproduce

TBD

Outcome

What did you expect?

  • Scripts should not overwrite the config

What happened instead?

  • The config was overwritten

Affected Docker Images

Unit images only

Anything else?

Originally reported by @TheAndrey in #412:

Any scripts located in /etc/unit/config.d completely overwrite the Unit configuration. To avoid this, we need to be able to specify the URI where the configuration file will be uploaded, as described in the documentation.

@jaydrogers jaydrogers added the 🧐 Bug: Needs Confirmation Something isn't working, but needs to be confirmed by a team member. label Oct 2, 2024
@jaydrogers jaydrogers self-assigned this Oct 2, 2024
@jaydrogers
Copy link
Member Author

I used the logic from the official images:

https://github.com/nginx/unit/blob/624debcf17ea7faab01fa841bd4dcd9f308cf306/pkg/docker/docker-entrypoint.sh#L58-L62

To my understanding, we want this to happen because I am placing a JSON file for the config here:

process_template "$UNIT_CONFIG_DIRECTORY/ssl-$ssl_mode.json.template" "$UNIT_CONFIG_DIRECTORY/config.json"

@TheAndrey: If you have any thoughts or feedback, I would love to hear it.

@jaydrogers jaydrogers moved this to In review in serversideup/php v3.4 Oct 2, 2024
@TheAndrey
Copy link

TheAndrey commented Oct 2, 2024

I decided not to continue using unit any further. Its main disadvantage is it's very limited configuration capabilities - there is no support for nested locations and rewrite.

Unit was convenient only because everything works within a single process.

@jaydrogers
Copy link
Member Author

Thanks for sharing your feedback!

I feel kind of the same way. It's been more difficult to use than I expected.

I am eager to give FrankenPHP a try and hoping to find a single process container to make managing PHP much easier 😃

Still lot's of work to do to the PR below. Stay tuned!

@kylecotter
Copy link

Just chiming in here, ran into a similar situation while playing with Unit.

Before I saw UNIT_MAX_BODY_SIZE was going to be added as an env variable, I had a json file that contained

{
    "settings": {
        "http": {
            "max_body_size": 104857600
        }
    }
}

I assumed dropping the script in /etc/unit/config.d would append this to the config. But rather, it completely replaces the configuration since it’s PUT’ing to http://localhost/config/ every time.

The solution in this example would require my JSON file to be

"http": {
    "max_body_size": 104857600
}

and PUT’ing that to http://localhost/config/settings/.

I love the appeal of Unit and everything being a single process. Coupled with something like Cloudflare I think you can get pretty close to what you would have with Nginx + PHP-FPM. I think you can deal with some of Unit's limitations higher up the chain.

I think something like Unit or FrankenPHP is the future. Happy to not deal with PHP-FPM configurations anymore.

Thanks for all your work on these!

@jaydrogers
Copy link
Member Author

Thanks a ton for chiming in as we learn this stuff together!

I think something like Unit or FrankenPHP is the future. Happy to not deal with PHP-FPM configurations anymore.

I totally agree with this. I am stoked to get 3.4 out the door because it's finally the release where FPM does what it needs to do and gets out of the way when we don't need it.

I'll leave this issue open for a future release and will see how things pan out with FrankenPHP. I will be so stoked to put FPM on the shelf someday 😃

@jpangborn
Copy link
Contributor

I would also like a method for changing the Unit configuration when necessary. In my application, I have a handful of routes that I need to add. I current have a full Unit configuration file based on the ssl-off.json.template and am copying that into /etc/unit/config.d. In the end, I have the customized configuration, but if there are any changes to base template in a future version, I would need to manually update.

As @kylecotter said above, Unit provides methods for updating a portion of the configuration, but you will need more than simply processing all of the files in a folder, as you need to know the proper config URI and HTTP method in order to avoid overwriting the entire config.

These types of changes may better handled with an entry point script that can apply the custom configuration changes after the default configuration is set. I'll attempt to move my changes to that method to see how complicated it is.

@jaydrogers
Copy link
Member Author

I am all for any improvements that the community sees fit. My goal would be to get it to work as much as our fpm-nginx image as much as possible.

In regards to the original part of this issue, this behavior was pulled from the default NGINX Unit image. I am all for refactoring this into a better DX if possible.

@hookenz
Copy link

hookenz commented Oct 17, 2024

I ended up create a completely new config.json and overwrite the default. It's the only way I could turn off the default behavior of having php route every call through index.php which is what you'd want with laravel etc.

I tried creating a custom image with FrankenPHP but ran into other difficulties with Franken and my app so went back to this.
Unit just has better performance than nginx + php-fpm but is missing some things you can do with nginx. If I had more time I might give Franken another shot.

None of them are quite on par feature wise. I'm sticking with unit for now.

@jaydrogers
Copy link
Member Author

I tried creating a custom image with FrankenPHP but ran into other difficulties with Franken and my app so went back to this.

Thanks for the feedback! I am thankful to hear some real world experience 🙌

@hookenz
Copy link

hookenz commented Dec 11, 2024

@TheAndrey I've abandoned nginx unit as well. It's good but limited as you say.
I managed to get FrankenPHP running and it's heaps better but I'm using a bit of a messy custom image and look forward to the franken branch being merged into main and released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧐 Bug: Needs Confirmation Something isn't working, but needs to be confirmed by a team member.
Projects
None yet
Development

No branches or pull requests

5 participants