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

Support for FreeBSD and few other features (reworked PR #264). #342

Merged
merged 1 commit into from
Nov 11, 2013

Conversation

ptomulik
Copy link
Contributor

@ptomulik ptomulik commented Sep 1, 2013

This is a successor of PR #264. If it is good to merge, the #264 should be closed.

This is quite big set of changes. I've rebased it, so it currently merges nicely to master.

I've merged three other commits from related pull requests: #341, #271, #304.

I have ran few tests on FreeBSD, checked with prefork, itk, event and worker mpms, tried with php module, at least these configurations worked.

@@ -45,19 +46,32 @@
$ports_file = $apache::params::ports_file,
) inherits apache::params {

package { 'httpd':
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to line 72

@@ -1,6 +1,8 @@
require 'spec_helper_system'

describe 'apache::mod::php class' do
when 'FreeBSD'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woops, error when merging.

@blkperl
Copy link
Contributor

blkperl commented Sep 5, 2013

So I merged #304, since #341 is so FreeBSD specific lets just keep it in this PR. #271 hasn't fixed their PR so we can just take your changes. If you get this rebased, I'll try to review it and get it merged.

@@ -54,6 +55,7 @@ The defaults are determined by your operating system (e.g. Debian systems have o
```puppet
class { 'apache':
default_mods => false,
default_confd_files => false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you line up the arrows?

@ptomulik
Copy link
Contributor Author

ptomulik commented Sep 5, 2013

rebasing in progress .. :)

@ptomulik
Copy link
Contributor Author

ptomulik commented Sep 5, 2013

rebased

@ptomulik
Copy link
Contributor Author

ptomulik commented Sep 6, 2013

Ok, I think it's ready for review.

@blkperl
Copy link
Contributor

blkperl commented Sep 15, 2013

I've created an issue on rspec-system to get a prefab image for FreeBSD.

@ptomulik
Copy link
Contributor Author

That's cool! Thanks!


$valid_mpms_re = $::osfamily ? {
'FreeBSD' => '(event|itk|peruser|prefork|worker)',
'Debian' => '(itk|prefork|worker)',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure that event is supported on Debuntu as well…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it should be easy to enable it in puppetlabs-apache (maybe as separate PR?)

@blkperl
Copy link
Contributor

blkperl commented Oct 1, 2013

@ptomulik Think you can submit all the non FreeBSD specific stuff in separate pull requests? It should make this PR easier to merge. We really need test images for FreeBSD to be positive that any of this will work and be supportable in future releases... unfortunately that seems to be our blocker at the moment.

@blkperl
Copy link
Contributor

blkperl commented Oct 1, 2013

puppetlabs/rspec-system/issues/52

@ptomulik
Copy link
Contributor Author

ptomulik commented Nov 4, 2013

I tried to extract non FreeBSD specific stuff but I always endup with changesets that do not add any value to the current functionality (until FreeBSD specific parts are reintroduced). Nevertheless, I'll leave few PRs just for case - some of them might be taken and extended to OSes other than FreeBSD.

ptomulik added a commit to ptomulik/puppetlabs-apache that referenced this pull request Nov 7, 2013
On some OSes (FreeBSD) apxs tool is used to put LoadModule directives
into httpd.conf during apache package (and apache modules)
insallation/reinstallation. The apxs expects some LoadModule directives
to be already present in httpd.conf (they may be commented-out) in order
to decide where to put its own directives.

This PR puts fake LoadModule directive (commented out) to httpd.conf.
The $apxs_workaround boolean parameter in apache class decides, whether
to use this workaround or not. This is used on FreeBSD, where apxs is
used by ports package provider (and perhaps all other). Without this,
the apache installation/reinstallation/deinstallation as well as
installation of additional modules would fail.

This PR was created in order to split puppetlabs#342 into smaller parts to make
review process easier, see
puppetlabs#342 (comment)
@apenney
Copy link
Contributor

apenney commented Nov 7, 2013

At some point I should get you to rebase this again master (after the server_root thing merges) so we can see where we stand now. :)

@ptomulik
Copy link
Contributor Author

ptomulik commented Nov 8, 2013

No problem :)

ptomulik added a commit to ptomulik/puppetlabs-apache that referenced this pull request Nov 8, 2013
Previously, the $httpd_dir was used as ServerRoot in httpd.conf
template. On some installations httpd_dir is not same as ServerRoot,
for example on FreeBSD apache22 (installed via ports) has ServerRoot set
to /usr/local by default.

This PR adds $server_root parameter to apache::params. Its purpose is to
be substituted in httpd.conf as ServerRoot. This parameter is used
in puppetlabs#342. The purpose of the whole thing is to plit puppetlabs#342 into smaller
parts to make review process easier, see
puppetlabs#342 (comment)
ptomulik added a commit to ptomulik/puppetlabs-apache that referenced this pull request Nov 8, 2013
Add $id, $path and $lib_path parameters to apache::mod. These parameters
are used in puppetlabs#342. The purpose of the whole thing is to plit puppetlabs#342 into
smaller parts to make review process easier, see
puppetlabs#342 (comment)

This PR covers all changes introduced by puppetlabs#271 to manifests/mod.pp. I
just don't have test/mod_load_params.pp as in puppetlabs#271.
ptomulik added a commit to ptomulik/puppetlabs-apache that referenced this pull request Nov 8, 2013
This changeset adds $mime_support_package and $mime_types_config
parameters to class `apache::mod::mime`. Their purpose is following:

- $mime_support_package - install this package, as it provides mime.conf
  file (definition of mime types used by mod_mime),
- $mime_types_config - used to substitute TypesConfig in
  mod/mime.conf.erb template.

This PR was created in order to plit puppetlabs#342 into smaller
parts to make review process easier, see
puppetlabs#342 (comment)
@ptomulik
Copy link
Contributor Author

ptomulik commented Nov 8, 2013

Rebased against master.

@blkperl
Copy link
Contributor

blkperl commented Nov 8, 2013

Doesn't look like it. Are you sure you pulled the latest master before rebasing?

@ptomulik
Copy link
Contributor Author

ptomulik commented Nov 8, 2013

Heh, as I said I rebased it (but forgot to push) :).

@apenney
Copy link
Contributor

apenney commented Nov 8, 2013

OK, I commited a few fixes for redhat/debian based on all the other PRs going in, so we'll need to rebase again off that to clear some test failures for event/ssl, however I did spot:

Error: validate_re(): "event" does not match "(itk|prefork|worker)" at /etc/puppet/modules/apache/manifests/init.pp:69 on node main.foo.vm.

This may be fixed by the change I made as I did wrap an event test in an if freebsd earlier. If you can rebase and repush I'll recheck and see where we stand.

Summary of the changes:

Overview:

* added support for FreeBSD
* added MPMs: event, peruser, itk (PR puppetlabs#304 + FreeBSD support)
* added `apache::package` to choose and install apache package,
* allow apache::mod to specify mod identifier and module path (puppetlabs#271)
* revisited specs for apache::dev and apache::mod::dev

Details:

* "${apache::params::conf_dir}/Includes" as $apache::confd_dir for FreeBSD,
* "${apache::params::conf_dir}/Modules" as $apache::mod_dir FreeBSD,
* "${apache::params::conf_dir}/Vhosts" as $apache::vhost_dir FreeBSD,
* added to apache::params:
        $root_group,
        $apache_package,
        $service_name,
        $server_root,
        $mime_support_package,
        $mime_types_config
* httpd.erb now uses $server_root (instead of $httpd_dir) as ServerRoot
* added $mime_support_package parameter to apache::mod::mime class,
* apache::mod::mime installs $mime_support_package package if needed,
* added $magic_file parameter to apache::mod::mime_magic class,
* added 'default_confd_files.pp', and confd template infrastructure to
  allow putting some files under conf.d/ (Includes/ under FreeBSD) by
  default (FreeBSD's apache22 installs Includes/no-accf.conf for
  example but puppet normally would purge it afterward),
* adjusted documentation (README.md),
* apache::dev requires apache::package on FreeBSD
* other (minor or forgotten) changes and additions,
@ptomulik
Copy link
Contributor Author

ptomulik commented Nov 9, 2013

rebased

apenney pushed a commit that referenced this pull request Nov 11, 2013
Support for FreeBSD and few other features (reworked PR #264).
@apenney apenney merged commit 79f540b into puppetlabs:master Nov 11, 2013
@apenney
Copy link
Contributor

apenney commented Nov 11, 2013

space

@ptomulik ptomulik deleted the freebsd_new branch November 12, 2013 11:58
@ptomulik
Copy link
Contributor Author

What a picture! Thanks ;)

amvapor pushed a commit to amvapor/puppetlabs-apache that referenced this pull request Nov 12, 2013
added peruser and event mpms

Workaround for apxs-loaded modules

On some OSes (FreeBSD) apxs tool is used to put LoadModule directives
into httpd.conf during apache package (and apache modules)
insallation/reinstallation. The apxs expects some LoadModule directives
to be already present in httpd.conf (they may be commented-out) in order
to decide where to put its own directives.

This PR puts fake LoadModule directive (commented out) to httpd.conf.
The $apxs_workaround boolean parameter in apache class decides, whether
to use this workaround or not. This is used on FreeBSD, where apxs is
used by ports package provider (and perhaps all other). Without this,
the apache installation/reinstallation/deinstallation as well as
installation of additional modules would fail.

This PR was created in order to split puppetlabs#342 into smaller parts to make
review process easier, see
puppetlabs#342 (comment)

Remove AllowOverride header for non-directories

When adding a dir with a different provider than directory (e.g. files,
or location), the AllowOverride header still added. This causes a
warning in apache.

Add unit tests

Fixed a typo. The directory provider should be lower case to match the manifests.

allow to choose the mpm_event mod from the init.pp

added a mod/event.pp manifest and edited the spec test
of the prefork/worker/event modules.

Add initial support for nss module (no directives in vhost template yet)

added $server_root parameter

Previously, the $httpd_dir was used as ServerRoot in httpd.conf
template. On some installations httpd_dir is not same as ServerRoot,
for example on FreeBSD apache22 (installed via ports) has ServerRoot set
to /usr/local by default.

This PR adds $server_root parameter to apache::params. Its purpose is to
be substituted in httpd.conf as ServerRoot. This parameter is used
in puppetlabs#342. The purpose of the whole thing is to plit puppetlabs#342 into smaller
parts to make review process easier, see
puppetlabs#342 (comment)

Allow apache::mod to specify module id and path

Add $id, $path and $lib_path parameters to apache::mod. These parameters
are used in puppetlabs#342. The purpose of the whole thing is to plit puppetlabs#342 into
smaller parts to make review process easier, see
puppetlabs#342 (comment)

This PR covers all changes introduced by puppetlabs#271 to manifests/mod.pp. I
just don't have test/mod_load_params.pp as in puppetlabs#271.

Add new params to apache::mod::mime class

This changeset adds $mime_support_package and $mime_types_config
parameters to class `apache::mod::mime`. Their purpose is following:

- $mime_support_package - install this package, as it provides mime.conf
  file (definition of mime types used by mod_mime),
- $mime_types_config - used to substitute TypesConfig in
  mod/mime.conf.erb template.

This PR was created in order to plit puppetlabs#342 into smaller
parts to make review process easier, see
puppetlabs#342 (comment)

Include mime if ssl is in use.

Without the default_mods we still try to setup an SSL vhost that fails
because AddType is used in the template.  Rather than trying to make
AddType conditional on the inclusion of the class we just require mime
as a standard mod no matter what you say.

Wrap the event test in a check for FreeBSD.

It seems this only works on FreeBSD currently.  In the future
we'll rework testing to test for 2.2 vs 2.4 or something.

Setting up the ability to do multiple rewrites and conditions. Allowing grouping and commenting

Changing the syntax of the directives. Now the rewrite directive is more intuitive and is consistent with other directives (i.e: aliases)

filling out the documentation a little more thoroughly

filling out the documentation a little more thoroughly

adding validation to the rewrites param

adding better tests for the new rewrite settings

make sure at least the first rewrite is a valid hash

adding warnings for depricated parameters

fixing spelling mistake

setting up real tests of rewrites

Setting up the ability to do multiple rewrites and conditions. Allowing grouping and commenting

Changing the syntax of the directives. Now the rewrite directive is more intuitive and is consistent with other directives (i.e: aliases)

filling out the documentation a little more thoroughly

filling out the documentation a little more thoroughly

adding validation to the rewrites param

adding better tests for the new rewrite settings

make sure at least the first rewrite is a valid hash

adding warnings for depricated parameters

fixing spelling mistake

setting up real tests of rewrites

fixing a misstype

renaming the rewrite_conds and rewrite_rules parameters to be singular to maintain consistency with previous standard
amvapor pushed a commit to amvapor/puppetlabs-apache that referenced this pull request Nov 19, 2013
added peruser and event mpms

Workaround for apxs-loaded modules

On some OSes (FreeBSD) apxs tool is used to put LoadModule directives
into httpd.conf during apache package (and apache modules)
insallation/reinstallation. The apxs expects some LoadModule directives
to be already present in httpd.conf (they may be commented-out) in order
to decide where to put its own directives.

This PR puts fake LoadModule directive (commented out) to httpd.conf.
The $apxs_workaround boolean parameter in apache class decides, whether
to use this workaround or not. This is used on FreeBSD, where apxs is
used by ports package provider (and perhaps all other). Without this,
the apache installation/reinstallation/deinstallation as well as
installation of additional modules would fail.

This PR was created in order to split puppetlabs#342 into smaller parts to make
review process easier, see
puppetlabs#342 (comment)

Remove AllowOverride header for non-directories

When adding a dir with a different provider than directory (e.g. files,
or location), the AllowOverride header still added. This causes a
warning in apache.

Add unit tests

Fixed a typo. The directory provider should be lower case to match the manifests.

allow to choose the mpm_event mod from the init.pp

added a mod/event.pp manifest and edited the spec test
of the prefork/worker/event modules.

Add initial support for nss module (no directives in vhost template yet)

added $server_root parameter

Previously, the $httpd_dir was used as ServerRoot in httpd.conf
template. On some installations httpd_dir is not same as ServerRoot,
for example on FreeBSD apache22 (installed via ports) has ServerRoot set
to /usr/local by default.

This PR adds $server_root parameter to apache::params. Its purpose is to
be substituted in httpd.conf as ServerRoot. This parameter is used
in puppetlabs#342. The purpose of the whole thing is to plit puppetlabs#342 into smaller
parts to make review process easier, see
puppetlabs#342 (comment)

Allow apache::mod to specify module id and path

Add $id, $path and $lib_path parameters to apache::mod. These parameters
are used in puppetlabs#342. The purpose of the whole thing is to plit puppetlabs#342 into
smaller parts to make review process easier, see
puppetlabs#342 (comment)

This PR covers all changes introduced by puppetlabs#271 to manifests/mod.pp. I
just don't have test/mod_load_params.pp as in puppetlabs#271.

Add new params to apache::mod::mime class

This changeset adds $mime_support_package and $mime_types_config
parameters to class `apache::mod::mime`. Their purpose is following:

- $mime_support_package - install this package, as it provides mime.conf
  file (definition of mime types used by mod_mime),
- $mime_types_config - used to substitute TypesConfig in
  mod/mime.conf.erb template.

This PR was created in order to plit puppetlabs#342 into smaller
parts to make review process easier, see
puppetlabs#342 (comment)

Include mime if ssl is in use.

Without the default_mods we still try to setup an SSL vhost that fails
because AddType is used in the template.  Rather than trying to make
AddType conditional on the inclusion of the class we just require mime
as a standard mod no matter what you say.

Wrap the event test in a check for FreeBSD.

It seems this only works on FreeBSD currently.  In the future
we'll rework testing to test for 2.2 vs 2.4 or something.
traylenator pushed a commit to traylenator/puppetlabs-apache that referenced this pull request Jun 7, 2022
* adding in note about use of latest image tag

* release prep for 3.0.0 (puppetlabs#342)

* V3.0.0 release prep (puppetlabs#343)

* release prep for 3.0.0

* Readme updates prior to release prep

* Minor updates

Minor updates

* CLOUD-2146-cleanup-of-cocker-compose-acceptance-tests (puppetlabs#341) (puppetlabs#344)

* pinning puppet version to fix failing spec tests (puppetlabs#346)

* CLOUD-2146-cleanup-of-cocker-compose-acceptance-tests (puppetlabs#341)

* pinning puppet version to fix failing spec tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants