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 389-ds healthcheck output message #6450

Closed
wants to merge 1 commit into from

Conversation

ssidhaye
Copy link
Contributor

With the commit #99a74d7, 389-ds changed the message returned in ipa-healthcheck.

Previously the message was:

"\n\nIn Directory Server, we offer one hash suitable for this " "(PBKDF2_SHA256) and one hash\nfor "legacy" support (SSHA512)." "\n\nYour configuration does not use these for password storage " "or the root password storage\nscheme.\n"

but now the message is:

\n\nIn Directory Server, we offer one hash suitable for this " "(PBKDF2-SHA512) and one hash\nfor "legacy" support (SSHA512)." "\n\nYour configuration does not use these for password storage " "or the root password storage\nscheme.\n"

PBKDF2_SHA256 has been replaced with PBKDF2-SHA512

Pagure: https://pagure.io/freeipa/issue/9238

Signed-off-by: Sumedh Sidhaye ssidhaye@redhat.com

@abbra
Copy link
Contributor

abbra commented Sep 15, 2022

NACK. This change would break the test when it is run against older 389-ds. We don't need the whole phrase to check, first part of it is unique enough for the comparison. Please change the code so that we only compare the first line.

"\n\nIn Directory Server, we offer one hash suitable for this "

and not the rest of the output.

@ssidhaye
Copy link
Contributor Author

@abbra ack, will fix it

@flo-renaud
Copy link
Contributor

@ssidhaye if you rebase your branch, the commit https://pagure.io/freeipa/c/94835d19b5a0e0bb2a4043bf8becc54062e5d9ab?branch=master will be picked and should fix the azure issue.

@flo-renaud
Copy link
Contributor

@ssidhaye thanks for the PR, LGTM. Please remove the temp commit.
Do we need backports to other branches?

@abbra
Copy link
Contributor

abbra commented Oct 6, 2022

@ssidhaye ping, could you please remove the temp commit and answer the question @flo-renaud asked in #6450 (comment) ?

@stanislavlevin
Copy link
Contributor

I confirm that this patch fixes test_ds_configcheck_passwordstorage against 389-ds-2.2.3.

…healthcheck.

Previously the message was:

"\n\nIn Directory Server, we offer one hash suitable for this "
"(PBKDF2_SHA256) and one hash\nfor \"legacy\" support (SSHA512)."
"\n\nYour configuration does not use these for password storage "
"or the root password storage\nscheme.\n"

but now the message is:

\n\nIn Directory Server, we offer one hash suitable for this "
"(PBKDF2-SHA512) and one hash\nfor \"legacy\" support (SSHA512)."
"\n\nYour configuration does not use these for password storage "
"or the root password storage\nscheme.\n"

PBKDF2_SHA256 has been replaced with PBKDF2-SHA512

Pagure: https://pagure.io/freeipa/issue/9238

Signed-off-by: Sumedh Sidhaye <ssidhaye@redhat.com>
@ssidhaye
Copy link
Contributor Author

ssidhaye commented Oct 11, 2022

@flo-renaud @abbra Sorry I was on PTO. Just removed the temp commit. I will check which branches need this fix add the appropriate labels. @stanislavlevin Thanks for verifying the fix.

@ssidhaye ssidhaye added the re-run Trigger a new run of PR-CI label Oct 11, 2022
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Oct 11, 2022
@ssidhaye ssidhaye added the ipa-4-10 Mark for backport to ipa 4.10 label Oct 12, 2022
@flo-renaud flo-renaud added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels Oct 12, 2022
@flo-renaud
Copy link
Contributor

master:

  • 42f73ea With the commit #99a74d7, 389-ds changed the message returned in ipa-healthcheck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-10 Mark for backport to ipa 4.10 pushed Pull Request has already been pushed
Projects
None yet
5 participants