wazuh / wazuh-puppet

Wazuh - Puppet module
https://wazuh.com
GNU General Public License v2.0
44 stars 132 forks source link

Backward compatibility with Puppet 3.8 #4

Closed elisiano closed 7 years ago

elisiano commented 7 years ago

Thanks for releasing this new module.

Let me start by saying that I know that Puppet 3.X is not supported anymore but in the real world there are still a lot of companies using it.

The module does not seem to be compatible with puppet 3.8. Are there any plans to make it so?

jlruizmlg commented 7 years ago

Hi @elisiano, you are right, this module has some code not compatible with 3.x, we will take a review and made it compatible.

elisiano commented 7 years ago

I think I managed to make it compatible (at least the parser does not complain) but I did not have the chance to test the code.

Do you want me to submit a PR or do you prefer reworking the code yourself?

jlruizmlg commented 7 years ago

Hi @elisiano sure!, send the pull request and we take a look! Thanks a lot!!

hex2a commented 7 years ago

The syntax (default resource) in #5 should work with 3.8.5 and future parser enabled.

soxwellfb commented 7 years ago

Will it work without the future parser enabled?

jlruizmlg commented 7 years ago

Hi @soxwellfb Puppet 3.8 is EOL, we will try to figure out some options but not sure if with puppet 3.8 will work without future parser.

Puppet 4.x works without problems.

soxwellfb commented 7 years ago

Hi @jlruizmlg - I'm aware it's EOLed, but rather stuck with it for the moment, and not in a position to enable the future parser. Exciting times.

jlruizmlg commented 7 years ago

Well we wil try our best as always!! :) i will keep you post.

elisiano commented 7 years ago

On top of the default resource syntax I found another statement which is not compatible (without the future parser enabled): accessing the $::os hash in params.pp. Same issue as described here.

By converting $::os[name] to $::operatingsystem fixed it.

jlruizmlg commented 7 years ago

Hi @elisiano this are fixed in the branch development with this commit https://github.com/wazuh/wazuh-puppet/commit/21a89fd4f3251d48993b39b239b938f15fbba311

elisiano commented 7 years ago

@jlruizmlg @hex2a I saw the comment on #5 and I answered. The defaults for the resources are local to the scope they're declared in. In that case only to wazuh server/client manifests. I've been using those changes (and the fact name mentioned in my previous comment) for days in production without any collateral issue.

What's the status for that PR? If you want me to implement any of the changes mentioned there just let me know. As @soxwellfb mentioned some of us are stuck with legacy installations without being able to enable the future parser.

I wouldn't want to have to keep a sync'ed fork with my changes.

jlruizmlg commented 7 years ago

hi @elisiano sorry for the delay, let me check this pull request, move to developer branch and test

jlruizmlg commented 7 years ago

hi @elisiano which concat version are you using?, I'm reading now the notes from concat and seems like the version 3.00 and 4.00 are only compatible with puppet 4.

I like to try your pull request with a full puppet 3.8 and evaluate the possible problems.

Thanks.

elisiano commented 7 years ago

apparently a very old one (didn't even notice): 1.2.5

jlruizmlg commented 7 years ago

hi @elisiano, keep the compatibilities with this old versions is not a easy thing.

Can you update the concat module?

elisiano commented 7 years ago

I had to create a separate environment to test this because a newer version of concat was conflicting with an old version of theforeman/puppet module.

Now this are the modules in the environment:

# puppet module list --tree --modulepath puppet_concat_latest
/srv/puppet/puppet-environments/puppet_concat_latest
└─┬ theforeman-puppet (v7.1.1)
  ├─┬ puppetlabs-apache (v1.11.0)
  │ ├── puppetlabs-stdlib (v4.12.0)
  │ └── puppetlabs-concat (v2.2.1)
  └── puppet-extlib (v1.1.0)

noteworthy: stdlib is not the latest because it caused issues with the hooks I have in place (puppet 4 code is not recognized) and concat version is the latest 2.X which is the one with puppet 3 support.

As far as I can tell the PR works as expected.

elisiano commented 7 years ago

Correction: I did not see any error on wazuh manager. Agents indeed throw errors at recent concat. Investigating.

elisiano commented 7 years ago

It might be an environment cache thing. After restarting the master it took a couple of runs in order to get everything in place (first run updated all plugins but still failed, then second run was ok).

The first time it failed with the following error:

puppet-agent[6664]: Could not retrieve catalog from remote server: Error 400 on SERVER: Invalid parameter target on Concat_fragment[ossec.conf_header] at /srv/puppet/puppet-environments/common/modules/concat/manifests/fragment.pp:56 on node mynode.
jlruizmlg commented 7 years ago

Hi @elisiano first at all, thanks for all your feedback and testing in this module. As you can see many modules are moving to puppet 4, so we think we should keep the master branch with puppet 4, but if you like we can create other branch to make this module compatible with puppet 3.8, what do you think?

elisiano commented 7 years ago

Yes I've noticed this trend but most modules did a transition, locking and documenting a specific version number as the last major version supporting old puppet. This being a new module is kinda different though, I totally understand the unwillingness to support EOL'ed puppet. It's just that there are a lot of us still out there :) . Yeah I guess a puppet3 branch would work.

jlruizmlg commented 7 years ago

Hi @elisiano after apply your changes in the pull request #5 and a few more (testing the correct modules like concat and apt, etc), I think we have a version compatible with puppet 3.8

Please take a look the branch https://github.com/wazuh/wazuh-puppet/tree/puppet3-compat, now I'm running manager and agents (in CentOS and Xenial) with puppet 3.8.7

[root@puppet wazuh-puppet]# rpm -qa | grep puppet
puppet-server-3.8.7-1.el7.noarch
puppetdb-terminus-2.3.8-1.el7.noarch
puppetdb-2.3.8-1.el7.noarch
puppetlabs-release-22.0-2.noarch
puppet-3.8.7-1.el7.noarch
[root@puppet wazuh-puppet]#

And agents with version 3.8.5 and 3.8.7

[root@centos7 ~]# puppet --version
3.8.7
[root@centos7 ~]#
root@ubuntu:~# puppet --version
3.8.5
root@ubuntu:~#