pingidentity / ldapsdk

UnboundID LDAP SDK for Java
Other
327 stars 79 forks source link

Add GraalVM native-image configuration for LDIF file #135

Closed marcusdacoregio closed 9 months ago

marcusdacoregio commented 1 year ago

Current behavior: Spring Boot has support for an embedded LDAP server which allow us to specify a schema file in order to add initial data. When building a native-image, the GraalVM needs hints in order to know which files it should consider when compiling natively.

This library uses the standard-schema.ldif but there is no hint for it.

We are now adding the hint by ourselves, but it would be nice if the hint could be added to the library itself.

Expected behavior: The library contains the resource-config.json file with the standard-schema.ldif resource as a hint.

{
  "resources": {
    "includes": [
      {
        "pattern": "\\Qcom\/unboundid\/ldap\/sdk\/schema\/standard-schema.ldif\\E"
      }
    ]
  }
}

Sample PR in another project: https://github.com/brettwooldridge/HikariCP/pull/1959

dirmgr commented 1 year ago

Sorry. I somehow accidentally closed this issue with some mistaken key combination.

Nevertheless, I'm confused by this. Are you saying that GraalVM's default assumption is that all non-class files in a jar file are completely superfluous and included for no good reason? That makes no sense whatsoever. Why would anyone include resources in a jar file if they weren't intended to be used in conjunction with the classes in that jar file? And are you also saying that there's no way of overriding this default behavior with a build flag to dissuade it of this idiotic assumption?

As I understand it from the GraalVM documentation, this resource-config.json file isn't something that's supposed to be included in the jar file itself but rather provided as a companion to be targeted with a command-line option. Is that correct? If so, is there a simple resource-config.json file that tells it to actually take everything in the jar file because it actually is all important and really is there for a reason?

Sorry for the argumentative tone, but I honestly can't understand why they would make that assumption.

marcusdacoregio commented 1 year ago

Hi!

Quote from GraalVM docs, emphasis by me:

By default, the native-image tool will not integrate any of the resources that are on the classpath into the native executable. To make calls such as Class.getResource() or Class.getResourceAsStream() (or their corresponding ClassLoader methods) return specific resources (instead of null), you must specify the resources that should be accessible at runtime. This can be achieved using a configuration file with the following content:

In order to build the native image, it processes all classes of an application and their dependencies, including those from the JDK. It statically analyzes these data to determine which classes and methods are reachable during the application execution. Then it ahead-of-time compiles that reachable code and data to a native executable for a specific operating system and architecture. During this static analysis, the native-image cannot detect reflective invocations or resources loaded by name, for example.

You are not compelled to apply this change tho. There are 3 possibilities for libraries:

  1. Ignore GraalVM and let users deal with a workaround
  2. Don't include GraalVM metadata and let users contribute to the shared GraalVM reachability metadata repository. Once the metadata files are in there, all users can benefit from them
  3. Include the metadata files in their own source tree

It's up to you to decide how you want to handle this. GraalVM proponents would prefer option 3, but option 2 would work too. I prefer the hints closer to the code which does the proxying / reflection etc. But don't feel bad closing the issue if you don't want to have this metadata in your code.

dirmgr commented 1 year ago

I've done some looking into this, and I've not yet been able to reproduce the problem in which the in-memory directory server doesn't have access to the default standard schema when bundled into a native image.

To test this, I created a very simple class that creates an in-memory directory server instance that uses the default standard schema, starts the server, and retrieves the schema from it. The code for that is:

import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
import com.unboundid.ldap.sdk.LDAPConnection;
import com.unboundid.ldap.sdk.schema.Schema;

/**
 * This class provides a simple test that creates an instance of the in-memory
 * directory server and retrieves its schema.
 */
public final class InMemoryDirectoryServerTest
{
  /**
   * Runs this tool with the provided set of arguments.
   *
   * @param  args  The command-line arguments provided to this program.  Any
   *               arguments provided will be ignored.
   *
   * @throws  Exception  If an unexpected problem occurs.
   */
  public static void main(final String... args)
         throws Exception
  {
    final InMemoryDirectoryServerConfig cfg =
         new InMemoryDirectoryServerConfig("dc=example,dc=com");
    cfg.setSchema(Schema.getDefaultStandardSchema());

    try (InMemoryDirectoryServer ds = new InMemoryDirectoryServer(cfg))
    {
      ds.startListening();

      try (LDAPConnection conn = ds.getConnection())
      {
        final Schema schema = conn.getSchema();
        System.out.println(schema.getSchemaEntry().toLDIFString());
      }
    }
  }
}

I compiled that class with the command:

$ javac -classpath unboundid-ldapsdk.jar InMemoryDirectoryServerTest.java

And then I verified that it works when running it using regular Java:

$ java -classpath .:unboundid-ldapsdk.jar InMemoryDirectoryServerTest

Next, I tried turning it into a native image using the command (using graalvm-ce-java17-linux-amd64-22.2.0.tar.gz, which is the latest build I found on their site):

${GRAALVM_HOME}/bin/native-image -classpath .:unboundid-ldapsdk.jar \
     InMemoryDirectoryServerTest in-memory-directory-server-test

That succeeded, and I ran the resulting command and verify that I got the same output when running the native image that I got when running it with the regular JVM:

