-
Notifications
You must be signed in to change notification settings - Fork 248
[AI Generated]: fix fips skip unsupported Ubuntu kernels - #4701
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.
🟡 Changes recommended
The updated Debian/Ubuntu enablement test now depends on node.tools[Fips], but the tool factory does not currently support Debian, which will cause the test to fail with UnsupportedDistroException.
Key Test Cases:
verify_fips_enablement|verify_azl_fips_status
Impacted LISA Features:
StartStop
Tested Azure Marketplace Images:
- canonical 0001-com-ubuntu-server-jammy 22_04-lts latest
- debian debian-12 12 latest
- microsoftcblmariner azure-linux-3 azure-linux-3 latest
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the FIPS tooling and security test coverage to better handle cases where /proc/sys/crypto/fips_enabled may be missing (treating it as "not in FIPS mode"), and refactors the Debian/Ubuntu enablement flow to use the shared Fips.is_kernel_fips_mode() helper.
Changes:
- Extend
lisa.tools.Fipsfactory logic to return a tool instance on Ubuntu, and makeis_kernel_fips_mode()tolerant of missing/proc/sys/crypto/fips_enabled. - Refactor Debian/Ubuntu FIPS enablement verification to use
Fips.is_kernel_fips_mode()and improve the post-enable assertion message. - Mark the node dirty before changing GRUB kernel cmdline and rebooting.
File summaries
| File | Description |
|---|---|
| lisa/tools/fips.py | Adds Ubuntu support in the tool factory and makes kernel FIPS mode detection tolerant to missing /proc flag. |
| lisa/microsoft/testsuites/security/fips.py | Uses Fips.is_kernel_fips_mode() in the Debian/Ubuntu path and marks the node dirty before GRUB changes + reboot. |
Review details
Suppressed comments (1)
lisa/tools/fips.py:42
FipsTests._verify_fips_enablement_debian()callsnode.tools[Fips]on Debian, but the factory currently only creates a tool instance for Ubuntu (and CBLMariner). This will raiseUnsupportedDistroExceptionon Debian and break the test; include Debian in the factory branch (and update the error message to match).
if isinstance(node.os, Ubuntu):
return Fips(node, args, kwargs)
raise UnsupportedDistroException(
os=node.os,
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
LiliDeng
commented
Sep 4, 2026
AI Test Case Selection
Selected 3 test case(s): verify_openssl_basic,verify_golang_sys_crypto,verify_openssl_speed_test
Marketplace image: microsoftcblmariner azure-linux-3 azure-linux-3 latest
Result: Succeeded
8509c5f to
ef8206c
Compare
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.
🟡 Changes recommended
The Fips tool is now exposed on Debian-based OSes but still contains CBL-Mariner-specific enable/disable semantics, which creates a correctness risk for future call sites.
Key Test Cases:
verify_fips_enablement|verify_azl_fips_status
Impacted LISA Features:
Disk
Tested Azure Marketplace Images:
- canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
- debian debian-12 12-gen2 latest
- microsoftcblmariner azure-linux-3 azure-linux-3-gen2 latest
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
LiliDeng
commented
Sep 4, 2026
AI Test Case Selection
Selected 3 test case(s): verify_openssl_basic,verify_golang_sys_crypto,verify_openssl_speed_test
Marketplace image: microsoftcblmariner azure-linux-3 azure-linux-3 latest
Result: Failed
ef8206c to
75ae7ba
Compare
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.
🟡 Changes recommended
The Debian/Ubuntu path should also skip when /proc/sys/crypto/fips_enabled is missing; otherwise the test may proceed into an enable/verify flow that cannot succeed on unsupported kernels.
Key Test Cases:
verify_fips_enablement|verify_azl_fips_status
Impacted LISA Features:
Disk
Tested Azure Marketplace Images:
- canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
- debian debian-12 12-gen2 latest
- microsoftcblmariner azure-linux-3 azure-linux-3-gen2 latest
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lisa/tools/fips.py:45
- The factory now returns the Fips tool for Debian-based OS, but the tool's enable/disable/assert paths are AZL/CBLMariner-specific (e.g., dracut-fips package handling). The error message currently implies broad Debian support; consider clarifying that Debian-based support is limited to probing /proc/sys/crypto/fips_enabled unless full Debian enablement is implemented.
if isinstance(node.os, Debian):
return Fips(node, args, kwargs)
raise UnsupportedDistroException(
os=node.os,
message="FIPS tool only supports CBLMariner 2.0/3.0 and Debian-based OS.",
)
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
LiliDeng
commented
Sep 4, 2026
AI Test Case Selection
Selected 3 test case(s): verify_openssl_basic,verify_golang_sys_crypto,verify_openssl_speed_test
Marketplace image: microsoftcblmariner azure-linux-3 azure-linux-3 latest
Result: Succeeded
Uh oh!
There was an error while loading. Please reload this page.
Description
Related Issue
Type of Change
Checklist
Test Validation
Key Test Cases:
Impacted LISA Features:
Tested Azure Marketplace Images:
Test Results