New validation for CFD CSCwf26626 #303
Conversation
- 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
…lidations.md file as it was having a typo
lovkeshsharma702
left a comment
lovkeshsharma702
left a comment
There was a problem hiding this comment.
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. |
| 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) |
There was a problem hiding this comment.
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.
|
Enclosing the pytest logs for the same. |
…he script execution for earlier issue as a WA
|
Code logic looks good . |
aci-preupgrade-validation-script.py
Outdated
|
|
||
| return Result(result=result, headers=headers, data=data, recommended_action=recommended_action, doc_url=doc_url) | ||
|
|
||
| <<<<<<< HEAD |
- Removed <<<<<<< HEAD, =======, and >>>>>>> upstream/master markers - Kept both disabled_cipher_check and apic_vmm_inventory_sync_faults_check functions - Fixed improper merge from commit 6cb1209
3 participants
Added additional test cases and modifed validations.md file