Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable FIPS compliancy in Makefile #6071

Merged
merged 5 commits into from
Oct 31, 2022
Merged

Enable FIPS compliancy in Makefile #6071

merged 5 commits into from
Oct 31, 2022

Conversation

naemono
Copy link
Contributor

@naemono naemono commented Oct 5, 2022

This potential change will enable FIPS compliancy for the operator build by making the changes:

  1. Add an ENABLE_FIPS environment variable which will add a build flag goexperiment.boringcrypto to the existing GO_TAGS variable which we use during go build and docker build operations.
  2. Append -fips to all container build names that we push to existing registries.
  3. Adjusts the release pipeline in Jenkins to run another docker build operation with ENABLE_FIPS=true environment variable set.

This change does not

1. Add any Jenkins/Buildkite changes that would allow FIPS these changes to be triggered in our current/future CI system.

@naemono naemono added >enhancement Enhancement of existing functionality discuss We need to figure this out labels Oct 5, 2022
@naemono naemono marked this pull request as ready for review October 17, 2022 13:47
@@ -38,6 +38,7 @@ pipeline {
steps {
sh '.ci/setenvconfig build'
sh 'make -C .ci license.key TARGET=ci-release ci'
sh 'make -C .ci license.key TARGET=build-operator-multiarch-image ci ENABLE_FIPS=true'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to do a whole other step here, with a different target in setenvconfig? It seems overkill when looking at this, but setenvconfig is used to set all env variables in all other steps, but since the build case block is split between nightly and release already, it seemed adding a 3rd would be ugly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this approach to call again the build in FIPS mode using an environment variable. I don't think we should rely on setenvconfig for this. Also I wonder if we should not make this builds in parallel with the old one.

stage('build') {
    failFast true
    parallel {
        stage("build operator image") {
            steps {
                sh '.ci/setenvconfig build'
                sh 'make -C .ci license.key TARGET=ci-release ci'
            }
        }
        stage("build operator image in FIPS mode") {
            steps {
                sh '.ci/setenvconfig build'
                sh 'make -C .ci license.key TARGET=ci-release ci ENABLE_FIPS=true'
            }
        }
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the parallel build makes perfect sense. I've updated this PR with this new parallel build phase.

@naemono naemono changed the title DRAFT: Enable FIPS compliancy in Makefile Enable FIPS compliancy in Makefile Oct 17, 2022
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@naemono naemono merged commit dcd4f32 into elastic:main Oct 31, 2022
@naemono naemono deleted the fips branch October 31, 2022 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss We need to figure this out >enhancement Enhancement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants