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

Nginx plugin not able to get version number from openresty nginx binary #4919

Closed
phoenixgao opened this issue Jul 11, 2017 · 4 comments
Closed
Assignees
Milestone

Comments

@phoenixgao
Copy link

My operating system is (include version):

Ubuntu 16.04

I installed Certbot with (certbot-auto, OS package manager, pip, etc):

certbot-auto

I ran this command and it produced this output:

certbot-auto certonly --nginx

Error: invalid literal for int() with base 10: ''

This is because in the nginx configurator code:
https://github.com/certbot/certbot/blob/master/certbot-nginx/certbot_nginx/configurator.py#L649

it uses regex version_regex = re.compile(r"nginx/([0-9\.]*)", re.IGNORECASE) but the version number of openresty nginx is like nginx version: openresty/1.11.2.2

Certbot's behavior differed from what I expected because:

nginx plugin should support openresty or prompt it doesn't support

@SwartzCr
Copy link
Contributor

SwartzCr commented Jul 12, 2017

This seems like something @ohemorange should know about
Also thanks for the super thorough bug report!

@ohemorange
Copy link
Contributor

Copied from #4833 (comment):

I think the best thing to do here is to leave the regex as is and do the existing check if we find matches. But if we don't find a version string matching the official version format, we should print a warning saying that we don't officially support forks of Nginx, and continue. This is different from what we do now in that we currently error out if we can't find a version string.

@phoenixgao
Copy link
Author

@ohemorange Yeah for me both way works (fix or warning message instead of error)
but on the other hand, since some of the forks like tenginex and openresty are widely used, if certbot could support them, it will help more websites move to https, what do you think?

@ohemorange
Copy link
Contributor

We will "support" them by not automatically breaking when we encounter them. Guaranteeing that we'll handle all bugs that are caused by using them instead of the standard version is a much larger burden.

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

No branches or pull requests

4 participants