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

ipatests: Test ipa-cert-fix warns when startup directive is missing from CS.cfg #5855

Closed
wants to merge 1 commit into from

Conversation

mrizwan93
Copy link
Contributor

@mrizwan93 mrizwan93 commented Jun 24, 2021

Earlier it used to fail when startup directive missing from CS.cfg.
With dogtagpki/pki#3466, it changed to display
a warning than failing.

related: https://pagure.io/freeipa/issue/8890

Signed-off-by: Mohammad Rizwan myusuf@redhat.com

@mrizwan93 mrizwan93 force-pushed the fix-startup-directive branch 2 times, most recently from 32aeb57 to db22edc Compare June 24, 2021 10:04
@flo-renaud
Copy link
Contributor

Hi @mrizwan93
please update the commit message with "Fixes: https://pagure.io/freeipa/issue/8890".

The test needs to have different expectations depending on the version of pki-server that is installed.
If pki-server < 10.11, the current code still applies, but if pki-server>=10.11 your patch applies. You can reuse the function tasks.get_pki_version(host) from ipatests/pytest_ipa/integration/tasks.py.


if (tasks.get_pki_version(self.master)
< tasks.parse_version('10.11.0')):
assert err_msg1 and err_msg2 in result.stderr_text
Copy link
Contributor

Choose a reason for hiding this comment

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

The above assert is equivalent to assert (err_msg1 is not None) and (err_msg2 in stderr), which is not what the test should check.

@mrizwan93 mrizwan93 force-pushed the fix-startup-directive branch 2 times, most recently from 6c96d94 to 88f6550 Compare July 9, 2021 06:33
@flo-renaud
Copy link
Contributor

Hi @mrizwan93
thanks for the patch, LGTM. You can remove the temp commit.
Should we also backport to ipa-4-9?

@flo-renaud flo-renaud self-assigned this Jul 9, 2021
@mrizwan93 mrizwan93 added the ipa-4-9 Mark for backport to ipa 4.9 label Jul 12, 2021
…rom CS.cfg

Earlier it used to fail when startup directive missing from CS.cfg.
With dogtagpki/pki#3466, it changed to display
a warning than failing.

related: https://pagure.io/freeipa/issue/8890

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
@flo-renaud flo-renaud added the ack Pull Request approved, can be merged label Jul 12, 2021
@flo-renaud
Copy link
Contributor

master:

  • ea8f4b6 ipatests: Test ipa-cert-fix warns when startup directive is missing from CS.cfg

@flo-renaud flo-renaud added the pushed Pull Request has already been pushed label Jul 12, 2021
@flo-renaud flo-renaud closed this Jul 12, 2021
@flo-renaud
Copy link
Contributor

Hi @mrizwan93
a manual backport is required for ipa-4-9, can you handle it? Thanks

mrizwan93 added a commit to mrizwan93/freeipa that referenced this pull request Aug 2, 2021
In freeipa#5855 was looking
into stdout_text for warning instead of stderr_text, hence
was failing for pki version > 10.11.0.

related: https://pagure.io/freeipa/issue/8890

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
mrizwan93 added a commit to mrizwan93/freeipa that referenced this pull request Aug 3, 2021
In freeipa#5855 was looking
into stdout_text for warning instead of stderr_text, hence
was failing for pki version > 10.11.0.

related: https://pagure.io/freeipa/issue/8890

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
mrizwan93 added a commit to mrizwan93/freeipa that referenced this pull request Aug 3, 2021
In freeipa#5855 was looking
into stdout_text for warning instead of stderr_text, hence
was failing for pki version > 10.11.0.

related: https://pagure.io/freeipa/issue/8890

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
abbra pushed a commit that referenced this pull request Aug 4, 2021
In #5855 was looking
into stdout_text for warning instead of stderr_text, hence
was failing for pki version > 10.11.0.

related: https://pagure.io/freeipa/issue/8890

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
flo-renaud pushed a commit to flo-renaud/freeipa that referenced this pull request Aug 4, 2021
In freeipa#5855 was looking
into stdout_text for warning instead of stderr_text, hence
was failing for pki version > 10.11.0.

related: https://pagure.io/freeipa/issue/8890

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
abbra pushed a commit that referenced this pull request Aug 4, 2021
In #5855 was looking
into stdout_text for warning instead of stderr_text, hence
was failing for pki version > 10.11.0.

related: https://pagure.io/freeipa/issue/8890

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
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-9 Mark for backport to ipa 4.9 pushed Pull Request has already been pushed
Projects
None yet
2 participants