Skip to content

Add FIPS specific testclusters configuration#41199

Merged
alpar-t merged 1 commit intoelastic:masterfrom
alpar-t:fix-reindex-fips
Apr 19, 2019
Merged

Add FIPS specific testclusters configuration#41199
alpar-t merged 1 commit intoelastic:masterfrom
alpar-t:fix-reindex-fips

Conversation

@alpar-t
Copy link
Copy Markdown
Contributor

@alpar-t alpar-t commented Apr 15, 2019

ClusterFormationTasks auto configured these properties for clusters.
This PR adds FIPS specific configuration across all test clusters from
the main build script to prevent coupling betwwen testclusters and the
build plugin.

Closes #40904

@alpar-t alpar-t requested a review from jkakavas April 15, 2019 13:49
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Apr 15, 2019

Pinging @elastic/es-core-infra

@alpar-t
Add FIPS specific testclusters configuration
2e1463a 
ClusterFormationTasks auto configured these properties for clusters.
This PR adds FIPS specific configuration across all test clusters from
the main build script to prevent coupling betwwen testclusters and the
build plugin.

Closes elastic#40904
@jkakavas
Copy link
Copy Markdown
Contributor

jkakavas commented Apr 15, 2019

Is this : https://github.com/elastic/elasticsearch/blob/master/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy#L303 still needed now ?

@alpar-t
Copy link
Copy Markdown
Contributor Author

alpar-t commented Apr 15, 2019

@jkakavas yes, other projects still use ClusterFormationTasks. That will go away when the whole class goes.

@jkakavas
Copy link
Copy Markdown
Contributor

jkakavas commented Apr 15, 2019

@jkakavas yes, other projects still use ClusterFormationTasks. That will go away when the whole class goes.

So, any further fips specific system properties should be added both in the build.gradle of the elasticsearch project and in the ClusterFormationTasks until that's removed, correct ?

jkakavas
jkakavas approved these changes Apr 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

LGTM

@jkakavas jkakavas self-requested a review April 15, 2019 19:21
@jkakavas
Copy link
Copy Markdown
Contributor

jkakavas commented Apr 15, 2019

On second thought: I'm adding a few things related to FIPS config in #41024 and my initial approach is to add that config in BuildPlugin too , i.e.

                // Set the system keystore/truststore password if we're running tests in a FIPS-140 JVM
                if (project.ext.inFipsJvm) {
                    ExportElasticsearchBuildResourcesTask buildResources = project.tasks.getByName('buildResources')
                    dependsOn buildResources
                    project.dependencies.add('testCompile', "org.bouncycastle:bc-fips:1.0.1:jar")
                    systemProperty 'javax.net.ssl.keyStore', buildResources.copy("cacerts.bcfks")
                    systemProperty 'javax.net.ssl.keyStoreType', 'BCFKS'
                    systemProperty 'javax.net.ssl.keyStorePassword', 'password'
                    systemProperty 'javax.net.ssl.trustStore', buildResources.copy("cacerts.bcfks")
                    systemProperty 'javax.net.ssl.trustStoreType', 'BCFKS'
                    systemProperty 'javax.net.ssl.trustStorePassword', 'password'
                    String policyFile = "java_${project.ext.runtimeJavaVersion.getMajorVersion()}_fips.policy"
                    String securityPropertiesFile = "java_${project.ext.runtimeJavaVersion.getMajorVersion()}_fips.security"
                    systemProperty 'java.security.policy', buildResources.copy(policyFile)
                    systemProperty 'java.security.properties', buildResources.copy(securityPropertiesFile)
                }

AFAIU, we can't move the ones depending on buildResources to the main build script, so would it be worth it having fips related config in multiple places ?

@alpar-t
Copy link
Copy Markdown
Contributor Author

alpar-t commented Apr 16, 2019

I think it's very much worth having everything in a single place, but it doesn't have to be the build plugin. Some projects ( tests ) will need fips without having that plugin applied. The build resources is still universally available since other tasks create it as well.
You could add configuration to the task with tasks.withType(ExportElasticsearchBuildResourcesTask) {} so it's only applicable when the task is created.

We could also have everything fips specific live in a new plugin.

@alpar-t alpar-t merged commit 92483d4 into elastic:master Apr 19, 2019
@alpar-t alpar-t deleted the fix-reindex-fips branch April 19, 2019 06:56
alpar-t added a commit that referenced this pull request Apr 19, 2019
@alpar-t
Add FIPS specific testclusters configuration (#41199)
e1e2568 
ClusterFormationTasks auto configured these properties for clusters.
This PR adds FIPS specific configuration across all test clusters from
the main build script to prevent coupling betwwen testclusters and the
build plugin.

Closes #40904
@alpar-t alpar-t mentioned this pull request May 2, 2019
Closed
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
@alpar-t
Add FIPS specific testclusters configuration (elastic#41199)
7d27feb 
ClusterFormationTasks auto configured these properties for clusters.
This PR adds FIPS specific configuration across all test clusters from
the main build script to prevent coupling betwwen testclusters and the
build plugin.

Closes elastic#40904
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkakavas jkakavas Awaiting requested review from jkakavas

Assignees

No one assigned

Labels

:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v7.2.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[CI] :modules:reindex task can't run on FIPS JVMs

5 participants

@alpar-t @elasticmachine @jkakavas @jakelandis @mark-vieira