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

Allow location of nginx.conf to be configurable by environment variable #358

Closed
Tracked by #485
robdimsdale opened this issue Mar 22, 2022 · 3 comments · Fixed by #373
Closed
Tracked by #485

Allow location of nginx.conf to be configurable by environment variable #358

robdimsdale opened this issue Mar 22, 2022 · 3 comments · Fixed by #373
Assignees

Comments

@robdimsdale
Copy link
Member

Currently the location of nginx.conf is hard-coded. This buildpack would be more resilient and flexible if this were configurable.

There are a couple of use-cases for this:

  • Upstream buildpacks (e.g. php-nginx) can instruct this buildpack as to where it can find the nginx config file that was created earlier in the build process
  • Application developers can instruct this buildpack as to where their nginx configuration file is, if it is not in the root of their application directory.

I propose the following configuration:

  • This buildpack optionally looks for the environment variable called BP_NGINX_CONF_LOCATION
  • If the environment variable is unset, fall back to the default location of /workspace/nginx.conf
  • If the environment variable is set to a relative filepath, assume that it is relative to /workspace/
  • If the environment variable is set to an absolute filepath, use it as-is (i.e. relative to /

I am happy to move this to an RFC if that process is preferred.

@sophiewigmore
Copy link
Member

We're looking for any input that the @paketo-buildpacks/web-servers-maintainers may have on this

@arjun024
Copy link
Member

Proposal Sounds good to me.

  • There was a related discussion about using bindings to provide nginx.conf (Including Service Bindings for nginx.conf and mime.types #281 (comment)) - not sure if you have seen this or if it will provide any extra input but worth checking out if you haven't.

  • Also, I have heard community interest about getting nginx buildpack to work on the tiny stack (e.g.). It would be nice to think if we have a desire for future tiny support whether using more environment variables would be tricky without a shell (direct process with configure binary running as exec.d). It's probably fine but worth checking out.

None of these are blockers. Just wanted to do a quick brain dump in case if it's helpful

@TisVictress
Copy link
Contributor

@arjun024 There is currently a PR (#343) that makes the configure binary run as exec.d. I don't think using environment variables would create an issue because we should be able to use them within the configure binary at launch.

@robdimsdale I like the name you proposed, BP_NGINX_CONF_LOCATION, and it make sense to have a default value.

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

Successfully merging a pull request may close this issue.

4 participants