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 readonly handling #358

Draft
wants to merge 26 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3302811
Update FreeBSD default version for OpenLDAP
smortex Jul 13, 2022
b8fffd8
Fix readonly handling
towo Aug 25, 2022
bc0cd00
Needs `newvalues` anolagous to mirromode to have symbols available
towo Aug 30, 2022
9840c62
Add spec c/o @smortex
smortex Aug 31, 2022
e10d002
Linter and spec fixes
towo Aug 31, 2022
efc3ae2
Implement a test for readonly being set to true
towo Aug 31, 2022
bfda214
fixup! Implement a test for readonly being set to true
towo Aug 31, 2022
2623158
fixup! Implement a test for readonly being set to true
towo Aug 31, 2022
617816a
fixup! Implement a test for readonly being set to true
towo Aug 31, 2022
1b47094
fixup! Implement a test for readonly being set to true
towo Aug 31, 2022
d0fe5db
fixup! Implement a test for readonly being set to true
towo Aug 31, 2022
0737c84
fixup! Implement a test for readonly being set to true
towo Aug 31, 2022
eca43b8
fixup! Implement a test for readonly being set to true
towo Aug 31, 2022
00cd63e
fixup! Implement a test for readonly being set to true
towo Aug 31, 2022
12078a1
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
21b822f
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
cec2110
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
91b30c8
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
3fafa30
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
ec7e5c6
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
f26427f
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
405d793
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
a297ef1
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
6da413a
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
cc08532
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
936c78b
fixup! Implement a test for readonly being set to true
towo Sep 1, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions data/os/FreeBSD.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
---
openldap::client::package: "openldap24-client"
openldap::client::package: "openldap26-client"
openldap::client::file: "/usr/local/etc/openldap/ldap.conf"
openldap::server::confdir: "/usr/local/etc/openldap/slapd.d"
openldap::server::conffile: "/usr/local/etc/openldap/slapd.conf"
openldap::server::package: "openldap24-server"
openldap::server::package: "openldap26-server"
openldap::server::escape_ldapi_ifs: true
openldap::server::ldapi_ifs:
- "/var/run/openldap/ldapi"
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/provider/openldap_database/olc.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def self.instances
when %r{^olcRelay: }
relay = line.split[1]
when %r{^olcReadOnly: }i
readonly = line.split[1]
readonly = line.split[1] == 'TRUE' ? :true : :false
Copy link
Member

Choose a reason for hiding this comment

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

I think we can use native types here:

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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 :(

Copy link
Member

Choose a reason for hiding this comment

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

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.

when %r{^olcSizeLimit: }i
sizelimit = line.split[1]
when %r{^olcDbMaxSize: }i
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/type/openldap_database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ def should_to_s(_newvalue)

newproperty(:readonly) do
desc 'Puts the database into read-only mode.'
newvalues(:true, :false)
defaultto(:false)
end

newproperty(:sizelimit) do
Expand Down
55 changes: 55 additions & 0 deletions spec/unit/puppet/provider/openldap_database/olc_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# frozen_string_literal: true

require 'spec_helper'

describe Puppet::Type.type(:openldap_database).provider(:olc) do
let(:params) do
{
suffix: 'dc=example,dc=com',
backend: 'mdb',
readonly: false,
# provider: described_class.name,
}
end

let(:resource) do
Puppet::Type.type(:openldap_database).new(params)
end
let(:provider) do
resource.provider
end

before do
allow(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
allow(provider).to receive(:slapcat)
allow(provider).to receive(:ldapmodify)
allow(provider).to receive(:ldapadd)
# allow(described_class).to receive(:slapcat)
# allow(described_class).to receive(:ldapmodify)
# allow(described_class).to receive(:ldapadd)
end

describe 'when creating' do
context 'with readonly set to false' do
it 'parses olcReadOnly as false' do
provider.create
expect(described_class.instances.first.readonly).to eq :false
# expect(described_class.instances.first.readonly).to eq(:false)
end
end

context 'with readonly set to true' do
let(:params) do
super().merge({ readonly: true })
end

it 'parses olcReadonly' do
expect(described_class.instances.first.readonly).to eq(:true)
end
end
end
end