voxpupuli / puppet-augeasproviders_sysctl

Augeas-based sysctl type and provider for Puppet
Apache License 2.0
10 stars 36 forks source link

Fix sysctl -a line parsing for FreeBSD #18

Closed mikej-hc closed 5 years ago

mikej-hc commented 7 years ago

Hello,

I ran in to a problem while using this nice module on FreeBSD.

Error: /Stage[main]/MyModule::Config/Sysctl[security.bsd.see_other_uids]: Could not evaluate: Error: security.bsd.see_other_uids is not a valid sysctl key Error: /Stage[main]/MyModule::Config/Sysctl[security.bsd.see_other_gids]: Could not evaluate: Error: security.bsd.see_other_gids is not a valid sysctl key

These are valid keys for FreeBSD, so it shouldn't be giving the error. I tracked it down to https://github.com/hercules-team/augeasproviders_sysctl/blob/master/lib/puppet/provider/sysctl/augeas.rb#L81

value = line.split('=')

It's splitting each line of output from sysctl -a, looking for the equals sign, so it can extract the key and the value. The problem is that the output from Linux has an equals sign seperator, while the output from BSD has a colon ":" seperator. Since it can't match the key with the colon seperator it throws the error.

I fixed it by changing L81 to a RegExp value = line.split(/(=|:)/) that matches on either an equals sign, or a colon.

Thanks

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 88.304% when pulling 2418cee437a7eecb58e5c4b5ade077dacfadc2af on mikej-hc:master into 012cd99bb6f1fdf210a2bc1c3f153138643dfb48 on hercules-team:master.

raphink commented 7 years ago

I see what you're doing, but it's not strictly equivalent. Iirc, there were cases when values contained =. Let's consider a line with the content: a.b.c = 3=4.

Old code:

irb(main):002:0> line="a.b.c = 3=4"
=> "a.b.c = 3=4"
irb(main):003:0> value = line.split('=')
=> ["a.b.c ", " 3", "4"]
irb(main):004:0> key = value.shift.strip
=> "a.b.c"
irb(main):005:0> value = value.join('=').strip
=> "3=4"

Your code

irb(main):016:0> line="a.b.c = 3=4"
=> "a.b.c = 3=4"
irb(main):017:0> value = line.split(/(=|:)/)
=> ["a.b.c ", "=", " 3", "=", "4"]
irb(main):018:0> key = value.shift.strip
=> "a.b.c"
irb(main):019:0> value = value.shift.to_s.strip
=> "="
mikej-hc commented 7 years ago

Thanks for the feedback. I updated the PR.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.1%) to 88.166% when pulling b6eab3d09e6823b7056ef79cc33fc460f3419b5d on mikej-hc:master into 012cd99bb6f1fdf210a2bc1c3f153138643dfb48 on hercules-team:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.1%) to 88.166% when pulling b6eab3d09e6823b7056ef79cc33fc460f3419b5d on mikej-hc:master into 012cd99bb6f1fdf210a2bc1c3f153138643dfb48 on hercules-team:master.

rick-pri commented 7 years ago

Is there any chance of some progress on this, I've just come across this because I got an error of Sysctl[kern.ipc.soacceptqueue]: Could not evaluate: Error:kern.ipc.soacceptqueueis not a valid sysctl key whilst trying to use this module.

raphink commented 7 years ago

That looks good to me (although a bit harder to understand). Do you think you could add tests for this?

mikej-hc commented 7 years ago

I'm new to Ruby and having trouble getting the tests to build. Do you have insight what I'm doing wrong? Thanks.

