quote slash (/) as a key separator when communicating with augeas

[johnwarburton]

Pull Request (PR) description

Quote sysctl keys with a slash in them

This Pull Request (PR) fixes the following issues

Partially fixes #17 by quoting slashes in keys (such as VLANs) when communicating with augeas

This requires augeas 1.14.x with patch to sysctl lens, that supports more non-alphanumeric characters such as a slash. Have tested by directly copying ne wlens to /opt/puppetlabs/puppet/share/augeas/lenses/dist/sysctl.aug of puppet agent 7.25

A new puppet agent with augeas 1.14.x needs to be advocated

Note that the changes provided here are probably not optimal and possibly break with comments, but its at the limit of my current provider knowledge

[johnwarburton]

I am not sure what to do about the archlinux test failing

[bastelfreak]

@johnwarburton ignore the archlinux job for now, we need to fix that in our CI. Are you able to add tests for this change?

[johnwarburton]

Hi @bastelfreak - sure

I am not sure what image you are using for CI, but the puppet-dev-tools image has augeas 1.11, and we need, at least 1.14 to get the new sysctl lens that accepts a slash in the key. Otherwise these tests fail trying to load the test data

How can we ensure we are testing with a 1.14 lens? I've force installed http://ftp.au.debian.org/debian/pool/main/a/augeas/augeas-lenses_1.14.0-1_all.deb to test. I am reluctant to submit the tests in this PR if they fail immediately due to the old lens being present

UPDATE - nope
augeas 1.11 with the 1.14 lens isn't enough - we need at least augeas 1.13 which can work with no alphanumerics in keys. The tests with 1.11 and 1.14 lens do not die, but also completely ignore kets with a slash

I looked at the puppet-dev-tools image and its using buster to get ruby 2.7.4, but augeas 1.14 does not appear in buster, only next release bookworm. But ruby 2.x is not available in bookworm so we're at a testing impasse 😞 - need augeas 1.14 to test this PR fully

So, yeah, I could go down the patch of creating a bookwork puppet-dev-tools but I am not sure that's the image you are using 🤷

[johnwarburton]

Hi @bastelfreak

I have dug in and tried to write tests for this particular PR, but have come to the conclusion, that like the PR, all the tests need to be rewritten to cater for a / in a key. My reasoning below that augeas is OK, but the test harness is not

I am really in the weeds here and really do not know what I am doing nor how to progress.

If we start with a vanilla copy of this PR and add a single change - a valid sysctl.conf line that is for configuring VLANs, just the basic tests cannot parse the file correctly

## NB latest version of augeasproviders_core
root@060760e3f1f9:/repo# grep version spec/fixtures/modules/augeasproviders_core/metadata.json
  "version": "4.0.2-rc0",

root@060760e3f1f9:/repo# tail -1 /repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full
net.ipv4.conf.eth0/123.rp_filter = 321

root@060760e3f1f9:/repo# rake spec

Failures:

  1) Puppet::Type::Sysctl::ProviderAugeas with full file should create new entry next to commented out entry
     Failure/Error: raise AugeasSpec::Error, "augparse failed:\n#{output}" unless $CHILD_STATUS == 0 && output.empty?

     AugeasSpec::Error:
       augparse failed:
       Syntax error in lens definition
       Failed to load /tmp/d20230718-3445-exhbz6/test_augeasproviders.aug
       Test failure:/tmp/d20230718-3445-exhbz6/test_augeasproviders.aug:2.2-7.55:
        Expected:
       {
         { "net.bridge.bridge-nf-call-ip6tables" = "0" }
         { "#comment" = "net.bridge.bridge-nf-call-iptables = 0" }
         { "net.bridge.bridge-nf-call-iptables" = "1" }
         { "net.bridge.bridge-nf-call-arptables" = "0" }
       }

        Actual:
       {
         { "net.bridge.bridge-nf-call-ip6tables" = "0" }
         { "#comment" = "net.bridge.bridge-nf-call-iptables = 0" }
         { "net.bridge.bridge-nf-call-iptables" = "1" }
         { "net.bridge.bridge-nf-call-arptables" = "0" }
         { "123.rp_filter" = "321" }
       }

And we can see in the "Actual" results that the tests have parsed and split the net.ipv4.conf.eth0/123.rp_filter key on the slash

