Fix readonly handling

[towo]

Pull Request (PR) description

The database readonly properly expects a boolean, but the olc
provider doesn't take care to parse the existing value into a boolean,
thus leading to issues.

Simply applies the same logic applied to olcMirrorMode for
olcReadOnly.

This Pull Request (PR) fixes the following issues

n/a

[smortex]

This also lacks unit tests. Can you add some? Here is a starter spec/unit/puppet/provider/openldap_database/olc_spec.rb:

# frozen_string_literal: true

require 'spec_helper'

describe Puppet::Type.type(:openldap_database).provider(:olc) do
  describe '::instances' do
    context 'with all parameters' do
      before do
        expect(described_class).to receive(:slapcat).with('(|(olcDatabase=monitor)(olcDatabase={0}config)(&(objectClass=olcDatabaseConfig)(|(objectClass=olcBdbConfig)(objectClass=olcHdbConfig)(objectClass=olcMdbConfig)(objectClass=olcMonitorConfig)(objectClass=olcRelayConfig)(objectClass=olcLDAPConfig))))').and_return(<<~SLAPCAT)
          dn: olcDatabase={1}mdb,cn=config
          olcDatabase: {1}mdb
          olcReadOnly: FALSE
        SLAPCAT
      end

      it 'parses olcReadOnly' do
        expect(described_class.instances.first.readonly).to be_falsey
      end
    end
  end
end

[smortex]

I think we can use native types here:

Suggested change

	readonly = line.split[1] == 'TRUE' ? :true : :false
	readonly = line.split[1] == 'TRUE'

[towo]

Well, I'm a bit confused at this, to be quite honest; the mirrormode setting defined the :true and :false values, which appears to be an accepted way as it's used all throughout various modules, including modules by @puppetlabs. So I just tried to mirror (hah) that, and also pushed f2e809d since the symbol approach needed that.

I could just drop it and go down to ruby booleans, but then there'd be a certain inconsistency, and one that the overall codebase, including @voxpupuli, seems split on. Or have I overlooked a style guide somewhere that defines the way to go?

[bastelfreak]

this won't help you, but: This is an ongoing problem since types and provider exists and everytime I don't know how to handle it properly :(

[smortex]

Deprecation of :true / :false in favor of true / false is quite vague for me. I feel like it was needed with older parser and Puppet 4 fixed that. As you say, there are inconsistencies in the various modules around. Some module even mix both ways: https://github.com/puppetlabs/puppetlabs-vcsrepo/blob/bcae373622364f3088fd6d0c89c4d499c05b3e5d/lib/puppet/type/vcsrepo.rb#L290-L321

This makes me think we can switch to pure true / false but if it cause trouble that's not a major issue.

[tdb]

This is still an issue, and I originally reported it in #207. This PR fixes it and is consistent with the way the rest of the file is done, so could it just be merged?

[smortex]

@tdb this got lost, do you mind opening a PR with these changes in a single commit on top of the master branch?