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 extending nginx config #1501

Closed
rthideaway opened this issue Mar 20, 2019 · 5 comments
Closed

Allow extending nginx config #1501

rthideaway opened this issue Mar 20, 2019 · 5 comments
Milestone

Comments

@rthideaway
Copy link

Would be cool if nginx conf could be extended instead of just replacing it. The possible solution could be the same as in lagoon platform -> https://github.com/amazeeio/lagoon/blob/master/images/nginx-drupal/drupal.conf

The key is to add the similar line in the server directive:
include /etc/nginx/conf.d/ddev/server_prepend*.conf;

So a developer would just create a partial configs which would be copied during docker build.

Use case. For example we have drupal private folder inside the public folder under sites/default/files/private. We obviously want to protect the private files. Ideally we would just create an nginx conf file like server_prepend_protect_private_files.conf with the content:

    location /sites/default/files/private/ {
        internal;
    }

And that's it. The default nginx config is extend with this part and our private files are protected. With this users can have much more control over their nginx configs instead of fully replacing them.

@rfay
Copy link
Member

rfay commented Mar 20, 2019

Yeah, I've always thought this would be a good idea. It seems quite unusual for people to edit the nginx config though. What is your need to extend it?

@rthideaway
Copy link
Author

rthideaway commented Mar 20, 2019

Our case is actually already described, we need to protect private files, which are inside public files folder (under /sites/default/files/private/). With current solution I had to copy whole d8 nginx conf and place additional lines inside server block:

    location /sites/default/files/private/ {
        internal;
    }

With an ability of extending inner server block out of the box I would just need to create another small file containing only that specific part:

    location /sites/default/files/private/ {
        internal;
    }

And it would be automatically embedded. We don't need to replace the whole config anymore because of one line we need. We would also benefit from that original nginx config even if it's updated in the future.

@rfay
Copy link
Member

rfay commented Mar 21, 2019

Thanks for explaining. In your case, why does this even matter for a local development environment?

And I'm sure you already know and have long known that private files should be nowhere that the webserver could even access them.

I definitely agree with your request, also, when this gets done, it's probably important to move the add-on configs into their own directory, .ddev/nginx, instead of having them in .ddev directly.

@rthideaway
Copy link
Author

Well, we are planning to use ddev on our development server for remote development which will be accessible from outside world, so the private files must be protected.

The reason why private files are there is, that we also use Lagoon for staging environment. Since they allow only one persistent folder (which is obviously files), we had to add the private folder inside public one, so private data are not lost on docker build (they persists across docker builds). And Lagoon is protecting private folder from outside world out of the box within their nginx conf. The same way as I posted - that location part.

Anyway, extending nginx conf is a good approach anyway. There will be much more use cases in the future. I can list another one - CORS headers configuration. Again on Lagoon this is extensible with one additional partial nginx file: https://github.com/drupal-graphql/drupal-decoupled-app/blob/master/backend/.lagoon/nginx-drupal-cors.conf

To sum it ip, we are trying to unify lagoon and ddev setup basically, that's it. And we also don't want to touch original ddev nginx conf.

@rfay
Copy link
Member

rfay commented Mar 21, 2019

Yes, I'm just pointing out that best practice for private files is for it to be completely outside the webroot, but it seems you have some constraints that way. I'm really surprised that Amazee hasn't supported private files in a more robust way. https://www.drupal.org/docs/8/core/modules/file/overview#content-accessing-private-files explains that

Whenever possible it's recommended that you choose a directory located outside of your Drupal root folder (or actually outside your web root), which may be tricky if you are on a shared host

Anyway, it would be worth talking with them about this, because it's important to do it right and not be dependent on webserver configuration.

But yes, I understand. I don't exactly know why it's important to protect it on ddev, but all is well.

Thanks!

@rfay rfay changed the title Allow extending of nginx config Allow extending nginx config Mar 27, 2019
@rfay rfay added this to the v1.8.0 milestone Apr 22, 2019
@rfay rfay closed this as completed in 97c660a Apr 29, 2019
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

No branches or pull requests

2 participants