v14 plugin install bombs if old version is packaged improperly

[gdalam]

Please go through all the tasks below

• Check this box only after you have successfully completed both the above tasks

Please provide a brief description of the problem. Please do not forget to attach the relevant screenshots from your side.

In version 14 of Rock, specifically commit 79ea86e, plugin installation was refactored.

Looking at the code, it seems that the process of installing actually goes through and installs each historical version of the plugin, beginning with the first published version. This was also true in version 13 of Rock, and the packaging documentation states this.

In version 13, however, if a plugin was packaged improperly, it would merely get skipped.

As seen in the v13 code above, checking that the filename starts with content is done as part of a where clause on the entries within the zip. This means that if a plugin was packaged improperly - eg a root directory was zipped instead of just the content, install, and uninstall directories - this portion of the code was merely skipped. It would then continue on to the next version of the plugin.

In version 14, however, this was changed.

Now, if an improperly packaged zip is detected, rather than skipping it, it returns false. This then bubbles up the call stack and is set as the variable wasActionTaken.

This then bubbles up to the wasStepInstalled variable and breaks, so it never attempts to install the later versions of the plugin.

The end result of this is that if you have an old version of a plugin that was improperly packaged, no one will be able to do a fresh install of your plugin or update if they have an even older version. This worked in version 13 - in the past I have accidentally submitted improperly versions of our plugin that was accepted into the Rock Shop - always by having an extra root directory that contained the content, install, and uninstall folders. While the published plugin would "install" from the shop for testing, we'd quickly realize that it hadn't actually updated, so we would merely submit a new version of the package to the shop which would then fix it.

As it stands now in v14, there's no path forward. Submitting a new version of the plugin doesn't do anything, because the install will never get there. And I'm unable to delete the old version of the plugin because the web request just 500's.

I understand that the versions are supposed to install in order, but I can't think of a case where if you'd like to install version 2 of a plugin, you need version 1 first. Block files and .dlls will be overwritten regardless, and data migrations are already safely handled.

Expected Behavior

Prior versions of plugins that were improperly packaged should not stop the rest of the versions from being installed.

We should be able to delete published versions of our plugins as the UI indicates.

Actual Behavior

As of v14, an improperly packaged plugin effectively bricks the plugin as it will never try to install a newer version.

Steps to Reproduce

Publish an improperly package to the Rock Shop, then later publish a fixed version. Update Rock to v14 and try to install the plugin.

Rock Version

14+

Client Culture Setting

en-US

[mikejed]

One thought here- some plugins have run.sql files included in them- this is a case where the model of installing version 1 before version 2 couldn’t be changed since version 2 will have been designed to assume version 1’s script already ran. In case that helps the discussion and considerations :)

[gdalam]

One thought here- some plugins have run.sql files included in them- this is a case where the model of installing version 1 before version 2 couldn’t be changed since version 2 will have been designed to assume version 1’s script already ran. In case that helps the discussion and considerations :)

Is a run.sql script something that's just run as part of install? Because as far as I know you're supposed to make SQL updates using the Data Migration process, which will run safely in order as long as you do it right.

[mikejed]

It's part of what can be included in the plugin package, as documented at https://community.rockrms.com/developer/book/26/26/content#anatomyofapackage . Generally yes I'd say that "it's just run as part of install", along with the list of files to be deleted. I don't have any answers to your OP, just trying to make sure that the full reality of plugins are considered in whatever approach is taken :)

[tcavaletto]

I see that users will get an error specifying the exact version that breaks. Are there any negative repercussions to an organization deleting a bad package version from their Rock Shop catalog if they get a report about that particular version breaking?

[gdalam]

I see that users will get an error specifying the exact version that breaks. Are there any negative repercussions to an organization deleting a bad package version from their Rock Shop catalog if they get a report about that particular version breaking?

Right now we can't delete bad package versions, the call to the web server errors when trying to do so.

[gdalam]

I understand that the point of the commit I referenced is to handle incorrect file paths (which can be caused by improperly packaged plugin submissions) - so I suppose the best solution isn't just to let the install continue as it did with prior versions. But at the very least we need to have the ability to delete old versions of our plugins from the Rock Shop so it doesn't try to install them, and the error returned to the user could use some more clarity. Currently the error says that the version was installed without completing action - whereas it wasn't actually installed at all, is the real culprit of the problem, and will prevent the plugin from being installed beyond that version.

[shauncummings]

@gdalam I have updated the error message to more clearly indicate that the package was not successfully installed. As Michael indicated and you seem to agree, there are reasons we do not want to continue processing packages after one item in the chain fails, because the later package versions may make assumptions about the actions that were performed.

If you have not resolved the packaging issue that prompted this issue, please contact info@sparkdevnetwork.org. You can either supply the correct package or ask them to deactivate the problematic one (if you've already corrected the issue with a later package version, which sounds like the case here). We will pursue adding some verbiage to the plugin author facing website to make this contact route a little more obvious.