[rhcos-4.17] - Backport failing test for FIPS & LUKS #4259
Conversation
![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 to verify that cryptsetup lukscreate fails with FIPS-incompatible algorithms. The overall logic of the test is sound. I've made a couple of suggestions to improve code quality and robustness: one related to error handling to preserve stack traces for better debugging, and another to simplify a helper function and make it more robust against invalid inputs.
| if err := ignitionFailure(c); err != nil { | ||
| c.Fatal(err.Error()) | ||
| } |
There was a problem hiding this comment.
Calling err.Error() can discard useful information, such as a stack trace if the error was created with pkg/errors. It's better to pass the error object directly to c.Fatal() for better debugging.
| if err := ignitionFailure(c); err != nil { | |
| c.Fatal(err.Error()) | |
| } | |
| if err := ignitionFailure(c); err != nil { | |
| c.Fatal(err) | |
| } |
| // Read file and verify if it contains a pattern | ||
| // 1. Read file, make sure it exists | ||
| // 2. regex for pattern | ||
| func fileContainsPattern(path string, searchPattern string) (bool, error) { | ||
| file, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| // File has content, but the pattern is not present | ||
| match := regexp.MustCompile(searchPattern).Match(file) | ||
| if match { | ||
| // Pattern found | ||
| return true, nil | ||
| } | ||
| // Pattern not found | ||
| return false, nil | ||
| } |
There was a problem hiding this comment.
This function can be simplified. The comments are noisy and partially incorrect. Using regexp.MustCompile on a pattern passed as an argument is risky as it will panic on an invalid pattern. A better approach is to use a function like regexp.Match which returns an error on compilation failure, making the function more robust.
// fileContainsPattern reads a file and reports whether it contains a match for a regular expression.
func fileContainsPattern(path string, searchPattern string) (bool, error) {
file, err := os.ReadFile(path)
if err != nil {
return false, err
}
return regexp.Match(searchPattern, file)
}
Backporting #4181