Skip to content

Conversation

@Harinadh-Saladi
Copy link

Added additional test cases and modifed validations.md file

- Implemented new check function to detect disabled cipher configuration issues
- Added comprehensive test suite with 6 test cases covering all scenarios
- Update validations.md documentation with check details and recommendations
Copy link

@lovkeshsharma702 lovkeshsharma702 left a comment

Choose a reason for hiding this comment

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

logic is not correct. it just verify the version and fail, In fab3 apic1, all cph are enabled and no nginx error but the check still fail.
Please work on correcting, testing, validating the main function and test cases.

cipher_api = "commCipher.json?query-target-filter=and(or(wcard(commCipher.id,\"ECDHE-RSA\"),wcard(commCipher.id,\"DHE-RSA\"),wcard(commCipher.id,\"TLS_AES_256\")),eq(commCipher.state,\"disabled\"))"
try:
disabled_ciphers = icurl("class", cipher_api)
disabled_cipher_count = len(disabled_ciphers)

Choose a reason for hiding this comment

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

why do we need to use len in this case? please use totalcount value to validate.

Copy link
Author

@Harinadh-Saladi Harinadh-Saladi Dec 18, 2025

Choose a reason for hiding this comment

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

Hi Lovkesh, The entire codebase uses the same pattern - working directly with the array returned by icurl function.
Also I felt more efficient using length, since it doesn't needs extra parsing whereas totalcount needs extra parsing from string to int.

@Harinadh-Saladi
Copy link
Author

logic is not correct. it just verify the version and fail, In fab3 apic1, all cph are enabled and no nginx error but the check still fail. Please work on correcting, testing, validating the main function and test cases.

Hi Lovkesh. Thanks for sharing your comment. I have debugged and done the changes. After testing it's working as anticipated. Actually the logic is correct only but the string "Failed to write nginxproxy conf file" appears in the command itself, so even when zgrep returns empty result, command echo still contains this string.

So it was a false positive, detecting the search term from the command, but not from actual grep results.

Have fixed nginx log check by filtering out command echo and prompt correctly. Now script will detect FOUND only if actual grep results contain error message. Have re-pushed the code.

@Harinadh-Saladi
Copy link
Author

Enclosing the pytest logs for the same.
Pytest logs.txt

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