-
Notifications
You must be signed in to change notification settings - Fork 270
test: check is an2ln plugin is disabled or not #8145
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces two new tests to verify the disabling of the 'an2ln' Kerberos localauth plugin in different environments (AD/IPA and LDAP). The tests check for the presence or absence of the localauth plugin configuration file and ensure that Kerberos TGT is available after logging in as a test user. I have identified a potential issue in the first test where the regular expression pattern might not be robust enough to handle variations in the path to the sssd_krb5_localauth_plugin.so module.
f5d64b3 to
b854636
Compare
c78529a to
33fd8ac
Compare
aplopez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
|
||
| @pytest.mark.importance("critical") | ||
| @pytest.mark.topology(KnownTopology.LDAP) | ||
| def test_disable_an2ln_kdc(client: Client, provider: GenericProvider, kdc: KDC): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the very least test name doesn't match what it does.
Test doesn't make sure 'an2ln' is disabled, it makes sure config snippet is absent.
And I'm not sure this test is really needed at all... imo, definitely not importance("critical")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
I changed the name and made the importance "high". I think this test is important because we want to make sure the file isn't present in the plain krb5 case because it's creation cannot be disabled otherwise.
bye,
Sumit
33fd8ac to
aabb521
Compare
The Kerberos 'an2ln' localauth plugin should be disabled in AD and IPA environments where SSSD's localauth plugin can handle the mapping. In a plain Kerberos environment libkrb5 defaults should be used. Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Alejandro López <allopez@redhat.com>
aabb521 to
80d5286
Compare
The Kerberos 'an2ln' localauth plugin should be disabled in AD and IPA environments where SSSD's localauth plugin can handle the mapping. In a plain Kerberos environment libkrb5 defaults should be used.
Please note, this test is expected to fail until #8136 is commited.