Now, I know that augeas isn't the problem as I can use augtool to read in the file and find the VLAN key correctly in the internal augeas format with the slash escaped

root@060760e3f1f9:/repo# augtool --version
augtool 1.14.0 <http://augeas.net/>

root@060760e3f1f9:/repo# augtool --root /repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/ --transform 'Sysctl incl /full'
augtool> print /files/
/files
/files/full
/files/full/#comment[1] = "Kernel sysctl configuration file"
/files/full/#comment[2] = "For binary values, 0 is disabled, 1 is enabled.  See sysctl(8) and"
/files/full/#comment[3] = "sysctl.conf(5) for more details."
/files/full/#comment[4] = "Controls IP packet forwarding"
/files/full/net.ipv4.ip_forward = "0"
/files/full/#comment[5] = "Controls source route verification"
/files/full/net.ipv4.conf.default.rp_filter = "1"
/files/full/#comment[6] = "net.ipv4.conf.default.accept_source_route: Do not accept source routing"
/files/full/net.ipv4.conf.default.accept_source_route = "0"
/files/full/#comment[7] = "SysRq setting"
/files/full/#comment[8] = "kernel.sysrq: controls the System Request debugging functionality of the kernel"
/files/full/kernel.sysrq = "0"
/files/full/#comment[9] = "Controls whether core dumps will append the PID to the core filename."
/files/full/#comment[10] = "Useful for debugging multi-threaded applications."
/files/full/kernel.core_uses_pid = "1"
/files/full/#comment[11] = "Disable netfilter on bridges."
/files/full/net.bridge.bridge-nf-call-ip6tables = "0"
/files/full/#comment[12] = "net.bridge.bridge-nf-call-iptables = 0"
/files/full/net.bridge.bridge-nf-call-arptables = "0"
/files/full/net.ipv4.conf.eth0\/123.rp_filter = "321"

Also its not the augeas ruby bindings - I munged a file from augeas core to simply read in and write out internal representation of the file spec/fixtures/modules/augeasproviders_core/spec/lib/augeas_spec/fixtures.rb

root@060760e3f1f9:/repo# cat fixtures.rb
#!/usr/local/bin/ruby

require 'augeas'

  # Open Augeas on a given file.  Used for testing the results of running
  # Puppet providers.
  def aug_open(file, lens)
    aug = Augeas.open(nil, nil, Augeas::NO_MODL_AUTOLOAD)
    begin
      aug.transform(
        lens: lens,
        name: lens.split('.')[0],
        incl: file,
        excl: []
      )
      aug.set('/augeas/context', "/files#{file}")
      aug.load!
      raise AugeasSpec::Error, "Augeas didn't load #{file}" if aug.match('.').empty?

      yield aug
    rescue Augeas::Error
      errors = []
      aug.match('/augeas//error').each do |errnode|
        aug.match("#{errnode}/*").each do |subnode|
          subvalue = aug.get(subnode)
          errors << "#{subnode} = #{subvalue}"
        end
      end
      raise AugeasSpec::Error, errors.join("\n")
    ensure
      aug.close
    end
  end

target = '/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full'

aug_open(target, "Sysctl.lns") do |aug|
  aug.match("/files/#{target}/*").each do |node|
    val = aug.get(node)
    puts node + ' = ' + val
  end
end

root@060760e3f1f9:/repo# ./fixtures.rb
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[1] = Kernel sysctl configuration file
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[2] = For binary values, 0 is disabled, 1 is enabled.  See sysctl(8) and
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[3] = sysctl.conf(5) for more details.
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[4] = Controls IP packet forwarding
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.ipv4.ip_forward = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[5] = Controls source route verification
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.ipv4.conf.default.rp_filter = 1
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[6] = net.ipv4.conf.default.accept_source_route: Do not accept source routing
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.ipv4.conf.default.accept_source_route = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[7] = SysRq setting
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[8] = kernel.sysrq: controls the System Request debugging functionality of the kernel
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/kernel.sysrq = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[9] = Controls whether core dumps will append the PID to the core filename.
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[10] = Useful for debugging multi-threaded applications.
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/kernel.core_uses_pid = 1
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[11] = Disable netfilter on bridges.
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.bridge.bridge-nf-call-ip6tables = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[12] = net.bridge.bridge-nf-call-iptables = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.bridge.bridge-nf-call-arptables = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.ipv4.conf.eth0\/123.rp_filter = 321