Any plans to make the module compatible with RHEL7?

ubellavance commented 8 years ago

Hi,

I am currently in the process of trying to install clamd on a RHEL7 machine but I can't find a module that is compatible with this version. Do you have any plans for making this module compatible. I can offer some help. However, I'm not too familiar with git and github.

edestecd commented 8 years ago

This module should run on RedHat 7. There are no statements to make it fail out. However, I have not extensively tested it on 7. I am interested in supporting RedHat 7, as we will be switching to it soon. If you would like to try it out and tell me what breaks, I can make adjustments.

ubellavance commented 8 years ago

A few things have changed, and I have posted some information in another project's issue: https://github.com/arusso/puppet-clamav/issues/8

The package name have changed. clamav-scanner-systemd works (I use systemd - default on EL7). - freshclam is now in a separate package: clamav-update. It is required to have a working setup.
The user has changed to clamscan (group?)
The service name has changed (clamd@scan.service)
The location of the config file has changed (/etc/clamd.d/scan.conf)
Default location for the pid file is now /var/run/clamd.scan/clamd.pid
Defaut location for the socket is now /var/run/clamd.scan/clamd.sock
I think that the config file may have changed as well. You may want to install the clamav-scanner package from EPEL on a Centos7 or RHEL7 system to see how the config file looks like. I can see that there are new options like ScanXMLDOCS and ScanHWP3. They can be enabled/disabled and have options. There also new options about PCRE, DDD/On-access scan, ForceInterpreter.

The thing that blocks me is that with your module I can't comment the LogFile option and there is no directory where the user could write and no log is provided (clamav-update provides /var/log/freshclam.log so it's not a problem for this one).

ubellavance commented 8 years ago

Here is a profile that works if I comment out the LogFile option in the erb template. Do you need more information? -Thanks!

class profile::local_clamd {
  class { 'clamav':
  manage_repo              => false,
  user                     => 'clamscan',
  clamd_package            => 'clamav-scanner-systemd',
  clamd_service            => 'clamd@scan.service',
  clamd_config             => '/etc/clamd.d/scan.conf',
  clamd_options            => {
    'User'        => 'clamscan',
    'PidFile'     => '/var/run/clamd.scan/clamd.pid',
    'LocalSocket' => '/var/run/clamd.scan/clamd.sock',
  },
  manage_clamd             => true,
  manage_user              => false,
  manage_freshclam         => false,
  clamd_service_ensure     => 'running',
  freshclam_service_ensure => 'stopped',
  }
}

edestecd commented 8 years ago

OK thanks. That gives me a good start. I'll fire up a CentOS 7.2 vagrant box this weekend and start messing around.

ubellavance commented 8 years ago

That is awesome! I guess that if you'd like to contact me you just have to add a comment to this issue. I won't be able to do anything computer-related until Sunday PM though, but I should be able to find plenty of time to test next week. Thanks!

ubellavance commented 8 years ago

Did you have time to start working on the CentOS/RHEL 7 support this week-end? Do you need more information?

edestecd commented 8 years ago

Not as much time as I wanted. I got started and ran into a spec issue that will require me to rewrite some spec tests. I should have more time today...

edestecd commented 8 years ago

@ubellavance the master branch should now mostly support RedHat 7. Can you give it a try and let me know how it goes? Let me know if you need help checking out the code from github...

ubellavance commented 8 years ago

I'll test it ASAP. Currently stuck on emergencies. I hope it will be before the end of the week. When using github code, I typically download the master zip file and copy it to my puppet server, then extract. I guess I'll have to remove the contents of the module's directory first.

mheiges commented 8 years ago

Thanks for this update. I have had success installing commit 53743a9 on CentOS 7.2 using the following. (some of the options may be unnecessarily restating the default values)

  class { 'clamav':
    manage_clamd         => true,
    manage_user          => false,
    manage_freshclam     => true,
    user                 => 'clamscan',
    clamd_package        => 'clamav-scanner-systemd',
    clamd_service        => 'clamd@scan.service',
    clamd_config         => '/etc/clamd.d/scan.conf',
    clamd_service_ensure => 'running',
    clamd_options        => {
      'LogSyslog'                => 'yes',
      'LocalSocket'              => '/var/run/clamd.scan/clamd.sock',
      'User'                     => 'clamscan',
      'AllowSupplementaryGroups' => 'yes',
    },
    freshclam_options    => {
      'LogSyslog'      => 'yes',
      'DatabaseMirror' => ['db.us.clamav.net', 'db.local.clamav.net'],
      'UpdateLogFile'  => '/var/log/freshclam.log',
    },
  }

