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

Plugins with only bin and config do not install correctly #7154

Closed

Conversation

dadoonet
Copy link
Member

@dadoonet dadoonet commented Aug 4, 2014

When installing a bin only plugin, it is identified as a site plugin.

A current workaround would be to create in the zip file another empty dir. So if you have:

  • bin/myfile.sh
  • empty/empty.txt

the bin content will be extracted as expected.

Closes #7152.

@dadoonet dadoonet added the review label Aug 4, 2014
@dadoonet dadoonet self-assigned this Aug 4, 2014

if (topLevelDirNames.size() == 1) {
String dirname = topLevelDirNames.iterator().next();
if (!"_site".equals(dirname) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe define a set like the existing "BLACKLIST" set in this class but which is called "VALID_TOP_LEVEL_PLUGIN_DIRS" or similar.
Then you can just check if the single root directory is contained in this set?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! That's definitely better indeed. :)

@dadoonet dadoonet removed the review label Aug 6, 2014
@dadoonet
Copy link
Member Author

dadoonet commented Aug 6, 2014

@markharwood Updated based on your comment and added a fix

I ran tests locally on my machine. Though I have no idea on how to cleanly test it using our unit/integration tests as it basically tries to modify $ES_HOME/bin or $ES_HOME/config dirs.

@markharwood
Copy link
Contributor

The changes you've made look good, but like you say it could do a test rig that can prove it is doing the right things. Maybe that can be covered in a different issue relating to the test infrastructure?

@dadoonet
Copy link
Member Author

@markharwood Actually we can test it! :) \o/ See latest commit.

When installing a bin only plugin, it is identified as a site plugin.

A current workaround would be to create in the zip file another empty dir. So if you have:

* `bin/myfile.sh`
* `empty/empty.txt`

the `bin` content will be extracted as expected.

Closes elastic#7152.
@dadoonet dadoonet removed the review label Aug 12, 2014
@dadoonet
Copy link
Member Author

Pushed in master, 1.x and 1.3

@dadoonet dadoonet closed this Aug 12, 2014
@dadoonet dadoonet deleted the issue/7152-plugin-configbin-dirs branch August 12, 2014 12:43
@clintongormley clintongormley changed the title plugins: bin and config only plugins do not install correctly Plugins: bin and config only plugins do not install correctly Sep 11, 2014
@clintongormley clintongormley added the :Core/Infra/Plugins Plugin API and infrastructure label Jun 7, 2015
@clintongormley clintongormley changed the title Plugins: bin and config only plugins do not install correctly Plugins with only bin and config do not install correctly Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plugins: bin and config only plugins do not install correctly
3 participants