./in-memory-directory-server-test

Could you provide a simple standalone test case and instructions for demonstrating the problem? I'd prefer to not have to deal with Spring or any other framework if it can be reproduced in a more simple manner.

marcusdacoregio commented 1 year ago

You are right with your commands, there is just one thing that you need to do. You probably have a WARNING in your logs after generating the native-image:

Warning: Image 'in-memory-directory-server-test' is a fallback image that requires a JDK for execution (use --no-fallback to suppress fallback image generation and to print more detailed information why a fallback image was necessary).

So if you add the --no-fallback option to your command, it won't create an native-image that requires JDK to run.

${GRAALVM_HOME}/bin/native-image --no-fallback -classpath .:unboundid-ldapsdk.jar \
     InMemoryDirectoryServerTest in-memory-directory-server-test

Once you run the generated binary, you'll get the error.

Exception in thread "main" LDAPException(resultCode=82 (local error), errorMessage='An error occurred while attempting to load or parse a default set of standard schema elements:  NullPointerException(<init>(Reader.java:168) / <init>(InputStreamReader.java:112) / <init>(LDIFReader.java:604) / <init>(LDIFReader.java:581) / getDefaultStandardSchema(Schema.java:1346) / <init>(InMemoryDirectoryServerConfig.java:253) / main(InMemoryDirectoryServerTest.java:20)), ldapSDKVersion=6.0.5, revision=b50104702e7cc07334a9c64953a91f3e6dbdb27c')
        at com.unboundid.ldap.sdk.schema.Schema.getDefaultStandardSchema(Schema.java:1357)
        at com.unboundid.ldap.listener.InMemoryDirectoryServerConfig.<init>(InMemoryDirectoryServerConfig.java:253)
        at InMemoryDirectoryServerTest.main(InMemoryDirectoryServerTest.java:20)
Caused by: java.lang.NullPointerException
        at java.io.Reader.<init>(Reader.java:168)
        at java.io.InputStreamReader.<init>(InputStreamReader.java:112)
        at com.unboundid.ldif.LDIFReader.<init>(LDIFReader.java:604)
        at com.unboundid.ldif.LDIFReader.<init>(LDIFReader.java:581)
        at com.unboundid.ldap.sdk.schema.Schema.getDefaultStandardSchema(Schema.java:1346)
        ... 2 more
dirmgr commented 1 year ago

I've just committed a change so that the LDAP SDK zip file now includes a resource/graalvm-native-image-resource-config.json file that is generated during the build process. The contents of that file are currently:

{ "resources":{ "includes":[ { "pattern":"\\Qcom/unboundid/ldap/sdk/schema/standard-schema.ldif\\E" },
                             { "pattern":"\\Qcom/unboundid/util/oid-registry.json\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-args.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-asn1.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-cert.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-controls.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-experimental.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-extop.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-interceptor.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-json-log.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-json.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-jsonfilter.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-ldap.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-ldif.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-listener.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-log-syntax.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-log.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-matchingrules.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-monitor.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-persist.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-protocol.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-schema.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-ssl.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-task.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-text-log.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-tools.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-transformations.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-unboundid-controls.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-unboundid-extop.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-unboundidds.properties\\E" },
                             { "pattern":"\\Qunboundid-ldapsdk-util.properties\\E" } ] } }

I've tested with providing this JSON file to the native-image tool and it seems to work properly.

I did consider including it in the jar file manifest, which would eliminate the need to explicitly provide the file to the native-image tool, but I decided against that in case you want to customize what gets included.

marcusdacoregio commented 1 year ago

Great @dirmgr, it is so nice that we can have this config file in the library itself.

I did consider including it in the jar file manifest, which would eliminate the need to explicitly provide the file to the native-image tool, but I decided against that in case you want to customize what gets included.

I don't think anyone would like to customize what's included in the native-image if the library already specified what should be included. It would be great if the config file could be added to the jar file so it can be picked up automatically by the native-image tool, if anyone wants to add something extra they can still use their own hints.

There are some high-profile libraries like Netty and Tomcat which include GraalVM config in their JAR too. https://github.com/netty/netty/blob/4.1/transport/src/main/resources/META-INF/native-image/io.netty/transport/reflection-config.json https://github.com/apache/tomcat/blob/main/res/graal/tomcat-embed-websocket/native-image/tomcat-resource.json

dirmgr commented 1 year ago

I went ahead and updated the build process so that the resource-config.json file is placed in the jar rather than packaged as a separate file.

marcusdacoregio commented 1 year ago

Great @dirmgr, thank you for adding this to the project, I can confirm that it is working. Do you have any schedule for the next release? I do not see any milestones.

dirmgr commented 1 year ago

Unless there's a critical issue that needs to be addressed, releases of the LDAP SDK are generally tied to the development of the Ping Identity Directory Server. I don't have a specific date for the next release of the LDAP SDK, but I expect it to be pretty soon (likely within the next week or two, and possibly even within the next few days).

marcusdacoregio commented 10 months ago

Hi @dirmgr, thank you very much for adding this. I assume this issue can be closed now?

dirmgr commented 9 months ago

If you believe this has been provided to satisfaction, then I'll go ahead and close it.