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

Fetch correct _module_id value in apache::mod::php #2163

Closed
wants to merge 2 commits into from
Closed

Fetch correct _module_id value in apache::mod::php #2163

wants to merge 2 commits into from

Conversation

storm49152
Copy link

[CentOS, can't speak for other distros] The _module_id works fine with PHP from standard repo's. However, the PHP version in those repositories is usually old, and sometimes you need a newer PHP version, in which case one often uses the Remi repository. If you want to use e.g. php74, it will be installed without issues, but Apache will not start:

class { 'apache::mod::php':
  package_name => 'php74',
  php_version => '74',
}
Jul 01 10:29:07 server-1 httpd[15883]: httpd: Syntax error on line 40 of /etc/httpd/conf/httpd.conf: Syntax error on line 1 of /etc/httpd/conf.modules.d/php.load: Can't locate API module structure `php_module' in file /etc/httpd/modules/libphp74.so: /etc/httpd/modules/libphp74.so: undefined symbol: php_module
# cat /etc/httpd/conf.modules.d/php.load
LoadModule php_module modules/libphp74.so

I traced this to:

$_module_id = $_php_major ? {
  '5'     => 'php5_module',
  '7'     => 'php7_module',
  default => 'php_module',
}

With the example above, $_php_major isn't matching 7 but 74. Therefore $_module_id defaults to php_module instead of php7_module and Apache will not start.
When a regex is used to match /^5/ and /^7/, $_module_id will be set to php7_module, configuring the php.load file properly and Apache will start:

# cat /etc/httpd/conf.modules.d/php.load
LoadModule php7_module modules/libphp74.so

[CentOS, can't speak for other distros] The _module_id works fine with PHP from standard repo's. However, the PHP version in those repositories is usually old, and sometimes you need a newer PHP version, one often uses the Remi repository. If you want to use e.g. php74, it will be installed without issues, but Apache will not load:

```
class { 'apache::mod::php':
  package_name => 'php74',
  php_version => '74',
}
```

```
Jul 01 10:52:14 gcp-nco-web-101 httpd[17830]: httpd: Syntax error on line 40 of /etc/httpd/conf/httpd.conf: Syntax error on line 1 of /etc/httpd/conf.modules.d/php7.4.load: Cannot load modules/libphp7.4.so into server: /etc/httpd/modules/libphp7.4.so: cannot open shared object file: No such file or directory
```

```
# cat /etc/httpd/conf.modules.d/php.load
LoadModule php_module modules/libphp74.so
```

This happens because `$_php_major` isn't matching `7`, but `74`. Therefore `$_module_id` defaults to `php_module` instead of using `php7_module` and Apache not start.
If a regex is used to match `/^7/`, the `php.load` file will be properly configured and Apache will start:

```
# cat /etc/httpd/conf.modules.d/php.load
LoadModule php7_module modules/libphp74.so
```
Fetch correct _module_id value in apache::mod::php
@storm49152 storm49152 requested a review from a team as a code owner July 1, 2021 09:42
@CLAassistant
Copy link

CLAassistant commented Jul 1, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@puppet-community-rangefinder
Copy link

apache::mod::php is a class

Breaking changes to this file WILL impact these 49 modules (exact match):
Breaking changes to this file MAY impact these 36 modules (near match):

This module is declared in 175 of 576 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I think the problem is that you pass the version as 74 but you should really pass 7.4. As you can see, it compares the major version. That's extracted using a regex on line 64.

@storm49152
Copy link
Author

You are correct. However, I did did try 7.4 and that didn't work either. I locally reverted the change I proposed and tested this again.

class { 'apache::mod::php':
  package_name => 'php74-php',
  php_version  => '7.4',
}
# puppet agent -t
[...snip...]
Error: /Stage[main]/Apache::Service/Service[httpd]: Failed to call refresh: Systemd restart for httpd failed!
journalctl log for httpd:
-- Logs begin at Thu 2021-07-01 05:05:51 CEST, end at Mon 2021-07-05 22:52:45 CEST. --
Jul 05 22:52:44 server-1 systemd[1]: Stopping The Apache HTTP Server...
Jul 05 22:52:45 server-1 systemd[1]: httpd.service: Succeeded.
Jul 05 22:52:45 server-1 systemd[1]: Stopped The Apache HTTP Server.
Jul 05 22:52:45 server-1 systemd[1]: Starting The Apache HTTP Server...
Jul 05 22:52:45 server-1 httpd[212559]: httpd: Syntax error on line 40 of /etc/httpd/conf/httpd.conf: Syntax error on line 1 of /etc/httpd/conf.modules.d/php7.4.load: Cannot load modules/libphp7.4.so into server: /etc/httpd/modules/libphp7.4.so: cannot open shared object file: No such file or directory
Jul 05 22:52:45 server-1 systemd[1]: httpd.service: Main process exited, code=exited, status=1/FAILURE
Jul 05 22:52:45 server-1 systemd[1]: httpd.service: Failed with result 'exit-code'.
Jul 05 22:52:45 server-1 systemd[1]: Failed to start The Apache HTTP Server.
[...snip...]

# cat /etc/httpd/conf.modules.d/php7.4.load 
LoadModule php7_module modules/libphp7.4.so

# ls /etc/httpd/modules/libphp7.4.so
ls: cannot access '/etc/httpd/modules/libphp7.4.so': No such file or directory

# ls /etc/httpd/modules/libphp*
lrwxrwxrwx 1 root root 55 Jun 29 18:24 /etc/httpd/modules/libphp74.so -> /opt/remi/php74/root/usr/lib64/httpd/modules/libphp7.so

So, even though my proposal seemed to work, looking back it looks like it's not for the right reason nor the correct fix for the actual problem. However, AFAICS there still needs to be some fix to correct the content of php7.4.load since the resulting config is not working.
Or could there something else I'm doing wrong in using the apache module..?

@ekohl
Copy link
Collaborator

ekohl commented Jul 6, 2021

I think you're right. Here's the relevant code:

$_php_major = regsubst($php_version, '^(\d+)\..*$', '\1')
$_php_version_no_dot = regsubst($php_version, '\.', '')
if $apache::version::scl_httpd_version {
$_lib = "librh-php${_php_version_no_dot}-php${_php_major}.so"
} else {
# Controls php version and libphp prefix
$_lib = "${libphp_prefix}${php_version}.so"
}

It looks like it should be using $_php_version_no_dot instead of $php_version there. However, I don't know if that breaks other distros.

@github-actions
Copy link

github-actions bot commented May 9, 2022

This PR has been marked as stale because it has been open for a while and has had no recent activity. If this PR is still important to you please drop a comment below and we will add this to our backlog to complete. Otherwise, it will be closed in 7 days.

@github-actions github-actions bot added the stale label May 9, 2022
@storm49152
Copy link
Author

So the ticket is now marked as stale.

I think I traced a problem that should be fixed. However, even though it felt trivial to do, apparently I don't know how to fix this properly. If anyone can take this from me, then that would be great. I fixed the problem locally as described. Maybe it's not the best/preferred way, but at least now we can use the module. Obviously, this local fix will be undone whenever the module is updated, so it's suboptimal.

@github-actions github-actions bot removed the stale label May 11, 2022
@github-actions
Copy link

Hello! 👋

This pull request has been open for a while and has had no recent activity. We've labelled it with attention-needed so that we can get a clear view of which PRs need our attention.

If you are waiting on a response from us we will try and address your comments on a future Community Day.

Alternatively, if it is no longer relevant to you please close the PR with a comment.

Please note that if a pull request receives no update for 7 after it has been labelled, it will be closed. We are always happy to re-open pull request if they have been closed in error.

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

Successfully merging this pull request may close these issues.

4 participants