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

[Scripting] Make Max Script Length Setting Dynamic #35184

Merged
merged 7 commits into from Nov 2, 2018

Conversation

jdconrad
Copy link
Contributor

@jdconrad jdconrad commented Nov 1, 2018

This changes the current script.max_size_in_bytes to be dynamic so it can be set through the cluster settings API. This setting is also applied to inline scripts in the compile method of ScriptService to prevent excessively long inline scripts from being compiled. The script length limit is removed from Painless as this is no longer necessary with the protection in compile.

Relates #23209

@jdconrad jdconrad added >enhancement :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.0.0 v6.6.0 labels Nov 1, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jdconrad jdconrad mentioned this pull request Nov 1, 2018
23 tasks
@jdconrad
Copy link
Contributor Author

jdconrad commented Nov 1, 2018

@elasticmachine test this please

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

Left one comment. LGTM otherwise.

return Collections.emptyMap();
}

ScriptMetaData scriptMetadata = clusterState.metaData().custom(ScriptMetaData.TYPE);
Copy link
Member

Choose a reason for hiding this comment

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

I think this can also return null if there are no stored scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Thank you.

@jdconrad
Copy link
Contributor Author

jdconrad commented Nov 2, 2018

@martijnvg Thanks for the review! Will commit once CI passes.

@jdconrad
Copy link
Contributor Author

jdconrad commented Nov 2, 2018

@elasticmachine test this please

@jdconrad
Copy link
Contributor Author

jdconrad commented Nov 2, 2018

@elasticmachine test this please

@jdconrad jdconrad merged commit 44f0871 into elastic:master Nov 2, 2018
jdconrad added a commit that referenced this pull request Nov 5, 2018
This changes the current script.max_size_in_bytes to be dynamic so it can be 
set through the cluster settings API. This setting is also applied to inline scripts 
in the compile method of ScriptService to prevent excessively long inline 
scripts from being compiled. The script length limit is removed from Painless as 
this is no longer necessary with the protection in compile.
ypid-geberit added a commit to ypid-geberit/examples that referenced this pull request Jan 17, 2019
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
ypid-geberit added a commit to ypid-geberit/examples that referenced this pull request Jun 25, 2020
DanRoscigno pushed a commit to elastic/examples that referenced this pull request Jul 23, 2021
…better error handling, --cacert, multiple time fields (#239)

* run_test.py: Improve/cleanup and add YAML support for input files

* Cleanup shell scripts according to ShellCheck recommendations

* run_test.py: Cleanup Indices after test to not pollute tests env

* run_test.py: More useful error message if logging action did not run

* run_test.py: Refactor

* run_test.py: Use load_file() for ES scripts as well to support YAML

* run_test.py: Implement --no-execute-watch needed for deployment

Needed for: elastic/elasticsearch#30112 (comment)

> I already have a workaround in place for this which consists of automatically deploying as many watches as I need to send different mails. Those watches are derived from my watch definition. For this, I extended [run_test.py](https://github.com/elastic/examples/blob/master/Alerting/Sample%20Watches/run_test.py) to inject Python code after the watch definition is read. Not ideal, but it is maintainable.

* run_test.py: Support to inject Python code, useful for deployment

Needed for: elastic/elasticsearch#30112 (comment)

> I already have a workaround in place for this which consists of automatically deploying as many watches as I need to send different mails. Those watches are derived from my watch definition. For this, I extended [run_test.py](https://github.com/elastic/examples/blob/master/Alerting/Sample%20Watches/run_test.py) to inject Python code after the watch definition is read. Not ideal, but it is maintainable.

* run_test.py: Implement --no-test-index needed for deployment

Needed for: elastic/elasticsearch#30112 (comment)

> I already have a workaround in place for this which consists of automatically deploying as many watches as I need to send different mails. Those watches are derived from my watch definition. For this, I extended [run_test.py](https://github.com/elastic/examples/blob/master/Alerting/Sample%20Watches/run_test.py) to inject Python code after the watch definition is read. Not ideal, but it is maintainable.

* run_test.py: Add --metadata-git-commit switch to augment watch metadata

* run_test.py: Add --cacert parameter

* run_test.py: More useful error message if logging action did not run

* run_test.py: Use `git rev-parse --short HEAD` for --metadata-git-commit

* run_test.py: More useful error message if transform failed

* run_test.py: Implement --minify-scripts

Workaround for: elastic/elasticsearch#35184

* "Scripts may be no longer than 16384 characters." is in ES<v6.6 not >6.6

* run_test.py: Improve compatibility with ES 7.0.x and index templates

* run_test.py: Better error message if expected_response is not defined

* run_test.py: Show watch exception on execution failure

* run_test.py: In case a transform fails the transform input is relevant

* run_test.py: Support multiple time fields

Useful when you have two time fields that in reality should be very
close so in testing it is enough to set them to the same value.

* [run_test.py] ES 7 support. Update to Py3 and drop elasticsearch_xpack.

* [run_test.py] Add --verbose parameter to debug ES responses

* [run_test.py] Comply with Python Enhancement Proposals

* [run_test.py] Comply with reuse.software

* [run_test.py] Avoid `not` in condition to make it easier to understand

* [run_test.py] Use str.format instead of "%s" % for consistency

* [run_test.py] Fix ./run_all_tests.sh test run. All passing again.

* [run_test.py] Support nested fields in time_fields test parameter

Example:

```yaml
time_fields:
  - '@timestamp'
  - 'event.created'
```

* [run_test.py] Use dict.get shortcut
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement v6.6.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants