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

#555 change "apache::mod::passenger" to support overriding all parameters expected by "apache::mod". #558

Closed
wants to merge 24 commits into from

Conversation

jonoterc
Copy link
Contributor

@jonoterc jonoterc commented Jan 7, 2014

apache::mod::passenger did not accept/pass (optional) module parameters to apache::mod,
thus preventing overriding the auto-generated module parameters. Without this option
apache::mod::passenger could not be used in conjunction with passenger modules generated
outside the OS package system (in particular, when compiled via
passenger-install-apache2-module), as those are output to different paths. This patch
enables overriding all parameters expected by "apache::mod", and includes spec coverage.

Pablo Fredrikson and others added 6 commits December 10, 2013 11:40
The current code allows you to set index_options but then ignores it.
Based on the git log for the commit that added this option it was
intended to be passed into directories.  The test that caught this
will be merged into spec/acceptance/vhost_spec.rb in a seperate PR.

You can reproduce the original failure with:

class { 'apache': }
host { 'test.server': ip => '127.0.0.1' }
apache::vhost { 'test.server':
  docroot    => '/tmp',
  index_options    => ['Charset=UTF-8'],
}

We're just going to remove them because they can be passed directly
into directory instead of indirectly.
The servername can be [schema://]hostname.domain[:port]. In the case of servername including schema and port, it is not appropriate to use as log file name. It will generate something like this: "ErrorLog /var/log/httpd/https://domain_error.log", which cause httpd failed to refresh/start.
…od::passenger

"apache::mod::passenger" was not accepting/passing (optional) mod paramters to apache::mod,
preventing overriding the auto-generated module parameters.  Without this option the
apache::mod::passenger could not be used in conjunction with passenger modules generated
outside the system package system (in particular, when compiled via
passenger-install-apache2-module), as those are output to different paths).  This patch
enables passing in all parameters used with "apache::mod".
support apache::mod params added to apache::mod:passenger with
specs (coverage is currently ubuntu-specific, along with the rest
of the spec coverage)
@igalic
Copy link
Contributor

igalic commented Jan 9, 2014

This will need spec/acceptance tests, and documentation.

@jonoterc
Copy link
Contributor Author

jonoterc commented Jan 9, 2014

I've got docs ready and spec/acceptance tests working for ubuntu (testing with the beaker set-up). However, HEL/Centos/SL's mod_passenger appears to have long-unstated dependencies on two separate 3rd-party repos; one is Passenger-specific and the other is EPEL . Any opinions on embedding these requirements in the module vs. pre-installing those dependencies "just for testing"? If you prefer these to go into the module, are there any concerns with adding a dependency on the stahnma/epel 3rd-party module?

jonoterc and others added 4 commits January 9, 2014 19:35
add basic spec/acceptance tests for passenger module; these verify a
default installation as well as passing in module loading parameters.
new tests revealed undocumented Redhat dependencies on passenger-
specific repository as well as the EPEL repository; as a first step
these dependencies are being manually resolved during setup for the
acceptance testing suite.

also, documented new apache::mod::passenger parameters (originatinig
with apache::mod) in README.passenger.md
This is implemented quite similar to RequestHeader directives support.
Includes updated documentation and a simple spec test. Fixes issue #573.
…'t lost

The mod_passenger RPM supplies /etc/httpd/conf.d/passenger.conf with the
correct PassengerRoot for the version of the package installed.  The most
reliable way to keep the PassengerRoot accurate is to leave this file in
place and then to install a second file (passenger_extra.conf) for other
Passenger customisations.

Hardcoding the PassengerRoot in the module means it gets out of step with
package repositories (3.0.21 is in EPEL6 at the time of writing, while the
module assumes .17).  Passenger 4.x refuses to start if the PassengerRoot is
incorrect.  The user of apache::mod::passenger is also able to override this
value, but they shouldn't be forced to, just to get a functioning install.

Fixes #560
@jonoterc
Copy link
Contributor Author

Are there any further updates that would be useful to make here?

hunner and others added 7 commits January 23, 2014 10:35
Delete this key, mistakenly added.
Pass this through to directories.
Configure Passenger in separate .conf file on RH so PassengerRoot isn't lost
Support Header directives in vhost context
Remove spec:system references and remove tabs from docs
Fix the servername used in log file name
@igalic
Copy link
Contributor

igalic commented Jan 24, 2014

Sorry for sleeping on the job!

Apparently you'll have to rebase against the current master

igalic and others added 6 commits January 24, 2014 08:30
Don't purge mods-available dir when separate enable dir is used
…od::passenger

"apache::mod::passenger" was not accepting/passing (optional) mod paramters to apache::mod,
preventing overriding the auto-generated module parameters.  Without this option the
apache::mod::passenger could not be used in conjunction with passenger modules generated
outside the system package system (in particular, when compiled via
passenger-install-apache2-module), as those are output to different paths).  This patch
enables passing in all parameters used with "apache::mod".
support apache::mod params added to apache::mod:passenger with
specs (coverage is currently ubuntu-specific, along with the rest
of the spec coverage)
add basic spec/acceptance tests for passenger module; these verify a
default installation as well as passing in module loading parameters.
new tests revealed undocumented Redhat dependencies on passenger-
specific repository as well as the EPEL repository; as a first step
these dependencies are being manually resolved during setup for the
acceptance testing suite.

also, documented new apache::mod::passenger parameters (originatinig
with apache::mod) in README.passenger.md
rebase to master and fix following conflicts:
- correct apache::params on redhat systems for passenger_root and
  passenger_ruby to ensure a working default configuration
- fix expected paths in mod_passenger_spec to accomodate
  system-specific passenger configuration paths

Also:
- add notice of redhat-specific package repository dependencies
  to passenger README
@jonoterc
Copy link
Contributor Author

No worries, thanks for the heads-up

correct passenger specs for RedHat - these were out-of-date with the
revised default passenger params (for RedHat)
@jonoterc
Copy link
Contributor Author

I've committed a fix for the spec/classes/passenger_spec.rb failures from the Travis CI build

@jonoterc
Copy link
Contributor Author

If this needs to be rebased to master, could you recommend working versions for serverspec and specinfra, etc? I'm currently seeing 100% failure when running spec/acceptance tests against the latest versions. Thanks!

@jonoterc
Copy link
Contributor Author

Just saw the pending pull request in beaker-rspec; I'll wait on that fix. :)

@hunner
Copy link
Contributor

hunner commented Jan 30, 2014

Looks like the rebase to master didn't quite work. This PR still has 24 commits and a bunch of merge commits in it.

@jonoterc
Copy link
Contributor Author

Should I re-do the changes in a new branch off master and create a new PR? I'm fine with whatever approach will be easiest for reviewing (esp. now that beaker-rspec is fixed and I can run acceptance tests).

@igalic
Copy link
Contributor

igalic commented Jan 31, 2014

@jonoterc whatever is easier to you. We're quite flexible!

@jonoterc
Copy link
Contributor Author

jonoterc commented Feb 4, 2014

Hi, it seemed simplest to cleanly apply the updates to a new branch off an up-to-date master - I've submitted a new PR: #607

@blkperl
Copy link
Contributor

blkperl commented Feb 4, 2014

Ok, I'll close this one then. Thanks!

@blkperl blkperl closed this Feb 4, 2014
traylenator pushed a commit to traylenator/puppetlabs-apache that referenced this pull request Jun 7, 2022
(maint) Release Mergeback version v3.9.0
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.

6 participants