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

Respect changes in the existing nginx.conf #10

Closed
motin opened this issue May 19, 2014 · 9 comments
Closed

Respect changes in the existing nginx.conf #10

motin opened this issue May 19, 2014 · 9 comments

Comments

@motin
Copy link

motin commented May 19, 2014

This plugin does not seem to respect the current nginx.conf. If another plugin modifies the app's nginx.conf, those changes are not propagated to the custom domains nginx configuration.

A better approach could be to modify the nginx.conf server_name row using sed during the nginx-pre-reload hook.

@motin motin changed the title Don't ignore other plugins' changes to nginx.conf Support SSL May 19, 2014
@motin motin changed the title Support SSL Respect changes in the existing nginx.conf May 19, 2014
@wmluke
Copy link
Owner

wmluke commented May 21, 2014

Hi @motin,

Can you give me an example of some attributes of nginx.conf that are clobbered by the domains vhost config?

Thanks,
Luke

@motin
Copy link
Author

motin commented May 21, 2014

Can you give me an example of some attributes of nginx.conf that are clobbered by the domains vhost config?

Sorry, I don't understand the question.

A better approach could be to modify the nginx.conf server_name row using sed during the nginx-pre-reload hook.

Btw, I sent a pr with the plugin rewritten to use this approach: #11

@wmluke
Copy link
Owner

wmluke commented May 21, 2014

You indicated that some changes to nginx.conf are not propagated. Which changes and by which other plugins? I'd like to understand the scope of the issue.

Thanks again,
Luke

@motin
Copy link
Author

motin commented May 21, 2014

You indicated that some changes to nginx.conf are not propagated.

Exactly.

Which changes?

All changes made by plugins whose names happen to come after "dokku-domains-plugin" alphabetically, when this plugin has already replaced nginx.conf with it's own version.

and by which other plugins?

Currently the only plugin I know of is https://github.com/neam/dokku-nginx-vhosts-custom-configuration but the implication of this issue is that no other plugins will be developed if the hooks are not sharable amonst plugins. Dokku has chosen pluginhook, and one out of three important aspects is that "Multiple plugins can handle a hook".

@wmluke
Copy link
Owner

wmluke commented May 22, 2014

Hi @motin,

To be clear dokku-domains-plugin does not overwrite or replace $DOKKU_ROOT/$APP/nginx.conf (nginx.conf). This plugin merely creates new vhost files for TLD's not listed in nginx.conf. These new vhosts reside in $DOKKU_ROOT/$APP/nginx-domains.conf (nginx-domains.conf). Both nginx.conf and nginx-domains.conf are loaded. This strategy composes well b/c folks can add vhosts for new TLD without interfering with nginx.conf. The only time there could be an issue is if someone adds the same TLD to both nginx.conf and nginx-domains.conf, but I feel that would be a misconfiguration (I may added some error checking to prevent this).

Furthermore, adding differing TLD's to nginx.conf as you propose could interfere with dokku's SSL support b/c dokku assumes a a single SSL cert for a sole TLD and subdomains. For example, it would be an issue if your cert is for *.foo.com but you've added foo.com and bar.com to nginx.conf.

Anyway, I appreciate your feedback and passion, but I don't feel that this is an issue for dokku-domains-plugin. Best of luck with dokku-nginx-vhosts-custom-configuration.

Cheers,
Luke

@wmluke wmluke closed this as completed May 22, 2014
@motin
Copy link
Author

motin commented May 22, 2014

To be clear dokku-domains-plugin does not overwrite or replace $DOKKU_ROOT/$APP/nginx.conf (nginx.conf). This plugin merely creates new vhost files for TLD's not listed in nginx.conf. These new vhosts reside in $DOKKU_ROOT/$APP/nginx-domains.conf (nginx-domains.conf). Both nginx.conf and nginx-domains.conf are loaded. This strategy composes well b/c folks can add vhosts for new TLD without interfering with nginx.conf.

Exactly, and because of this, If another plugin modifies the app's nginx.conf, those changes are not propagated to the custom domains nginx configuration (nginx-domains.conf).

This means that the configuration ends up being either similar but different - a catastrophic situation - between the app-name.dokkuhost.com and appname.com, so all manual and automated acceptance tests have to run again against both domains in order to verify that it all works.

Or, it can end up with the configuration being simply vastly different from each other, if one is using another plugin that has added custom configuration to nginx.conf.

