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

Replace deprecated params in examples #4957

Merged
merged 1 commit into from
Feb 12, 2025
Merged

Replace deprecated params in examples #4957

merged 1 commit into from
Feb 12, 2025

Conversation

edewata
Copy link
Contributor

@edewata edewata commented Feb 11, 2025

The sample config files have been updated to use pki_ds_url instead of pki_ds_ldap_port and pki_ds_ldaps_port since those params have been deprecated since PKI 11.5 and will generate deprecation warnings if used.

https://github.com/edewata/pki/blob/ci/base/server/etc/default.cfg#L88-L92
https://github.com/dogtagpki/pki/blob/master/docs/changes/v11.5.0/Server-Changes.adoc#add-pki_ds_url-parameter
https://github.com/dogtagpki/pki/blob/master/docs/manuals/man5/pki_default.cfg.5.md#internal-database-parameters

@edewata edewata requested a review from ladycfu February 11, 2025 14:58
@edewata edewata changed the title Remove deprecated params from examples Replace deprecated params in examples Feb 11, 2025
Copy link
Contributor

@ladycfu ladycfu left a comment

Choose a reason for hiding this comment

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

  1. I don't see pki_ds_url being introduced in default.cfg. Add?
  2. I don't see where (default.cfg?) pki_ds_ldap_port and pki_ds_ldaps_port are being introduced as deprecated. Add? (we ask people to look at default.cfg for "additional parameters"
  3. are we sure no installation would expect both ports to be present?

@edewata
Copy link
Contributor Author

edewata commented Feb 11, 2025

The pki_ds_* params that define the DS connection were actually deprecated in PKI 11.5 and replaced with a single pki_ds_url param. We have documented this under docs/changes and docs/manuals, also pkispawn will generate a deprecation warning if the old params are used, but I just added a note in default.cfg about the deprecation (see links above).

The pki_ds_url itself is not defined in default.cfg since the value can be specified by the user or dynamically constructed from the old params (for backward compatibility), so it can't have a default static value. See:
https://github.com/dogtagpki/pki/blob/master/base/server/python/pki/server/deployment/__init__.py#L201-L217

The pki_ds_secure_connection is boolean, so we only need either the plain LDAP port or the secure one.

Copy link
Contributor

@ladycfu ladycfu left a comment

Choose a reason for hiding this comment

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

LGTM

The sample config files have been updated to use pki_ds_url
instead of pki_ds_ldap_port and pki_ds_ldaps_port since those
params have been deprecated since PKI 11.5 and will generate
deprecation warnings if used.
@edewata
Copy link
Contributor Author

edewata commented Feb 12, 2025

@ladycfu Thanks! I had to return pki_ds_hostname to the original location since for some reason it's breaking a CI test.

@edewata edewata merged commit 28cdc45 into dogtagpki:master Feb 12, 2025
170 of 176 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants