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

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

Merged
merged 5 commits into from
Aug 17, 2018

Conversation

jkakavas
Copy link
Member

Elasticsearch versions earlier than 6.4.0 cannot properly run in a
FIPS 140 JVM. This commit ensures that we use a non-FIPS JVM for
nodes that we spin up in BWC tests even when we're testing FIPS.

Resolves #32737

It also reverts e497173 and e64bb48 and as such resolves #32868

Elasticsearch versions earlier than 6.4.0 cannot properly run in a
FIPS 140 JVM. This commit ensures that we use a non-FIPS JVM for
nodes that we spin up in BWC tests even when we're testing FIPS.
@jkakavas jkakavas added >test Issues or PRs that are addressing/adding tests :Delivery/Build Build or test infrastructure v7.0.0 v6.4.0 v6.5.0 labels Aug 16, 2018
@jkakavas jkakavas requested review from rjernst and alpar-t August 16, 2018 10:05
@@ -177,6 +177,9 @@ 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 !

Copy link
Member

@rjernst rjernst 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
Copy link
Member Author

jenkins test this please

@jkakavas
Copy link
Member Author

Jenkins test this please

@jkakavas
Copy link
Member Author

I'd like to merge this to get the fips CI green before the weekend. It seems that packaging test runs are still not getting started but the changes in here don't have any impact on packaging. Thoughts @rjernst ?

@rjernst
Copy link
Member

rjernst commented Aug 17, 2018

I think this is ok to merge.

@jkakavas jkakavas merged commit e3aa68b into elastic:master Aug 17, 2018
jkakavas added a commit that referenced this pull request Aug 17, 2018
Elasticsearch versions earlier than 6.4.0 cannot properly run in a
FIPS 140 JVM. This commit ensures that we use a non-FIPS JVM for
nodes that we spin up in BWC tests even when we're testing FIPS.
jkakavas added a commit that referenced this pull request Aug 17, 2018
Elasticsearch versions earlier than 6.4.0 cannot properly run in a
FIPS 140 JVM. This commit ensures that we use a non-FIPS JVM for
nodes that we spin up in BWC tests even when we're testing FIPS.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 18, 2018
* master:
  NETWORKING: Make RemoteClusterConn. Lazy Resolve DNS (elastic#32764)
  [DOCS] Splits the users API documentation into multiple pages (elastic#32825)
  [DOCS] Splits the token APIs into separate pages (elastic#32865)
  [DOCS] Creates redirects for role management APIs page
  Bypassing failing test PainlessDomainSplitIT#testHRDSplit (elastic#32966)
  TEST: Mute testRetentionPolicyChangeDuringRecovery
  [DOCS] Fixes more broken links to role management APIs
  [Docs] Tweaks and fixes to rollup docs
  [DOCS] Fixes links to role management APIs
  [ML][TEST] Fix BasicRenormalizationIT after adding multibucket feature
  [DOCS] Splits the roles API documentation into multiple pages (elastic#32794)
  [TEST]  Run pre 6.4 nodes in non-FIPS JVMs (elastic#32901)
  Make Geo Context Mapping Parsing More Strict (elastic#32821)
jasontedor added a commit that referenced this pull request Aug 18, 2018
* elastic/master: (46 commits)
  NETWORKING: Make RemoteClusterConn. Lazy Resolve DNS (#32764)
  [DOCS] Splits the users API documentation into multiple pages (#32825)
  [DOCS] Splits the token APIs into separate pages (#32865)
  [DOCS] Creates redirects for role management APIs page
  Bypassing failing test PainlessDomainSplitIT#testHRDSplit (#32966)
  TEST: Mute testRetentionPolicyChangeDuringRecovery
  [DOCS] Fixes more broken links to role management APIs
  [Docs] Tweaks and fixes to rollup docs
  [DOCS] Fixes links to role management APIs
  [ML][TEST] Fix BasicRenormalizationIT after adding multibucket feature
  [DOCS] Splits the roles API documentation into multiple pages (#32794)
  [TEST]  Run pre 6.4 nodes in non-FIPS JVMs (#32901)
  Make Geo Context Mapping Parsing More Strict (#32821)
  [ML] fix updating opened jobs scheduled events (#31651) (#32881)
  Scripted metric aggregations: add deprecation warning and system property to control legacy params (#31597)
  Tests: Fix timezone conversion in DateTimeUnitTests
  Enable FIPS140LicenseBootstrapCheck (#32903)
  Fix InternalAutoDateHistogram reproducible failure (#32723)
  Remove assertion in testDocStats on deletedDocs counter (#32914)
  HLRC: Move ML request converters into their own class (#32906)
  ...
jasontedor added a commit that referenced this pull request Aug 18, 2018
* 6.x: (42 commits)
  [DOCS] Splits the users API documentation into multiple pages (#32825)
  [DOCS] Splits the token APIs into separate pages (#32865)
  [DOCS] Creates redirects for role management APIs page
  Bypassing failing test PainlessDomainSplitIT#testHRDSplit (#32966)
  TEST: Mute testRetentionPolicyChangeDuringRecovery
  [DOCS] Fixes more broken links to role management APIs
  [Docs] Tweaks and fixes to rollup docs
  [DOCS] Fixes links to role management APIs
  [ML][TEST] Fix BasicRenormalizationIT after adding multibucket feature
  [DOCS] Splits the roles API documentation into multiple pages (#32794)
  [TEST]  Run pre 6.4 nodes in non-FIPS JVMs (#32901)
  Remove assertion in testDocStats on deletedDocs counter (#32914)
  [ML] fix updating opened jobs scheduled events (#31651) (#32881)
  Enable FIPS140LicenseBootstrapCheck (#32903)
  HLRC: Move ML request converters into their own class (#32906)
  [DOCS] Update getting-started.asciidoc (#29518)
  Fix allowed value for HighlighterBuilder encoder in javadocs (#32780)
  [DOCS] Add "remove a tag" script logic as an example (#32556)
  RFC: Test that example plugins build stand-alone (#32235)
  Security: remove put privilege API (#32879)
  ...
@jkakavas jkakavas deleted the pre6.4-fips-bwc branch September 14, 2018 06:49
@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
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v6.4.0 v6.5.0 v7.0.0-beta1
Projects
None yet
5 participants