Puppet 4.x defaults to using strict_variables so it warns about the undefined $freshclam_service variable when running on RHEL. I understand the RHEL uses cron instead of a service but perhaps the $freshclam_service variable can be given a dummy default value that can be tested for in the clamav::freshclam class?

Gotchas not related to this puppet module include:

The clamd@scan.service expects /etc/clam.d/scan.conf but clamdscan is hard coded for to look for /etc/clamd.conf. So I symlink /etc/clamd.conf to /etc/clam.d/scan.conf in my profile. (There's also a clamdscan --config-file option). https://bugzilla.redhat.com/show_bug.cgi?id=859339

EPEL's clamav-data-0.99.1-1 installs a corrupt main.cvd file and that prevents clamd@scan.service from starting until freshclam is run to update the databases. https://bugzilla.redhat.com/show_bug.cgi?id=1325482

ubellavance commented 8 years ago

I just checked my RHEL 7 systems that I provisionned for testing on May 12th to find out that freshclam didn't run via cron on them as I tought it would. It is because of one line in /etc/sysconfig/freshclam: FRESHCLAM_DELAY=disabled-warn # REMOVE ME Is the module supposed to manage that?

Thanks,

mheiges commented 8 years ago

oh, yeah, I forgot about that. I have my local profile empty that file ( content => '' ) because I don't use any of the options. It would be better in general for the module to template the file like you did with the freshclam_options and clamd_options.

edestecd commented 8 years ago

Hopefully I can get back to this this week. I want to include more fixes from your feedback before releasing the next version to the forge.

I also want to make sure the clamd service runs out of the box by providing more useful config file defaults than the epel package. (yanked from closed issue)

kronos-pbrideau commented 8 years ago

I would be glad to make a PR about it, I would also want these changes...

I think using the same defaults for both Debian and RedHat based systems would be nice, is it what you thought? I mean: Make the variables $clamd_default_options and $fresclam_default_options in params.pp out of the if $::osfamily ==

edestecd commented 8 years ago

@kronos-pbrideau I had thought of making them the same for both RedHat and Debian, but I'm not sure about all the paths etc. Sometimes config files are located in different locations etc... But maybe they are similar enough? I'd definitely have a look at a PR that attempted this.

I think the basic complaint was that the default epel options (which I cloned here) would not allow you to start the service out of the box. For example, no tcp listen or socket path was specified etc.

ubellavance commented 8 years ago

Do you think it could be possible to integrate clamav-milter in this module? Just asking...

edestecd commented 8 years ago

I'm not opposed to clamav-milter, as long as its a separate, optional class.

ubellavance commented 8 years ago

That would be great. I agree that it should be optionnal.

Would you like to have some help? I should be able to provide you some information shortly (similar to what I've done for clamd), I have already started to check what is required to get milter-clamav working on RHEL7 through EPEL. Should I create a new issue for that?

edestecd commented 8 years ago

Yes a new issue would be nice. Would you be able to help with some code / PRs?

ubellavance commented 8 years ago

Ok, I'll create a separate issue. I would love to contribute in the most efficient way, which seems to be code/PRs but I have a few constraints:

I don't know how to use git much (I only know subversion)
I would probably merely copy what you did for other classes and adapt it for milter-clamav
I don't know at all how puppet module testing works

But if you think you can guide me a little and that I can bring value, I'd be more than happy to put efforts into it.

edestecd commented 8 years ago

There's no better time than now to learn git! I made the transition from svn several years ago. It is well worth the effort. Most of my work starts that way. I find something that is the most similar to what I want and copy it. Supported modules from puppet are great examples to copy from. You know the code will work.

The test suite will auto run every time you push a commit / pr to this repo, so you can see if there is an issue. All the tests are in the spec folder. You can also run them on your box by doing:

bundle install
rake

I'm always happy to provide any help, just ask! Honestly if you could just get a good start, I can tweak and make suggestions from there. I am not really familiar with milter and its uses...

edestecd commented 8 years ago

@ubellavance this is good. https://try.github.io

ubellavance commented 8 years ago

@edestecd Looks great. I now have the basic knowledge of how to use git. I'll setup git on a machine when I have time and hopefully I will produce some code.

ubellavance commented 8 years ago

Just to let you know, I started working on it. I have done a git clone on my computer at home and started working on files. Is there a simple way to get my changes on github and then back on my test server at work? Or I can transfer them in a more traditional way (scp or email).

ubellavance commented 8 years ago

I've done a few commits: https://github.com/ubellavance/puppet-clamav. Had no time to test up to now. Feel free to have a look. What do I do when I am ready to "submit" my code? Pull request? How?

edestecd commented 8 years ago

@ubellavance if you install git on your server you can just git clone your fork then git pull updates to test out. You can also download a zip / tar.gz from github of the latest master branch.

edestecd commented 8 years ago

On the main page of your fork there should be a "New Pull Request" button.

ejunker commented 8 years ago

I tested with commit 53743a9 on CentOS 7 with the following and it appears to be working.

  class { 'clamav':
    manage_user          => false,
    manage_clamd         => true,
    manage_freshclam     => true,
    user                 => 'clamscan',
    clamd_service_ensure => 'running',
    clamd_options        => {
      'LogSyslog'                => 'yes',
      'LocalSocket'              => '/var/run/clamd.scan/clamd.sock',
      'TCPSocket'                => '3310',
      'TCPAddr'                  => '127.0.0.1',
      'User'                     => 'clamscan',
      'AllowSupplementaryGroups' => 'yes',
    }
  }

Is there anything remaining to be done for RHEL 7? I would love to get a tagged version that supports RHEL 7!

ubellavance commented 8 years ago

One thing to make sure is that the documentation is consistent with the module, even for RHEL7, so if you have a puppet config that works for RHEL6 hosts, the same puppet config should work on RHEL hosts as well. So all the desired default options must be in the module, so that the puppet config is minimal for the user.

The documentation says that this minimal config works:

class { 'clamav':
  manage_clamd             => true,
  manage_freshclam         => true,
  clamd_service_ensure     => 'running',
  freshclam_service_ensure => 'stopped',
}

so using this should be enough to get clamav, clamd and freshclam working on RHEL7 hosts.

LinguineCode commented 8 years ago

I tested with commit 53743a9 on CentOS 7 with the following and it appears to be working.

class { 'clamav': manage_user => false, manage_clamd => true, manage_freshclam => true, user => 'clamscan', clamd_service_ensure => 'running', clamd_options => { 'LogSyslog' => 'yes', 'LocalSocket' => '/var/run/clamd.scan/clamd.sock', 'TCPSocket' => '3310', 'TCPAddr' => '127.0.0.1', 'User' => 'clamscan', 'AllowSupplementaryGroups' => 'yes', } } Is there anything remaining to be done for RHEL 7? I would love to get a tagged version that supports RHEL 7!

+1 this works for me. It didn't work without specifying clamd_options listed here.

@ubellavance @edestecd I'm new to clamav, but I can help by submitting PRs to puppet code. Please list things that need to be done. I can start:

If you guys agree, we can use @ejunker's https://github.com/edestecd/puppet-clamav/issues/20#issuecomment-231140003 and modify https://github.com/edestecd/puppet-clamav/blob/43f6df9/manifests/params.pp#L40

edestecd commented 8 years ago

I'd like to get this merged first: #23 Then we can try and get in these new defaults... I'd put them in the $clamd_default_options options however: https://github.com/edestecd/puppet-clamav/blob/43f6df9/manifests/params.pp#L41

Rough Road map:

I'll try get #23 and #27 rebased/merged (shoot for this evening)
Update the defaults to meet @ejunker comment @seanscottking, you want to do this?
Release a new major version to the forge.
Start working on merging in clamav milter with @ubellavance.

edestecd commented 8 years ago

When #23 is merged, you will no longer need to set those options for it to work out of the box... The only one you will need is the TCP Socket stuff, if you need that. The default is LocalSocket only, which is the safest.

edestecd commented 8 years ago

I just pushed, to master, what I believe will be the next tag and release to the forge. Please try it out if you have a chance.

This should fully support RedHat 7 out of the box, without tweaking to get it running. I believe I have addressed all issues mentioned here. Please let me know, if not.

edestecd commented 8 years ago

Tagged as release 1.0.0 -> Straight to the forge...

edestecd / puppet-clamav

Any plans to make the module compatible with RHEL7? #20