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

[Task] Port Rules engine from Nashorn to GraalVM JS #990

Closed
andrewazores opened this issue Jun 9, 2022 · 2 comments
Closed

[Task] Port Rules engine from Nashorn to GraalVM JS #990

andrewazores opened this issue Jun 9, 2022 · 2 comments
Labels
chore Refactor, rename, cleanup, etc. dependencies Pull requests that update a dependency file help wanted Extra attention is needed question Further information is requested

Comments

@andrewazores
Copy link
Member

andrewazores commented Jun 9, 2022

Related to #274 - when ported to Quarkus, Nashorn should definitely be dropped in favour of Graal/Mandrel's implementation. Can the downstream product depend on the GraalVM impl below?

Nashorn was already deprecated in JDK11 and has been fully removed from the main JDK in more recent releases. There is still a separate upstream https://github.com/openjdk/nashorn project that we can pull in as a replacement dependency, but it would probably be even better to switch to GraalVM's JS implementation.

https://www.graalvm.org/22.1/reference-manual/js/NashornMigrationGuide/

https://www.graalvm.org/22.1/reference-manual/js/RunOnJDK/

@andrewazores andrewazores added help wanted Extra attention is needed chore Refactor, rename, cleanup, etc. labels Jun 9, 2022
@andrewazores andrewazores moved this to Todo in 2.2.0 Release Jun 9, 2022
@andrewazores andrewazores added question Further information is requested dependencies Pull requests that update a dependency file labels Jun 15, 2022
@andrewazores andrewazores moved this from Todo to Stretch Goals in 2.2.0 Release Sep 16, 2022
@andrewazores andrewazores moved this to Stretch Goals in 2.3.0 release Nov 3, 2022
@andrewazores andrewazores moved this to Stretch Goals in 2.3.0 release Feb 7, 2023
@andrewazores andrewazores moved this from Stretch Goals to Pushed to 2.4.0 in 2.3.0 release Apr 6, 2023
@andrewazores
Copy link
Member Author

andrewazores commented Jun 29, 2023

Here is an interesting alternative:

https://github.com/google/cel-spec
https://github.com/google/cel-java

From the outset, using a parser-restricted JavaScript subset for the matchExpressions was always a compromise between ease of use for the user (syntactical similarity to Java, which Cryostat users are probably familiar with), minimal implementation effort and maintenance overhead (much less work and code than implementing a small language ourselves), and security/performance (clients should not be able to submit arbitrary expressions that are Turing-complete and cause arbitrary execution on the server).

The Common Expression Language meets these goals by having a very similar and familiar syntax, having a small and non-Turing-complete language, and having a Java implementation that we can mostly plug-and-play on the Cryostat server.

This is unlikely to be 100% syntax compatible with existing JavaScript-based matchExpressions, so changing the implementation in this way should be held for a major version.

@andrewazores
Copy link
Member Author

Closing. 3.0 now uses Project Nessie cel-java, not a JavaScript engine.

@github-project-automation github-project-automation bot moved this from Todo to Done in 3.0.0 release Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. dependencies Pull requests that update a dependency file help wanted Extra attention is needed question Further information is requested
Projects
No open projects
Status: Stretch Goals
Status: Pushed to 2.4.0
Status: Done
Development

No branches or pull requests

1 participant