search

mattermost / mattermost-plugin-legal-hold

Plugin to create and manage legal holds in Mattermost
Other
5 stars 6 forks source link

Remove parentheses from legal hold folder path to work on MM Cloud servers #74

Closed mickmister closed 4 months ago

mickmister commented 4 months ago

Summary

On MM cloud servers, this plugin returned no data when downloading the legal hold data from the UI. It produces an empty zip file as noted in https://github.com/mattermost/mattermost-plugin-legal-hold/issues/69

Upon debugging this, I found that having parentheses in the folder path makes it incompatible with the cloud server's file backend. I believe this has to do with this server's usage of bifrost https://github.com/mattermost/bifrost. In the case of using the parenthese (i.e. before the changes in this PR), I've verified that the files are correctly written and read from, though the call to p.FileBackend.ListDirectoryRecursively(legalHold.BasePath()) here returns an empty array of strings in the case of using parentheses and running on the MM Cloud server.

This PR changes the directory where a given legal hold's files are stored in S3/local. The original format was:

fmt.Sprintf("legal_hold/%s_(%s)", lh.Name, lh.ID)

and has been changed to

fmt.Sprintf("legal_hold/%s_%s", lh.Name, lh.ID)

Ticket Link

Fixes https://github.com/mattermost/mattermost-plugin-legal-hold/issues/69

mickmister commented 4 months ago

@agnivade I haven't verified this is specifically related to bifrost, but there is something different about the Cloud server S3 connections that causes this behavior. Any ideas about this?

agnivade commented 4 months ago

@agnivade I haven't verified this is specifically related to bifrost, but there is something different about the Cloud server S3 connections that causes this behavior. Any ideas about this?

Hmm .. there was a similar issue related to the + character (https://github.com/mattermost/bifrost/pull/30), but no I don't think there was an issue with ( or ). And in any case, the PR should fix any such special chars.

You can actually create a spinwick with and without bifrost. If you can repro this purely to be a bifrost issue, then I can take a look at. Let me also cc @lieut-data since he sent the PR.