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

Fix sysctl -a line parsing for FreeBSD #18

Closed
wants to merge 2 commits into from
Closed

Fix sysctl -a line parsing for FreeBSD #18

wants to merge 2 commits into from

Conversation

mikej-hc
Copy link

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
Copy link

coveralls commented Sep 15, 2017

Coverage Status

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

@@ -78,11 +78,10 @@ def self.instances(reference_resource = nil)
resources ||= []

sysctl('-a').each_line do |line|
value = line.split('=')
value = line.split(/(=|:)/)
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather use the form /[=:]/

@raphink
Copy link
Member

raphink commented Sep 18, 2017

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
Copy link
Author

Thanks for the feedback. I updated the PR.

@coveralls
Copy link

coveralls commented Sep 18, 2017

Coverage Status

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

1 similar comment
@coveralls
Copy link

Coverage Status

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

@rick-pri
Copy link

rick-pri commented Oct 2, 2017

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.soacceptqueue is not a valid sysctl key whilst trying to use this module.

@raphink
Copy link
Member

raphink commented Oct 4, 2017

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

@mikej-hc
Copy link
Author

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
Copy link

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
Copy link

rick-pri commented Feb 19, 2018

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
Copy link
Contributor

@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
Copy link

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
Copy link

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
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

@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
Copy link
Contributor

@raphink Any reason not to merge this?

@raphink
Copy link
Member

raphink commented Jul 16, 2019

Fixed in #30

@raphink raphink closed this Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants