voxpupuli / puppet-zabbix

Puppet module for creating and maintaining zabbix components with puppet.
https://forge.puppet.com/puppet/zabbix
Apache License 2.0
79 stars 228 forks source link

Configure DirectoryIndex by default #854

Closed teluq-pbrideau closed 1 year ago

teluq-pbrideau commented 1 year ago

Pull Request (PR) description

Make sure the DirectoryIndex is configured when apache 'default_mods` are disabled:

  class { 'apache':
    default_mods  => false,
  }

  class { 'zabbix' :
    [...]
  }

Feel free to correct me if you think it should not be in this module, but instead only in my own profile.

This Pull Request (PR) fixes the following issues

Fixes #853

smortex commented 1 year ago

If the module is required, then it make sense to include it :+1:.

There seems to be some style issues in this file, do you mind adding a commit to fix those?

teluq-pbrideau commented 1 year ago

the problems aren’t related to my changes, but here is the linter fixed I expect the other tests to fail, I fixed the problem in #850 , do you recomment me to merge the changes to this PR, or to wait for it to be merged in master?

smortex commented 1 year ago

Ah, thanks for the pointer! The other PR looked good, I merged it.

Can you please rebase your changes on top of the main branch (the second commit being already in master it should be removed in the process)?

From your working directory:

git fetch origin       # Download the latest code we have here
git rebase origin/master # Move your commits on top of the main branch
git push -f            # Send the changes (-f is required because we re-wrote history)
teluq-pbrideau commented 1 year ago

well, the archlinux build seems broken:

archlinuxrolling-64.example.com 14:26:51$ puppet module install puppetlabs-postgresql -v 7.5.0
  Could not find 'english' (>= 0.a) among 97 total gem(s)
  Checked in 'GEM_PATH=/root/.local/share/gem/ruby/3.0.0:/usr/lib/ruby/gems/3.0.0' , execute `gem env` for more information
teluq-pbrideau commented 1 year ago
Error: The template Linux SNMP does not exist in Zabbix. Please use a correct one.

It appears upstream has renamed the snmp templates: Linux SNMP -> Linux by SNMP

It should probably be fixed in its own PR

smortex commented 1 year ago

It should probably be fixed in its own PR

Fine. You can create the feature branch on top of this branch if it helps you with testing, just remind us that "This PR also include # " in the new PR.

The CI failure put aside, this LGTM!

smortex commented 1 year ago

855 has been merged, which should fix CI. The change is unlikely to have any impact regarding CI so let's merge this, thanks!