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

[WIP] Add support for JSR-223 scripting of effectors #153

Closed
wants to merge 2 commits into from
Closed

[WIP] Add support for JSR-223 scripting of effectors #153

wants to merge 2 commits into from

Conversation

grkvlt
Copy link
Member

@grkvlt grkvlt commented May 23, 2016 •
edited
Loading

This is an attempt to add scripting language support to Brooklyn blueprints, so that effectors can be added in various languages directly. JSR-223 supports JavaScript and I have also added the Python 2.7.0 plugin, but many other languages are available.

See the script-effector-example.yaml file for an example of how these scripting languages can be used to implement effectors.

@grkvlt grkvlt force-pushed the script-effector branch 4 times, most recently from 0eb870f to a77cb7a Compare May 23, 2016 18:50
@grkvlt grkvlt changed the title [WIP] Add a ScriptEffector using JSR-223 embedded scripting Add support for JSR-223 scripting for effector implementations May 23, 2016
@grkvlt grkvlt changed the title Add support for JSR-223 scripting for effector implementations Add support for JSR-223 scripting for effectors May 23, 2016
@grkvlt grkvlt changed the title Add support for JSR-223 scripting for effectors Add support for JSR-223 scripting of effectors May 23, 2016
@grkvlt
Copy link
Member Author

grkvlt commented May 23, 2016 •
edited
Loading

@ahgittin I'd be interested in your thoughts about this idea... Still need to add more examples and tests, as well as writing documentation - where do you suggest this should go?

@grkvlt grkvlt force-pushed the script-effector branch 2 times, most recently from 1b6da78 to 82454ea Compare May 23, 2016 20:36
@grkvlt grkvlt mentioned this pull request May 23, 2016
Closed
johnmccabe
johnmccabe reviewed May 23, 2016
View reviewed changes
core/pom.xml
<dependency>
<groupId>org.python</groupId>
<artifactId>jython</artifactId>
<version>2.7.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible/feasible to install additional python modules for use with jython? Does the cPython ecosystem place nicely?

Copy link
Member Author

@grkvlt grkvlt May 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I believe that as long as the archives with the .py or .pyc files inside are on the CLASSPATH this is possible, and executing import whatever will work, and have seen examples, but I have not tested this.

Copy link
Contributor

Choose a reason for hiding this comment

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

For a while, we bundled jython (via winrm4j, until we re-wrote that as pure Java). The jython libraries pulled in about 20 or 30MB of dependencies. It would certainly be nice if folk can add that to the classpath (or OSGi bundles) themselves, but I don't think we want to ship it with Brooklyn.

Copy link
Member

Choose a reason for hiding this comment

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

Agree that jython is better left as an optional addon - not worth the weight it brings in. Plus we already bundle groovy and javascript is provided by the JRE so there are alternatives to choose from.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add it as a test-only dependency instead, to validate that we can call additional JSR-223 languages if required, but document that we only support JavaScript (as either Rhino in 7.x or Nashorn in 8.x) by default,

@grkvlt grkvlt mentioned this pull request May 23, 2016
Merged
duncangrant
duncangrant reviewed May 24, 2016
View reviewed changes
core/src/test/java/org/apache/brooklyn/core/effector/script/ScriptEffectorTest.java
Maybe<Effector<?>> javaScriptEffector = entity.getEntityType().getEffectorByName("javaScriptEffector");
Assert.assertTrue(javaScriptEffector.isPresentAndNonNull(), "The JavaScript effector does not exist");
Object result = Entities.invokeEffector(entity, entity, javaScriptEffector.get()).getUnchecked();
Assert.assertTrue(result instanceof String, "Returned value is of type String");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "Returned value is not of type string"?

@grkvlt
Copy link
Member Author

grkvlt commented May 24, 2016

@sjcorbett Once #152 is merged I will update this to take advantage of the same features

@neykov
Copy link
Member

neykov commented May 25, 2016

+1 for the idea - very useful in yaml context. I've been wanting to see this for a while.
What I think needs discussion is the security implications this brings. Currently as long as you can add (Java) catalog items you can gain full access to the Brooklyn instance so only privileged users should be allowed to do it. For application deployments "regular" users can only use existing catalog items which won't let them tinker with Brooklyn internals. Having this in the stock distribution will change that.
Take for example the Cloud Foundry Service Bridge. Any CF user is able to spin up an application in Brooklyn.

@grkvlt
Copy link
Member Author

grkvlt commented May 25, 2016 •
edited
Loading

@neykov that's a reasonable point - this exposes managementContext as a script engine attribute binding, which lets you do anything. However it's the same set of capabilities as if you had a catalog entry with a brooklyn.libraries entry pointing to a custom Jar file defining your entity and its effectors, isn't it? And a "regular" user can do that currently, I believe.

@grkvlt
Copy link
Member Author

grkvlt commented May 29, 2016

I talked with @neykov about this, and we agreed that a sandbox would be required, and also that it would be difficult. I have been thinking about how to implement it, and one mechanism might be to inject a brooklyn object that allows only method calls to the equivalent of the $brooklyn: functions in the DSL. I'm still thinking about this, and will post to the Brooklyn dev list when I have something more concrete.

@grkvlt grkvlt force-pushed the script-effector branch from 82454ea to 023ab32 Compare May 29, 2016 21:17
@grkvlt grkvlt force-pushed the script-effector branch from 023ab32 to 60e734b Compare June 1, 2016 23:47
@grkvlt grkvlt changed the title Add support for JSR-223 scripting of effectors [WIP] Add support for JSR-223 scripting of effectors Jun 22, 2016
@neykov
Copy link
Member

neykov commented Oct 20, 2016 •
edited
Loading

Looks like it's possible to secure a script engine so it can run any 3d party scripts, but it's language spefic. There's nothing in JSR-223 that we can rely on.
See what it would take to secure a Rhino engine in http://codeutopia.net/blog/2009/01/02/sandboxing-rhino-in-java/.

@grkvlt
Copy link
Member Author

grkvlt commented Mar 28, 2017

Closing this, as much of the desired functionality can be obtained through use of the meta effectors in #605

@grkvlt grkvlt closed this Mar 28, 2017
@grkvlt grkvlt deleted the script-effector branch March 28, 2017 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@grkvlt @neykov @johnmccabe @aledsage @duncangrant