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

Re-enable integration tests for secure HDFS fixtures #76793

Closed
wants to merge 1 commit into from

Conversation

jbaiera
Copy link
Member

@jbaiera jbaiera commented Aug 20, 2021

When running Elasticsearch on JDK 16 we must add exports for the internal java security packages in a few places in order to make use of the HDFS client in Kerberos secured environments:

  1. Add exports within MiniHDFS fixtures so that it can access security packages for Kerberos authentication server-side.
  2. Add exports within Elasticsearch fixtures when HDFS client is in use so that it can access packages for Kerberos on the client-side.
  3. Add exports in integration test JVMs when directly testing the HDFS client's Kerberos usage in integration test methods.

This does highlight that operating Elasticsearch on JDK 16 against secured HDFS installations requires the Elasticsearch JVM options to add exports for the internal java security packages in order for the HDFS client code in any plugins to function correctly. This should be highlighted in the documentation.

Resolves #68075

@jbaiera jbaiera added >test Issues or PRs that are addressing/adding tests :Core/Features/Features labels Aug 20, 2021
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Aug 20, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

Copy link
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

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

This does highlight that operating Elasticsearch on JDK 16 against secured HDFS installations requires the Elasticsearch JVM options to add exports for the internal java security packages in order for the HDFS client code in any plugins to function correctly. This should be highlighted in the documentation.

Indeed. Perhaps we should add something to the default jvm.options file? This -add-exports business is starting to get a bit messy. There's still no concrete timeline on when --illegal-access will be removed. It still exists in JDK 17 which is an LTS release so I suspect not anytime immediately soon. @rjernst perhaps you have some thoughts here.

@masseyke
Copy link
Member

Does the hadoop 3 client still require this? If we upgraded to the hadoop 3 client could we avoid having to have extra JVM args? Hadoop 3 supposedly has wire compatibility with hadoop 2: https://blog.cloudera.com/upgrading-clusters-workloads-hadoop-2-hadoop-3/

@rjernst
Copy link
Member

rjernst commented Aug 20, 2021

Perhaps we should add something to the default jvm.options file?

Since we've been moving away from the singular jvm.options file, and would rather have additional options in jvm.options.d, I think we should output a log error when repository-hdfs is initialized and the export is not present. We can do this by calling KerberosUtil.getDefaultRealm(), catching IllegalAccessException. If we get an exception, the export does not exist, and we should output the log message indicating the exact line to add to a file in jvm.options.d. Wdyt? (Obviously separate from this PR, which looks fine)

@mark-vieira
Copy link
Contributor

Is there anyway to provide a more seamless user experience, such as dynamically add the required JVM arguments in the presence of ths installed plugin? Can we wire this into the jvm arguments parser somehow?

@jbaiera
Copy link
Member Author

jbaiera commented Aug 23, 2021

Is there anyway to provide a more seamless user experience, such as dynamically add the required JVM arguments in the presence of ths installed plugin? Can we wire this into the jvm arguments parser somehow?

Maybe we could add an extra hook in the plugin installation process so that plugins that need certain JVM options to function can provide them during installation? Then we could potentially do some kind of validation. Not sure how widespread of a problem this is though, and whether it warrants such a change yet. Perhaps we should start a new issue for the discussion?

Does the hadoop 3 client still require this? If we upgraded to the hadoop 3 client could we avoid having to have extra JVM args?

I suspect that we will still require this, but I'm doing some quick testing to find out.

@mark-vieira
Copy link
Contributor

For all practical purposes, we used to be running Elasticsearch with --illegal-access=permit as that was the default until Java 16. The concern with continuing to use this option is that it is deprecated and will be removed... eventually.

Given that our default runtime configuration was to allow all unprotected access in ES code it doesn't seem any worse to simply add this --exports flag by default when running on Java 16+. It's already more secure than when running on older JDKs and avoid the bad user experience of having folks manually add this.

@masseyke
Copy link
Member

I just confirmed locally that with the hadoop 3 client we do not need those -add-exports calls. And it seems to work fine against a hadoop 2 HDFS. And it fixes #76734. That might be the way to go.

@masseyke
Copy link
Member

I put up #76897 as a possible alternative way of doing this by upgrading to hadoop 3. Obviously a little riskier, so probably worth discussing.

@rjernst
Copy link
Member

rjernst commented Sep 1, 2021

@masseyke If I understand the change correctly, the way it avoids the illegal access is that Hadoop 3 has a proper client jar, which no longer needs to open up jdk kerberos classes? The PR, though, still includes the exports cli option for testclusters, is that still needed?

@masseyke
Copy link
Member

masseyke commented Sep 2, 2021

@rjernst Yes, the hadoop 3 client avoids the illegal access (and a NoClassDefFoundError that we still get with this PR) because they've rewritten the client classes to not depend on sun.* classes.
The reason that the PR still has "--ad-exports" is that the PR still intentionally uses a hadoop 2 hdfs server in the test fixture, just to confirm that the hadoop 3 client would work against a hadoop 3 hdfs. We probably ought to expand the PR to have a hadoop 3 test fixture as well.

@jbaiera
Copy link
Member Author

jbaiera commented Sep 29, 2021

Closing this in favor of #76897

@jbaiera jbaiera closed this Sep 29, 2021
@jbaiera jbaiera deleted the fix-hdfs-integtest-jdk-16 branch September 29, 2021 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v8.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] Secure Hdfs Fixture in JDK16
7 participants