Relevant use cases for when the nginx vhost configuration needs to be customized can be to set proxy timeouts in order to allow long running requests, setting specific SSL directives, enabled uploading of large files and the like. Having to maintain two different set-ups of nginx vhost configuration adds to the maintenance costs and provides more sources for error.

If you add a TLD to your heroko app, you can count on the fact that it will work in the same manner as the appname.heroku.com domain.

How btw does a user set these custom nginx directives manually or in his/her own plugin to additional domains - it seems that it is not supported at all. The only ways with this plugin to add, say client_max_body_size 100M; to a TLD would be to make dokku-nginx-vhosts-custom-configuration also modify the nginx-domains.conf, or for this plugin to include app-specific nginx configuration files.

Creating these extra set of dependencies and require devops or plugin authors to manually synchronize changes between different configuration set-ups is imo a bad thing. Remember the Unix philosophy: "Write programs that do one thing and do it well. Write programs to work together."

Furthermore, adding differing TLD's to nginx.conf as you propose could interfere with dokku's SSL support b/c dokku assumes a a single SSL cert for a sole TLD and subdomains. For example, it would be an issue if your cert is for *.foo.com but you've added foo.com and bar.com to nginx.conf.

Agree. I haven't set-up proper SSL support in #11 , but that is a different issue than the one we are discussing here. I'll reply to this comment in #11 since I believe you simply commented in the wrong thread.

Anyway, I appreciate your feedback and passion, but I don't feel that this is an issue for dokku-domains-plugin. Best of luck with dokku-nginx-vhosts-custom-configuration.

Closing this based on the arguments put forth until now clearly shows that there is a great misunderstanding going on. This issue is still unsolved and clearly affects the usability of the plugin in a larger context.

@wmluke
Copy link
Owner

wmluke commented May 22, 2014

My sense is that you're taking on too much scope...both for how these plugins work with each other and what you want from dokku.

My sense of dokku is that it is a minimalistic PaaS. And just b/c dokku is modeled after heroku, does not mean that it should mimic all of heroku's behaviors. As indicated in the dokku docs, if someone wants this much control over their vhosts, then they can (a) look at PaaS alternatives such as flynn, (b) manage their vhosts manually, which isn't terribly hard, or (c) use dokku-nginx-vhosts-custom-configuration.

My sense of dokku-nginx-vhosts-custom-configuration is that it can work reasonably well without dokku-domains-plugin. The dokku community is bright enough to make sense of plugin fragmentation, which occurs in most open source communities.

Thanks again for the discussion,
Luke

@motin
Copy link
Author

motin commented May 23, 2014

My sense is that you're taking on too much scope...both for how these plugins work with each other and what you want from dokku.

My sense of dokku is that it is a minimalistic PaaS.

Agree. Dokku and it's plugins should remain minimalistic in their implementations. Compare the complexity of this plugin and it's readme before and after #11 - which one is more minimalistic?

And just b/c dokku is modeled after heroku, does not mean that it should mimic all of heroku's behaviors. As indicated in the dokku docs, if someone wants this much control over their vhosts, then they can (a) look at PaaS alternatives such as flynn, (b) manage their vhosts manually, which isn't terribly hard, or (c) use dokku-nginx-vhosts-custom-configuration.

This doesn't make sense. Yes, (a) flynn could be an alternative down the road, but (b) managing vhosts manually is not an option and (c) the whole point of this issue (#10) is to be able to use dokku-nginx-vhosts-custom-configuration AND dokku-domains-plugin at the same time, which is not possible.

My sense of dokku-nginx-vhosts-custom-configuration is that it can work reasonably well without dokku-domains-plugin.

To be clear, these are NOT alternatives to each other. They complement each other. dokku-nginx-vhosts-custom-configuration allows for custom configuration directives but can't add domain aliases to server_name, and dokku-domains-plugin can add custom domains but not custom configuration directives.

The dokku community is bright enough to make sense of plugin fragmentation, which occurs in most open source communities.

Yes it occurs and yes the community often manages but plugin fragmentation is not something to strive for and should be avoided when possible.

Thanks again for the discussion,

I see you are not interested in reconsidering my points above and do not agree with me. Since the first comment on #11 was "this looks awesome! nice job @motin", I still feel that someone else would appreciate the alternative approach, so I'll fork and publish my own version of the plugin, and our future efforts will be fragmented.

@motin
Copy link
Author

motin commented May 23, 2014

For anyone interested, I released the fork earlier this morning: https://github.com/neam/dokku-custom-domains

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