Skip to content

Drop stored scripts with the old style-id #48078

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

Merged
merged 6 commits into from
Oct 16, 2019
Merged

Conversation

jdconrad
Copy link
Contributor

This PR fixes (#47593). Stored scripts with the old-style id of lang#id are saved through the upgrade process but are no longer accessible in recent versions. This fix will drop those scripts altogether since there is no way for a user to access them.

@jdconrad jdconrad added >bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.5.0 v7.4.1 labels Oct 15, 2019
@jdconrad jdconrad requested a review from martijnvg October 15, 2019 17:26
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Scripting)

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-sample

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/1

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

BytesReference.bytes(builder).streamInput());
ScriptMetaData smd = ScriptMetaData.fromXContent(parser);
assertNull(smd.getStoredScript("painless#test"));
assertNull(smd.getStoredScript("lang#test"));
Copy link
Member

Choose a reason for hiding this comment

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

Maybe check the size of the Map returned from getStoredScripts()?

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 call. Added.

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

1 similar comment
@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

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.

LGTM

@jdconrad
Copy link
Contributor Author

@martijnvg @rjernst Thanks for the reviews! Will commit as soon as CI passes.

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-sample

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

@jdconrad jdconrad merged commit 18a694f into elastic:master Oct 16, 2019
jdconrad added a commit that referenced this pull request Oct 16, 2019
This PR fixes (#47593). Stored scripts with the old-style id of lang#id are 
saved through the upgrade process but are no longer accessible in recent 
versions. This fix will drop those scripts altogether since there is no way for 
a user to access them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.4.1 v7.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants