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

Rework openldap::server::database interface for the syncrepl parameter #412

Merged
merged 5 commits into from
Apr 7, 2024

Conversation

smortex
Copy link
Member

@smortex smortex commented Mar 26, 2024

The current syncrepl data type (Optional[Variant[String[1],Array[String[1]]]]) is tedious and error prone (#221, #407). Thanks to data types, we can improve validation at the Puppet level, making it easier for the user to write a catalog that configure replication using the module, and harder to provide invalid OpenLDAP configuration.

Because of the new validation, this PR is a breaking change. The interface is therefore adjusted to mandate an Array of replication configurations, even if a single one is required. This is intended to make the interface simpler.

The provided configuration is now validated with a dedicated Openldap::Syncrepl data type, which is serialized to the previously expected string by the openldap::server::database class and passed to the openldap_database puppet type.

Fixes: #221

Updating your configuration

If you are using the syncrepl parameter, you need to update your manifests when updating the module.

Transform a configuration like:

openldap::server::database { 'dc=example,dc=com':
  ensure   => present,
  syncrepl => 'rid=1 provider=ldap://server2.example.net searchbase="dc=example,dc=com"',
}

openldap::server::database { 'dc=example,dc=net':
  ensure   => present,
  syncrepl => [
    'rid=1 provider=ldap://server2.example.net searchbase="dc=example,dc=net"',
    'rid=2 provider=ldap://server3.example.net searchbase="dc=example,dc=net"',
  ],
}

Into this:

openldap::server::database { 'dc=example,dc=com':
  ensure   => present,
  syncrepl => [
    {
      rid        => 1,
      provider   => 'ldap://server2.example.com',
      searchbase => 'dc=example,dc=net',
    },
  ],
}

openldap::server::database { 'dc=example,dc=net':
  ensure   => present,
  syncrepl => [
    {
      rid        => 1,
      provider   => 'ldap://server2.example.net',
      searchbase => 'dc=example,dc=net',
    },
    {
      rid        => 2,
      provider   => 'ldap://server3.example.net',
      searchbase => 'dc=example,dc=net',
    },
  ],
}

Optional[realm] => String[1],
Optional[secprops] => String[1],
}
]
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good, but a couple options are missing:

Optional[logbase] => String[1],
Optional[logfilter] => String[1],
Optional[retry] => String[1],
Optional[syncdata] => String[1],
Optional[sizeLimit] => Variant[Integer[0], Enum['unlimited']],
Optional[timelimit] => Variant[Integer[0], Enum['unlimited']],

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a PR against this branch with the missing fields I found

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice! Do you think this interface is better and we should go this way?

@Valantin
Copy link
Contributor

Intresting, a simil work can be good for limit to

@smortex
Copy link
Member Author

smortex commented Mar 27, 2024

Intresting, a simil work can be good for limit to

@Valantin I am not sure to follow (missing words?), is it related to the changes @coreone did?

* Add missing documentation;
* Fix wrong data types;
* Correctly serialize parameters without arguments.
@Valantin
Copy link
Contributor

@smortex I mean, a struct to set olcLimits array similar to what you do to olcSyncprov parameter.

REFERENCE.md Outdated Show resolved Hide resolved
@smortex smortex changed the title Rework syncrepl Rework openldap::server::database interface for the syncrepl parameter Mar 27, 2024
@tparkercbn
Copy link

I use the syncrepl feature and thing this PR is great! It will make defining syncrepl entries in hiera much cleaner and easier to define and debug. Thanks @smortex for this work. Hope to see it merged soon :)

types/syncrepl.pp Outdated Show resolved Hide resolved
@smortex
Copy link
Member Author

smortex commented Mar 30, 2024

@smortex I mean, a struct to set olcLimits array similar to what you do to olcSyncprov parameter.

@Valantin Thanks! I added you as reviewer on #414. Just like syncrepl, I don't use this in my home setup so your experience may help spot mistakes I made.

It is not uncommon to set the `rid` as a fixed-width string with 0
padding. For Puppet, `001` is the octal number 1 and the fixed-width is
"lost", so add the ability to pass it as a string of up to three digits
like `"001"` which will be kept unchanged.
@smortex smortex marked this pull request as ready for review April 4, 2024 01:52
@smortex
Copy link
Member Author

smortex commented Apr 4, 2024

I think this is ready to ship!

Available parameters have not changed between these versions, but doc
related to the older version will likely disapear before the one of the
latest version.
@smortex smortex added this to the 8.0.0 milestone Apr 4, 2024
@smortex smortex merged commit 583a368 into master Apr 7, 2024
30 checks passed
@smortex smortex deleted the rework-syncrepl branch April 7, 2024 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Syncrepl needs a better documentation
5 participants