-
Notifications
You must be signed in to change notification settings - Fork 64
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
LDAPS security profile ansible support #414
LDAPS security profile ansible support #414
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.
The proposed functional code changes and regression test results look good. I am requesting a couple of changes to this PR to improve UT test coverage and to post the full output for the regression test results:
- Please add UT coverage for the new option. (No new test cases are needed, but please make the needed changes in the "tests/unit/modules/network/sonic/fixtures/sonic_ldap.yaml" file.)
- Also, please attach a new version of the regression results that doesn't chop off the rightmost columns. This can be done by printing to a file in "landscape" (instead of "portrait") mode and scaling the output as needed so it fits.
New regression report: |
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.
With the latest updates, all proposed code changes, including new test cases, and all corresponding test results look good.
Thank you for providing this enhancement to the functionality of the sonic_ldap resource module.
Approved.
SUMMARY
Added the changes required for LDAP security profile
GitHub Issues
List the GitHub issues impacted by this PR. If no Github issues are affected, please indicate this with "N/A".
ISSUE TYPE
COMPONENT NAME
sonic_ldap
OUTPUT
ldaps_security_profile regression report.pdf
Checklist:
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration