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

(#5121) Clobber mod_ssl RPM package path for ssl.conf file on RHEL7 #1635

Closed
wants to merge 3 commits into from
Closed

(#5121) Clobber mod_ssl RPM package path for ssl.conf file on RHEL7 #1635

wants to merge 3 commits into from

Conversation

traylenator
Copy link
Contributor

@traylenator traylenator commented Jun 22, 2017

https://tickets.puppetlabs.com/browse/MODULES-5121

This reverts bf9f0d0 and allows mod::ssl to behave
like other modules again. In paticular with a configuration.

class{'apache':
    class { '::apache':
    confd_dir           => '/etc/httpd/conf.puppet.d',
    default_confd_files => false,
    default_mods        => false,
    default_vhost       => false,
    mod_dir             => '/etc/httpd/conf.modules.puppet.d',
    vhost_dir           => '/etc/httpd/conf.puppet.d',
}
class{'apache::mod::ssl':}

The ssl.conf location will still respect mod_dir parameter above.

In addition there are a couple of extra tests to catch this test case.

traylenator referenced this pull request Jun 22, 2017
This is solving a problem with the SSL configuration on a Red Hat-based
OS that results in a duplicate 'Listen 443' statement after a package
update, causing Apache to no longer start.

The mod_ssl packaging ships a default ssl.conf in apache's main conf
dir, that among other things, contains 'Listen 443'.

However, this module puts all the Listen statements in ports.conf
centralized.

Generally this is no problem, because the module would purge the conf
directory.  Apache hums along happily -- until the apache package gets
an update and it restores the default ssl.conf into
/etc/httpd/conf.d/ssl.conf as no such file exists by the name on EL7
(the module's ssl.conf goes into conf.modules.d).

Apache will then fail to start with this error until puppet runs again:

        Address already in use: AH00072: make_sock: could not bind to address 0.0.0.0:443

The RPM won't overwrite the file if it's changed, but it does put it
back when removed.  So to avoid this problem, this change moves the
ssl.conf to the Apache conf dir on EL7. That replaces the one created by
the RPM.

When the package is updated, it won't touch the ssl.conf and apache
will continue to work.
Check location where ssl.conf is written to and
test that it respects the `mod_dir` directive
to the apache class.
@stbenjam
Copy link
Contributor

Im not sure I agree with reverting my fix. You're reintroducing the bug where puppetlabs-apache config doesn't work after an upgrade. The module should work right out of the box. Your workaround to use an alternative mod dir seems to make sense but if you want ssl.conf in the mod dir you can override $ssl_file. You don't need to revert a commit to do that.

If you want ssl.conf always in the mod dir by default that's ok I guess and makes sense but the module should maybe create a dummy ssl.conf in the main dir so the packaging issue is avoided.

@traylenator
Copy link
Contributor Author

traylenator commented Jun 22, 2017

Hi, thanks for the response.

The following would work I guess to set ssl file back to the desired location but its a breaking
change.

class{'apache':
    class { '::apache':
    confd_dir           => '/etc/httpd/conf.puppet.d',
    default_confd_files => false,
    default_mods        => false,
    default_vhost       => false,
    mod_dir             => '/etc/httpd/conf.modules.puppet.d',
    vhost_dir           => '/etc/httpd/conf.puppet.d',
   ssl_file                => '/etc/httpd/conf.modules.puppet.d/ssl.conf'
}

But don't see why ssl should be special. Looking on CentOS 7 box with epel enabled
there are 148 files in packages that could be /etc/httpd/conf.d/

/etc/httpd/conf.d/sagator.conf
/etc/httpd/conf.d/shibboleth-ds.conf
/etc/httpd/conf.d/shib.conf
/etc/httpd/conf.d/smokeping.conf
/etc/httpd/conf.d/squid.conf
/etc/httpd/conf.d/squirrelmail.conf
/etc/httpd/conf.d/ssl.conf
.....

Anyone of these might contain something that beaks a puppet managed apache node. Best advice is to avoid the problem which is now harder because of the ssl patch.

@traylenator
Copy link
Contributor Author

Don't understand the test failiure.

@stbenjam
Copy link
Contributor

It's explained in the commit you're reverting In detail. ssl.conf in the package contains a duplicate listen statement. This module puts all port listen directives in ports.conf, so on package upgrade apache no longer starts. This is the only conflict case I know of with the default package files.

The mod ssl conf can get moved to mod_dir, if you create a dummy empty ssl.conf in conf_dir on el7.

@traylenator
Copy link
Contributor Author

The mod ssl conf can get moved to mod_dir, if you create a dummy empty ssl.conf in conf_dir on el7

This would be better and restore previous behaviour. Will add another comit.

This is solving a problem with the SSL configuration on a Red Hat-based
OS that results in a duplicate 'Listen 443' statement after a package
update, causing Apache to no longer start.

The mod_ssl packaging ships a default ssl.conf in apache's main conf
dir, that among other things, contains 'Listen 443'.

However, this module puts all the Listen statements in ports.conf
centralized.

Generally this is no problem, because the module would purge the conf
directory.  Apache hums along happily -- until the apache package gets
an update and it restores the default ssl.conf into
/etc/httpd/conf.d/ssl.conf as no such file exists by the name on EL7
(the module's ssl.conf goes into conf.modules.d).

Apache will then fail to start with this error until puppet runs again:

        Address already in use: AH00072: make_sock: could not bind to address 0.0.0.0:443

The RPM won't overwrite the file if it's changed, but it does put it
back when removed.  So to avoid this problem, this change moves the
ssl.conf to the Apache conf dir on EL7. That replaces the one created by
the RPM.

When the package is updated, it won't touch the ssl.conf and apache
will continue to work.

This is a reworking of patch originally contributed

bf9f0d0
@traylenator traylenator changed the title (#5121) Revert hard coding of ssl.conf file on RHEL7 (#5121) Clobber mod_ssl RPM package path for ssl.conf file on RHEL7 Jun 22, 2017
@traylenator
Copy link
Contributor Author

New commit now overwrites /etc/httpd/conf.d/ssl.conf with a trivial file on redhat unless that path is the actual configured path.
This addresses the original aim of bf9f0d0

Happy to rebase this into one commit rather than a revert and new?

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

Successfully merging this pull request may close these issues.

2 participants