fix: correct syncrepl option passed to openldap_database if type Array

[coreone]

Pull Request (PR) description

The defined type openldap::server::database shows that an Array can be passed to the syncrepl parameter,. However, if you pass an Array of Strings, an error is thrown because it shows the provider sending an array of individual parameters instead of what it is expecting, which is a String of those parameters joined with a single space. This fix modifies the defined type to send the provider what it expects. Another fix would be to make the provider treat Array values differently, but I am not fluent in Ruby, so I instead went this route.

This Pull Request (PR) fixes the following issues

Fixes #221

This should make an example database as follows to be configured as expected:

openldap::server::databases:
  'dc=example,dc=org':
    ensure: 'present'
    backend: 'bdb'
    dboptions:
      cachesize: '10000'
      checkpoint: '128 15'
      dbconfig:
        - set_cachesize 0 268435456 1
        - set_lg_regionmax 262144
        - set_lg_bsize 2097152
    rootdn: 'cn=Manager,dc=example,dc=org'
    rootpw: 'somesecretvaluehere'
    sizelimit: 'unlimited'
    syncrepl:
      - 'rid=102'
      - 'provider=ldaps://ldap.example.org:636'
      - 'searchbase=dc=example,dc=org'
      - 'type=refreshOnly'
      - 'interval=60'
      # The first 10 tries will occur once every 60 seconds, followed by 3 attempts every 300 seconds
      - 'retry="60 10 300 3"'
      - 'filter=(objectClass=*)'
      - 'scope=sub'
      - 'schemachecking=off'
      - 'bindmethod=simple'
      - 'binddn=cn=syncrepl,dc=example,dc=org'
      - 'credentials=somesecretvaluehere'
      - 'tls_cacert=/etc/pki/tls/certs/ca-bundle.crt'
    timelimit: 'unlimited'

[smortex]

I would just have:

Suggested change

	syncrepl => $_final_syncrepl,
	syncrepl => Array($syncrepl).join(' '),

Can you add a test to ensure we don't break it?

[coreone]

I looked at this again, and now that I see the unit test, I think I did something different than the original design for that parameter. It looks like String and Array are supported types because you can either pass a String with the settings or you can pass an Array of strings, each of which is the total configuration for a syncrepl. So, at this point I'm not sure how to proceed. I don't want to change the default behavior people expect, but I also don't think its manageable to potentially have a one-line string with all those parameters.

The way I would probably move forward would still change the default behavior by just requiring an Array of Arrays. Something like:

openldap::server::databases:
  'dc=example,dc=org':
    syncrepl:
      -
        - 'rid=102'
        - 'provider=ldaps://ldap.example.org:636'
        - 'searchbase=dc=example,dc=org'
        - 'type=refreshOnly'
        - 'interval=60'
        - 'retry="60 10 300 3"'
        - 'filter=(objectClass=*)'
        - 'scope=sub'
        - 'schemachecking=off'
        - 'bindmethod=simple'
        - 'binddn=cn=syncrepl,dc=example,dc=org'
        - 'credentials=somesecretvaluehere'
        - 'tls_cacert=/etc/pki/tls/certs/ca-bundle.crt'
      -
        - 'rid=2'
        - 'provider=ldap://localhost'
        - 'searchbase="dc=foo,dc=example,dc=com"'

or maybe a Hash of Arrays so it doesn't have such awkward syntax:

openldap::server::databases:
  'dc=example,dc=org':
    syncrepl:
      rid_102:
        - 'rid=102'
        - 'provider=ldaps://ldap.example.org:636'
        - 'searchbase=dc=example,dc=org'
        - 'type=refreshOnly'
        - 'interval=60'
        - 'retry="60 10 300 3"'
        - 'filter=(objectClass=*)'
        - 'scope=sub'
        - 'schemachecking=off'
        - 'bindmethod=simple'
        - 'binddn=cn=syncrepl,dc=example,dc=org'
        - 'credentials=somesecretvaluehere'
        - 'tls_cacert=/etc/pki/tls/certs/ca-bundle.crt'
      rid_2:
        - 'rid=2'
        - 'provider=ldap://localhost'
        - 'searchbase="dc=foo,dc=example,dc=com"'

Thoughts?

[coreone]

In the interim, I also updated as you suggested, but it breaks if syncrepl is undef. So, I modified slightly.

[smortex]

I don't use syncrepl, I think I missed a key point: you can have multiple olcSyncRepl entries and each one is composed of multiple key/values.

So the structural change you suggest seems a good move.

IMHO if we want to make the interface more user-friendly, we could switch to an Array of Struct that can verify all the keys and values passed to syncrepl. Your second example would be something like:

openldap::server::databases:
  'dc=example,dc=org':
    syncrepl:
      -
        rid: 102
        provider: ldaps://ldap.example.org:636
        searchbase: dc=example,dc=org
        type: refreshOnly
        interval: 60
        retry: 60 10 300 3
        filter: (objectClass=*)
        scope: sub
        schemachecking: off
        bindmethod: simple
        binddn: cn=syncrepl,dc=example,dc=org
        credentials: somesecretvaluehere
        tls_cacert: /etc/pki/tls/certs/ca-bundle.crt
      -
        rid: 2
        provider: ldap://localhost
        searchbase: dc=foo,dc=example,dc=com

That would of course be a backward-incompatible change but that's very reasonable IMHO.

[smortex]

I opened #412 to show what was on my mind (may need more adjustments). If you think this make sense, can you give it a try?

[coreone]

I tested #412 (with the modifications in my PR) on my ldap server and it works very well!

[coreone]

Closing this in deference to #412