Add check-for-go-fips-test pre-commit hook #18
Conversation
Implements GitHub issue requirements: - Detects packages using go-fips in environment or pipeline - Verifies they have corresponding go-fips test (uses: test/go-fips-check) - Reports failures for packages missing the required test
- Detect any go-fips variant (go-fips, go-fips-md5, etc.) using startswith() - Check each package/subpackage independently for compliance - Require matching tests for each go-fips usage (not just any test) - Provide detailed error messages specifying which component lacks tests - Handle subpackage test sections properly Fixes comprehensive validation of go-fips test requirements.
dannf
left a comment
dannf
left a comment
There was a problem hiding this comment.
The test looks good to me, a few comments about optimizations inside. Would you mind raising this in #eng-os? I'd like to give other people a chance to chime in, since this is establishing a de facto policy (and I'm not really a go-fips expert). @murraybd might be interested in it as well, as a test proliferater himself :)
One note: this test is looking at the YAML file in a pre-compiled state, so it will miss things such as packages with names like ${{vars.expands-to-go-fips}}-foo. If we implemented it as a melange linter or melange compiled it, it wouldn't have that problem. I wonder if there's a way to have one hook do the compilation such that all follow-on tests could consume it w/o recompiling?
| [^/]+\.ya?ml # matches .yaml or .yml files at the top level only | ||
| )$ | ||
| - id: check-for-go-fips-test | ||
| files: '^[^.][^/]*\.yaml$' # matches non-hidden .yaml files at the top level only |
There was a problem hiding this comment.
It might be more efficient to use this to identify the go-fips packages that need to be parsed. If we can filter out packages at this level, we needn't spawn another python interpreter to run the check. Of course, if it might be the case that only a subpackage will match, then that won't work.
| # Check if main package uses go-fips | ||
| main_uses_fips = False | ||
| main_has_test = False | ||
|
|
There was a problem hiding this comment.
I feel like we shouldn't need to duplicate the code between main package parsing and subpkg parsing. See the shellcheck test, where it puts the main package and any sub package in a list, and just iterates through them all the same way. We should probably consider creating a library class that just does stuff like this for us so we can share it.
Summary
Implements GitHub issue #9313 - adds a pre-commit hook that ensures packages using go-fips also have corresponding go-fips tests.
go-fips,go-fips-md5, etc.) in environment packages orgo-packagedeclarationstest/go-fips-checkTest plan
🤖 Generated with Claude Code