Enable unit tests with -tags=requirefips#43611
Enable unit tests with -tags=requirefips#43611michel-laterman merged 15 commits intoelastic:mainfrom
Conversation
michel-laterman
added
:Testing
release-note:skip
botelastic
bot
added
needs_team
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
pierrehilbert
added
Team:Elastic-Agent-Data-Plane
|
Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform) |
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
|
Pinging @elastic/sec-deployment-and-devices (Team:Security-Deployment and Devices) |
|
@elastic/sec-linux-platform, @elastic/observablt-ci can I please get a review for this PR? |
andrewkroh
left a comment
andrewkroh
left a comment
There was a problem hiding this comment.
I'm very curious to see if we can implement some of the conditional testing via runtime checks rather than build-time tags. There may be cases where this is unavoidable if the tests themselves have direct dependencies on crypto functions, but I imagine most of this can be done via runtime checks (e.g., if fips140.Enabled() { t.Skip() }).
As we have seen with the integration build tags, this pattern can make maintenance/refactoring more difficult and may also hide errors.
| // specific language governing permissions and limitations | ||
| // under the License. | ||
|
|
||
| //go:build requirefips |
There was a problem hiding this comment.
Is there way to accomplish the variant testing without utilizing build tags? Minimizing the amount of code behind build tags makes maintenance easier. Build tags can hide errors.
For example, is there some runtime method of checking if the binary is in FIPS mode (akin to https://pkg.go.dev/crypto/fips140#Enabled) that we can use to skip tests at runtime instead of using build tags?
There was a problem hiding this comment.
fips140.Enabled() returns true if fips140 is set to on, or only.
On enables FIPS compliant algorithms, and will allow you to use non-FIPS algorithms (such as SHA1)
Only forces you to use FIPS (fips 140-3) compliance, without using non-FIPS algorithms.
Our current binaries (built with microsoft/go target fips 140-2) function the same as if the flag has an on value.
The next step in our FIPS testing will be to run these unit tests with fips140=only just to make sure we don't accidentally add non compliant algorithms in the future.
There was a problem hiding this comment.
I didn't mean to use fips140.Enabled() directly, but was asking if there was something similar in nature that we could use to minimize the amount code hidden behind build tags? One possible example would be to have a very slim package that exports a constant based on the build tag used, e.g.
//go:build requirefips
const FIPS = true
//go:build !requirefips
const FIPS = false
Then utilize this value to control the expectations set by the tests.
There was a problem hiding this comment.
Currently we don't have anything available in beats.
I'll create an issue to discuss this; enabling test coverage is our priority at the moment
Enable unit tests with -tags=requirefips for auditbeat, filebeat, metricbeat, and libbeat. (cherry picked from commit 01ae298)
* Enable unit tests with -tags=requirefips (#43611) Enable unit tests with -tags=requirefips for auditbeat, filebeat, metricbeat, and libbeat. (cherry picked from commit 01ae298) * Split up root_test for FIPS TLS changes --------- Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> Co-authored-by: michel-laterman <michel.laterman@elastic.co>
6 participants
Proposed commit message
Enable unit tests with -tags=requirefips for auditbeat, filebeat, and metricbeat.
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration filesI have added an entry inCHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Disruptive User Impact
N/A
How to test this PR locally
FIPS=true mage unitTest