-
Notifications
You must be signed in to change notification settings - Fork 176
Painless docs overhaul (explore & analyze) #3580
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
Conversation
benironside
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As requested, I didn't do a thorough review here since others already did. But from doing a spot check, this looks awesome. Great work, tons of improvements!
karenzone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨
shainaraskas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did only a very brief sweep due to limited time. couple of things:
-
the newly introduced tutorials aren't structured as tutorials - the steps are all one big block of prose. they really need to be broken into sequential steps.
-
language in the tutorials is very flowery. could pull it back a little.
-
Lots of booboo typos and formatting errors here and there (commented on the first handful I found). Could use a finer toothed comb. Would also suggest just requesting a review from copilot in github to do an editing pass
|
Thanks for the suggestions @shainaraskas! I've resolved all of your comments. I think the "How to write scripts" area could do with some more style and structure improvements, apart from what you've suggested, but since that content wasn't touched as part of the Kibernum team's review (apart from just splitting the original file into separate pages), I've left that alone for now. I can definitely come back to it another day, though. |
|
@jdconrad Would you mind giving this a final technical review? I've changed some of the phrasing and layout, but the code and examples are untouched from what the Kibernum team provided. Same thing for the Troubleshooting docs section, and then there will be a final PR to come for the Reference docs. Thank you! |
|
I will take a look soon. |
jdconrad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The examples are great, and something that has been missing for a while. I added a few comments around the correctness of the security layers.
|
|
||
| Painless provides three core benefits across all scripting contexts: | ||
|
|
||
| * **Security:** Fine-grained allowlists that prevent access to restricted Java APIs and enforce multiple security layers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The allowlists don't enforce multiple security layers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this to just * **Security:** Fine-grained allowlists that prevent access to restricted Java APIs.
| ::: | ||
|
|
||
| Finally, scripts used in [scripted metrics aggregations](elasticsearch://reference/aggregations/search-aggregations-metrics-scripted-metric-aggregation.md) can be restricted to a defined list of scripts, or forbidden altogether. This can prevent users from running particularly slow or resource intensive aggregation queries. | ||
| The fine-grained allowlist operates as the **first security layer**. Anything that is not part of the allowlist will result in a compilation error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This depends. It could also be a runtime error if the type isn't known at compile-time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just say error then, without specifying the type. I've changed it to: The fine-grained allowlist operates as the first security layer. Anything that is not part of the allowlist will result in an error.
|
|
||
| {{es}} uses [seccomp](https://en.wikipedia.org/wiki/Seccomp) in Linux, [Seatbelt](https://www.chromium.org/developers/design-documents/sandbox/osx-sandboxing-design) in macOS, and [ActiveProcessLimit](https://msdn.microsoft.com/en-us/library/windows/desktop/ms684147) on Windows as additional security layers to prevent {{es}} from forking or running other processes. | ||
| :::{image} /explore-analyze/images/elasticsearch-painless-security-architecture-overview.png | ||
| :alt: Flow chart showing four layers of security between script request and safe script execution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure a flow chart is a good way to present this. Each piece of security is sort of separate from the others in many cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I've removed the flow chart graphic, and rephrased the text a bit to de-emphasize the "layers", so as not to suggest that the security features are applied sequentially. I've also removed the bold format from part of the text.
| Finally, scripts used in [scripted metrics aggregations](elasticsearch://reference/aggregations/search-aggregations-metrics-scripted-metric-aggregation.md) can be restricted to a defined list of scripts, or forbidden altogether. This can prevent users from running particularly slow or resource intensive aggregation queries. | ||
| The fine-grained allowlist operates as the **first security layer**. Anything that is not part of the allowlist will result in a compilation error. | ||
|
|
||
| The **second layer of security** is [Java Security Manager (JSM)](https://www.oracle.com/java/technologies/javase/seccodeguide.html). As part of its startup sequence, {{es}} enables JSM to limit the actions that portions of the code can take. Painless uses this additional layer of defense to prevent scripts from doing things such as writing files and listening to sockets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be updated to entitlements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed:
from: enables JSM to limit the actions that portions of the code can take.
to: enables JSM to limit the entitlements that portions of the code have.
Please let me know if that's not correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "script_fields": { | ||
| "my_doubled_field": { | ||
| "script": { | ||
| "source": "field('my_field').get(null) * params['multiplier']", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field access API isn't a direct replacement for doc. The behavior includes a default value (the null) and may access both doc values and _source as underlying structures depending on the field type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed the last paragraph to:
You can use this abbreviated syntax anywhere that {{es}} supports scripts, such as when you’re creating [runtime fields](../../manage-data/data-store/mapping/map-runtime-field.md). Be mindful, however, that the fieldaccess API is not a direct replacement fordoc. This shortened version of the original script includes a default value (the null) so the script may access both docvalues and_source as underlying structures, depending on the field type.
Again, let me know if I've misunderstood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is close.
Depending on the field type the script may access either doc values or _source. Some fields will use _source as a fallback if doc values aren't available for a specific field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect. Thanks. I've changed this as suggested.
This shortened version of the original script includes a default value (the
null), so depending on the field type the script may access eitherdocvalues or_source. Some fields will use_sourceas a fallback ifdocvalues aren't available for a specific field.
|
Thanks a lot for the review @jdconrad! I've applied fixes but please let me know if anything's not right. |
|
@kilfoyle Thanks for the revisions. I added a few more comments. Please let me know if they make sense or need any further clarification. I think not referencing the Java Security Manager is really important. |
|
Thanks again @jdconrad! I've fixed those last couple of remaining things. Please see comments above. |
jdconrad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for all the work done here.

This refreshes the Scripting section of the Explore and Analyze docs, based on the Painless doc project.
The content has had both technical and writer reviews, so this is essentially a port from the gdoc project folder into the V3 docs system.
All changes are in this Introduction to Painless section.
Rel: https://github.com/elastic/docs-content-internal/issues/299