bundle install --path vendor
bundle exec rake spec
Cloning into 'spec/fixtures/modules/augeasproviders_core'...
I, [2017-10-20T13:00:59.225131 #35669]  INFO -- : Creating symlink from spec/fixtures/modules/augeasproviders_sysctl to /root/src/augeasproviders_sysctl
/usr/local/rvm/rubies/ruby-2.1.10/bin/ruby -I/root/src/augeasproviders_sysctl/vendor/ruby/2.1.0/gems/rspec-core-3.7.0/lib:/root/src/augeasproviders_sysctl/vendor/ruby/2.1.0/gems/rspec-support-3.7.0/lib /root/src/augeasproviders_sysctl/vendor/ruby/2.1.0/gems/rspec-core-3.7.0/exe/rspec --pattern spec/\{aliases,classes,defines,unit,functions,hosts,integration,type_aliases,types\}/\*\*/\*_spec.rb --color

An error occurred while loading ./spec/unit/puppet/provider/sysctl/augeas_spec.rb.
Failure/Error: raise("Missing augeasproviders_core dependency") if Puppet::Type.type(:augeasprovider).nil?

Puppet::Error:
  Could not autoload puppet/type/sysctl: Could not autoload puppet/provider/sysctl/augeas: Missing augeasproviders_core dependency
# ./lib/puppet/provider/sysctl/augeas.rb:6:in '<top (required)>'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:68:in 'load'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:68:in 'load_file'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:83:in 'block in loadall'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:81:in 'each'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:81:in 'loadall'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:208:in 'loadall'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/metatype/manager.rb:126:in 'newtype'
# ./lib/puppet/type/sysctl.rb:6:in '<top (required)>'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:68:in 'load'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:68:in 'load_file'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/util/autoload.rb:194:in 'load'
# ./vendor/ruby/2.1.0/gems/puppet-5.3.2/lib/puppet/metatype/manager.rb:171:in 'type'
# ./spec/unit/puppet/provider/sysctl/augeas_spec.rb:5:in '<top (required)>'
# ------------------
# --- Caused by: ---
# RuntimeError:
#   Missing augeasproviders_core dependency
#   ./lib/puppet/provider/sysctl/augeas.rb:6:in '<top (required)>'

Finished in 0.00058 seconds (files took 3.12 seconds to load)
0 examples, 0 failures, 1 error occurred outside of examples

[Coveralls] Outside the Travis environment, not sending data.
/usr/local/rvm/rubies/ruby-2.1.10/bin/ruby -I/root/src/augeasproviders_sysctl/vendor/ruby/2.1.0/gems/rspec-core-3.7.0/lib:/root/src/augeasproviders_sysctl/vendor/ruby/2.1.0/gems/rspec-support-3.7.0/lib /root/src/augeasproviders_sysctl/vendor/ruby/2.1.0/gems/rspec-core-3.7.0/exe/rspec --pattern spec/\{aliases,classes,defines,unit,functions,hosts,integration,type_aliases,types\}/\*\*/\*_spec.rb --color failed
rick-pri commented 7 years ago

It looks like you also need augeasproviders_core @mikej-hc and that's about all that I can suggest.

I too am new to Ruby and try to interact with it as little as Puppet allows me to (which is far too much for my liking) but this is definitely an area that I need to learn.

rick-pri commented 6 years ago

I've had a good look into how you are supposed to add the augeasproviders_core module when you're testing this module and there's nothing online that in any way helps to explain what on earth you're supposed to do. There is zero direction anywhere.

There is this comment: check that you have the augeasproviders_core module present in this issue but I already had guessed that. How you actually go about ensuring that that module is present I have no clue. This is extremely frustrating

trevor-vaughan commented 6 years ago

@rick-pri Check and make sure that the augeasproviders_core module is listed in the .fixtures.yml file in the module.

It appears to be in my copy, but may have been removed at some point. Also, you may need to remove spec/fixtures/modules/* manually and try again if something went wrong.

rick-pri commented 6 years ago

Hi @trevor-vaughan it seems that I do have a .fixtures.yml file and it specifies the repo:

fixtures:
  repositories:
    "augeasproviders_core":
      repo: "git://github.com/hercules-team/augeasproviders_core.git"
  symlinks:
    augeasproviders_sysctl: "#{source_dir}"

Actually, I can see that the module gets imported by watching the directory as the test run so the issue seems to be maybe one of pathing.

rick-pri commented 6 years ago

The error starts with An error occurred while loading ./spec/unit/puppet/provider/sysctl/augeas_spec.rb

If I do a require_relative with unit/puppet/provider/sysctl/augeas_spec I get another failure. I'm not sure that my ruby environment is clean as I'm just using the system ruby install on Ubuntu and then I've gem installed the likes of puppet.

I had a look at using rbenv but it doesn't seem to just work like a python virtualenv so I'll have to come back to this when I can be bothered ... ruby just makes me sad every time I have to work with it.

trevor-vaughan commented 6 years ago

@rick-pri Try following these instructions for setting up a test environment. I generally prefer rvm over rbenv because it tends to work around system issues better.

http://simp.readthedocs.io/en/master/getting_started_guide/ISO_Build/Environment_Preparation.html

rick-pri commented 6 years ago

I followed that as far as I could. I have rvm installed, tried to activate rvm (how do you know if you're using the rvm? virtualenv changes the shell on sourcing the env so you know when you're in the virtualenv).

I've done a bundle install and it installed a bunch of stuff. Now in python venv that would have been installed to the virtualenv, and I'd have been able to see the contents of the venv with a pip list, but the documentation seems to imply that I have to create a gemset maybe, and that somehow I have to install into the gemset?

trevor-vaughan commented 6 years ago

@rick-pri You should be good to go.

You can see what Ruby you're using by running rvm list and you can see what is installed via bundler by bundle list.

The shell doesn't change by default due to not wanting to disrupt the developer's environment.

Think of rvm as venv and bundle as pip and you're not far off.

trevor-vaughan commented 6 years ago

@raphink Any reason not to merge this?

raphink commented 5 years ago

Fixed in #30