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

Add BP_WEB_SERVER_LOCATION_INCLUDES to Enable Custom Location Directives #668

Open
ChuckQuinnIV opened this issue Jan 26, 2024 · 3 comments
Labels
enhancement A new feature or request

Comments

@ChuckQuinnIV
Copy link

Describe the Enhancement

Add a new optional environment variable, BP_WEB_SERVER_LOCATION_INCLUDES (or similar) to allow users to provide location-scoped NGINX directives while taking advantage of the generated nginx.conf file.

Motivation

Users migrating from buildpacks like Cloud Foundry's Staticfile buildpack commonly only customize server root, push state, and location directives. Currently, users can use this buildpack with a generated nginx.conf using BP_WEB_SERVER_ENABLE_PUSH_STATE and BP_WEB_SERVER_ROOT for the first two features, but must maintain their own nginx.conf if they need any custom location directives. This proposed feature will remove the need for these users to eject from the generated nginx.conf.

@sophiewigmore
Copy link
Member

@paketo-buildpacks/web-servers-maintainers

@arjun024
Copy link
Member

I'd be open to a configuration that allows an include inside the default location block. PRs welcome

@lnhrdt
Copy link

lnhrdt commented Nov 11, 2024

I've also been wishing for something like this for a while. I'm not a Go developer but I'm motivated enough that I'm open to working with the tests to submit a PR. However I think it would be a good idea to discuss an approach first, @arjun024. (And in the event a discussion here prioritizes this for the Paketo team to work on it, I'd be happy to not learn Go just to contribute a PR here. 😄)

My typical use case for this nginx buildpack is containerizing a client side React application. The BP_WEB_SERVER_ENABLE_PUSH_STATE configuration gets close to what I need, but because I need to adjust the caching of assets (bundled hash assets should cache "forever" while index.html should "never" cache) I end up needing to "eject" from the provided nginx.conf. My usual process is 1) navigate to this repo 2) download the default.conf file 3) manually replace the templating with my values 4) replace the entire provided location block with my own 5) commit it to my application's repository. The only part that I'm actually customizing is the location block.

I don't love this process because now I have to care about the whole nginx.conf file when I wish I could just rely on paketo's smart defaults and if the project updated the default config I wouldn't automatically benefit. I'd like to get back to a more proper "buildpack experience" with regards to the nginx.conf concerns.

So perhaps this idea for including custom location directives could be the solution?

I have a few questions and would like to hear your opinion:

  • Should location includes completely replace the existing location directive? I don't think it would work for my use case if it didn't replace, but I haven't completely thought it through.
  • Should it be accepted that the nested WebServerEnablePushState configuration under the existing location directive would be incompatible with using custom location directives? Is that reasonable behavior for a user?
  • Assuming a BP_WEB_SERVER_LOCATION_INCLUDES environment variable as suggested by @ChuckQuinnIV, should it accept paths to files, directories, or both?
  • Instead of location directive includes, would better caching defaults or configuration options be a better solution for my use case and this project?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or request
Projects
Status: Not scoped
Development

No branches or pull requests

4 participants