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.

Sign up for GitHub

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

Jump to bottom

[TEST] Run pre 6.4 nodes in non-FIPS JVMs #32901

Merged
merged 5 commits into from
Aug 17, 2018
Merged

[TEST] Run pre 6.4 nodes in non-FIPS JVMs #32901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bbfd15e
Run pre 6.4 nodes in non-FIPS JVMs
jkakavas Aug 16, 2018
89f73cc
Revert "Tests: Disable rolling upgrade tests with system key on fips …
jkakavas Aug 16, 2018
d5c67cc
Revert "mute test #32737"
jkakavas Aug 16, 2018
8f69f49
Address feedback
jkakavas Aug 16, 2018
2d4cfaa
Merge remote-tracking branch 'origin/master' into pre6.4-fips-bwc
jkakavas Aug 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,12 @@ class NodeInfo {
javaVersion = 8
} else if (nodeVersion.onOrAfter("6.2.0") && nodeVersion.before("6.3.0")) {
javaVersion = 9
} else if (project.inFipsJvm && nodeVersion.onOrAfter("6.3.0") && nodeVersion.before("6.4.0")) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm sure I'm missing something, but wouldn't this only change the version that we try to start on, but not stop the node from attempting to start?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. The intention is to not mute the tests but keep running them even when we're in a FIPS JVM in CI. The way to achieve this is to make sure that older ES version ( not supporting fips ) nodes start with a non fips java version

Copy link
Member

Choose a reason for hiding this comment

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

This is not at all clear from the code. Can you please add a comment explaining it? If I understand correctly, by adding this other elseif condition, non fips testing will fall through and continue using the RUNTIME_JAVA_HOME? But isn't that a fips jvm in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not at all clear from the code. Can you please add a comment explaining it?

Sure thing. I tried to capture it in // Versions before 6.4.0 cannot be run in a FIPS 140 JVM but I agree that's not very clear.

If I understand correctly, by adding this other elseif condition, non fips testing will fall through and continue using the RUNTIME_JAVA_HOME? But isn't that a fips jvm in this case?

Not sure I follow your thought. When RUNTIME_JAVA_HOME is a fips JVM, project.inFipsJvm will also be true.
In summary:

  • When running in a non FIPS JVM

    • Nothing changes from the previous behavior
    • We run < 6.2.0 nodes with Java 8
    • We run > 6.2.0 and < 6.3.0 nodes with Java 9
    • We run > 6.3.0 nodes with RUNTIME_JAVA_HOME ( non FIPS )

  • When running in a FIPS JVM

    • project.inFipsJvm is true
    • We run < 6.2.0 nodes with Java 8 (non fips)
    • We run > 6.2.0 and < 6.3.0 nodes with Java 9 (non FIPS)
    • We run > 6.3.0 and < 6.4.0 nodes with Java 10 (non FIPS)
    • We run > 6.4.0 nodes with RUNTIME_JAVA_HOME ( which is FIPS but > 6.4 nodes can run fine in a FIPS JVM)

Does this make more sense ? Bear with me if I've missed your point entirely.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, that is more clear.

Copy link
Member Author

Choose a reason for hiding this comment

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

@rjernst I updated the comment, let me know if this is clear enough, thanks !

/*
* Elasticsearch versions before 6.4.0 cannot be run in a FIPS-140 JVM. If we're running
* bwc tests in a FIPS-140 JVM, ensure that the pre v6.4.0 nodes use a Java 10 JVM instead.
*/
javaVersion = 10
}

args.addAll("-E", "node.portsfile=true")
Expand Down
8 changes: 0 additions & 8 deletions x-pack/qa/full-cluster-restart/with-system-key/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +0,0 @@
import org.elasticsearch.gradle.test.RestIntegTestTask

// Skip test on FIPS FIXME https://github.com/elastic/elasticsearch/issues/32737
if (project.inFipsJvm) {
tasks.withType(RestIntegTestTask) {
enabled = false
}
}
9 changes: 0 additions & 9 deletions x-pack/qa/rolling-upgrade/with-system-key/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1 @@
import org.elasticsearch.gradle.test.RestIntegTestTask

// Skip test on FIPS FIXME https://github.com/elastic/elasticsearch/issues/32737
if (project.inFipsJvm) {
tasks.withType(RestIntegTestTask) {
enabled = false
}
}

group = "${group}.x-pack.qa.rolling-upgrade.with-system-key"