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

Don't parse files other than *.conf on Debian 8 #4523

Closed
jmorahan opened this issue Apr 18, 2017 · 3 comments
Closed

Don't parse files other than *.conf on Debian 8 #4523

jmorahan opened this issue Apr 18, 2017 · 3 comments

Comments

@jmorahan
Copy link

My operating system is (include version):

Debian 8 (I expect Ubuntu 14.04 and later will also be affected)

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

Probably OS package manager (I don't know for sure because I'm filing this bug report based on someone else's post in the community forum)

I ran this command and it produced this output:

sudo certbot --apache
The apache plugin is not working; there may be problems with your existing configuration.
The error was: PluginError(('There has been an error in parsing the file (%s): %s', '/etc/apache2/sites-available/000-default.conf.save', 'Syntax error'),)

Certbot's behavior differed from what I expected because:

Certbot shouldn't have tried to parse that file at all, because Apache only includes files matching *.conf by default on Debian 8.

@jmorahan
Copy link
Author

@SwartzCr
Copy link
Contributor

@joohoi any thoughts?

@SwartzCr
Copy link
Contributor

I'm going to assign this to @joohoi, and he can make a determination about whether this is a legitimate issue when he starts working on apache reliability

@bmw bmw closed this as completed in #4104 Sep 25, 2017
bmw pushed a commit that referenced this issue Sep 25, 2017
…on Debian / Ubuntu (#4104)

This changes the apache plugin behaviour to only parse enabled configuration files and respecting the --apache-vhost-root CLI parameter for new SSL vhost creation. If --apache-vhost-root isn't defined, or doesn't exist, the SSL vhost will be created to originating non-SSL vhost directory.

This PR also implements actual check for vhost enabled state, and makes sure parser.parse_file() does not discard changes in Augeas DOM, by doing an autosave.

Also handles enabling the new SSL vhost, if it's on a path that's not parsed by Apache.

Fixes: #1328
Fixes: #3545
Fixes: #3791
Fixes: #4523
Fixes: #4837
Fixes: #4905

* First changes

* Handle rest of the errors

* Test fixes

* Final fixes

* Make parse_files accessible and fix linter problems

* Activate vhost at later time

* Cleanup

* Add a new test case, and fix old

* Enable site later in deploy_cert

* Make apache-conf-test default dummy configuration enabled

* Remove is_sites_available as obsolete

* Cleanup

* Brought back conditional vhost_path parsing

* Parenthesis

* Fix merge leftovers

* Fix to work with the recent changes to new file creation

* Added fix and tests for non-symlink vhost in sites-enabled

* Made vhostroot parameter for ApacheParser optional, and removed extra_path

* Respect vhost-root, and add Include statements to root configuration if needed

* Fixed site enabling order to prevent apache restart error while enabling mod_ssl

* Don't exclude Ubuntu / Debian vhost-root cli argument

* Changed the SSL vhost directory selection priority

* Requested fixes for paths and vhost discovery

* Make sure the Augeas DOM is written to disk before loading new files

* Actual checking for if the file is parsed within existing Apache configuration

* Fix the order of dummy SSL directives addition and enabling modules

* Restructured site_enabled checks

* Enabling vhost correctly for non-debian systems
@bmw bmw added this to the 0.19.0 milestone Sep 25, 2017
sydneyli pushed a commit that referenced this issue Jun 15, 2018
…on Debian / Ubuntu (#4104)

This changes the apache plugin behaviour to only parse enabled configuration files and respecting the --apache-vhost-root CLI parameter for new SSL vhost creation. If --apache-vhost-root isn't defined, or doesn't exist, the SSL vhost will be created to originating non-SSL vhost directory.

This PR also implements actual check for vhost enabled state, and makes sure parser.parse_file() does not discard changes in Augeas DOM, by doing an autosave.

Also handles enabling the new SSL vhost, if it's on a path that's not parsed by Apache.

Fixes: #1328
Fixes: #3545
Fixes: #3791
Fixes: #4523
Fixes: #4837
Fixes: #4905

* First changes

* Handle rest of the errors

* Test fixes

* Final fixes

* Make parse_files accessible and fix linter problems

* Activate vhost at later time

* Cleanup

* Add a new test case, and fix old

* Enable site later in deploy_cert

* Make apache-conf-test default dummy configuration enabled

* Remove is_sites_available as obsolete

* Cleanup

* Brought back conditional vhost_path parsing

* Parenthesis

* Fix merge leftovers

* Fix to work with the recent changes to new file creation

* Added fix and tests for non-symlink vhost in sites-enabled

* Made vhostroot parameter for ApacheParser optional, and removed extra_path

* Respect vhost-root, and add Include statements to root configuration if needed

* Fixed site enabling order to prevent apache restart error while enabling mod_ssl

* Don't exclude Ubuntu / Debian vhost-root cli argument

* Changed the SSL vhost directory selection priority

* Requested fixes for paths and vhost discovery

* Make sure the Augeas DOM is written to disk before loading new files

* Actual checking for if the file is parsed within existing Apache configuration

* Fix the order of dummy SSL directives addition and enabling modules

* Restructured site_enabled checks

* Enabling vhost correctly for non-debian systems
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