FIPS with classifier #772
FIPS with classifier #772
Conversation
|
|
||
| # allow custom python to be used | ||
| parser = argparse.ArgumentParser() | ||
| parser.add_argument('--classifier') |
There was a problem hiding this comment.
I use the argparser instead of env_variable because, in CI, linux-musl-armv7 needs to pass the env var into the docker, which is not easy to do.
But for "CRT_FIPS", I also want our test to get the info and verify the run time check for FIPS is working as expected.
graebm
left a comment
graebm
left a comment
There was a problem hiding this comment.
didn't get all the way through, but here are comments as I head out for the day
Co-authored-by: Michael Graeb <graebm@amazon.com>
…and don't know why
CMakeLists.txt
Outdated
|
|
||
| project(aws-crt-jni C) | ||
| option(BUILD_DEPS "Builds aws common runtime dependencies as part of build" ON) | ||
| option(CRT_FIPS "Whether to build with FIPS compliance" OFF) |
There was a problem hiding this comment.
debatable: I see you failing if FIPS=ON and this is 32bit linux, but there are still a lot of other cases where we do NOT fail (e.g. Windows, Apple, musl, armv6/7)
I'm just spooked about us thinking we're making FIPS builds but one day a year from now, one of the many intermediate steps for it to actually be FIPS doesn't work right during the release process
There was a problem hiding this comment.
Maybe add a check later in the file like if (CRT_FIPS AND NOT FIPS) fail fail fail
since we only set FIPS=ON when going down the aws-lc path
| # prepare FIPS uber jar, download the regular libs | ||
| - aws s3 cp --recursive s3://aws-crt-java-pipeline/${GIT_TAG}/lib $CODEBUILD_SRC_DIR/aws-crt-java/target/cmake-build/lib | ||
| # Override with the FIPS libs | ||
| - aws s3 cp --recursive s3://aws-crt-java-pipeline/${GIT_TAG}/fips_lib $CODEBUILD_SRC_DIR/aws-crt-java/target/cmake-build/lib |
There was a problem hiding this comment.
again, spooked about accidentally uploading fips jar without the expected fips libs. Can we assert that s3://.../fips_lib has the expected files in it?
There was a problem hiding this comment.
well, the fips shared lib will have the same structure and name as the regular one, I don't think we can assert on the files to make sure it works.
We can probably run a test instead.
Co-authored-by: Michael Graeb <graebm@amazon.com>
| - aws s3 cp --recursive s3://aws-crt-java-pipeline/${GIT_TAG}/fips_lib $CODEBUILD_SRC_DIR/aws-crt-java/target/cmake-build/lib | ||
| # Run a test to make sure we get the FIPS libs | ||
| - export CRT_FIPS=ON | ||
| - mvn test -Dtest=software.amazon.awssdk.crt.test.SystemInfoTest -Dshared-lib.skip=true |
There was a problem hiding this comment.
trivial: cool! but let's be a bit more explicit
| - mvn test -Dtest=software.amazon.awssdk.crt.test.SystemInfoTest -Dshared-lib.skip=true | |
| - mvn test -Dtest=software.amazon.awssdk.crt.test.SystemInfoTest.testIsFIPS -Dshared-lib.skip=true |
CMakeLists.txt
Outdated
|
|
||
| project(aws-crt-jni C) | ||
| option(BUILD_DEPS "Builds aws common runtime dependencies as part of build" ON) | ||
| option(CRT_FIPS "Whether to build with FIPS compliance" OFF) |
There was a problem hiding this comment.
Maybe add a check later in the file like if (CRT_FIPS AND NOT FIPS) fail fail fail
since we only set FIPS=ON when going down the aws-lc path
2 participants
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.