[rhcos-4.14] tests: add fips.hmac to verify VM will fail to reboot with FIPS and wrong hmac#4474
Conversation
|
Hi @openshift-cherrypick-robot. Thanks for your PR. I'm waiting for a coreos member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a new test, fips.hmac, to verify that a VM with FIPS enabled will fail to reboot if the kernel's HMAC file is corrupted. The changes include adding a new test flag NoDracutFatalCheck to bypass generic dracut failure checks, and the implementation of the test itself. The overall approach is sound. I have one suggestion in the new test file to improve maintainability by replacing a magic number with a named constant.
| // Wait for the boot to fail. Since the HMAC is corrupted, the machine | ||
| // will fail FIPS integrity check and never come back online. | ||
| // Using a 90 second timeout to allow enough time for boot attempt to fail. | ||
| time.Sleep(90 * time.Second) |
There was a problem hiding this comment.
The 90-second sleep duration is a magic number. It's better to define it as a named constant to improve readability and make it easier to adjust if needed. This is particularly important for timeouts in tests, as they can be a source of flakiness if not chosen carefully.
Consider defining it as a constant, for example at the top of the function:
func runFipsHMACTest(c cluster.TestCluster) {
const bootFailTimeout = 90 * time.Second
// ...
time.Sleep(bootFailTimeout)
// ...
}
|
/approve |
2 participants
This is an automated cherry-pick of #4437
/assign HuijingHei