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

Don't mount shares with failed underlying storages #41014

Merged
merged 3 commits into from
Oct 6, 2023

Conversation

jvillafanez
Copy link
Member

Description

Share mounts with failed underlying storages won't be mounted. A common cause of such failure is the underlying storage has been removed or the sharer has been deleted.
This should help the occ files:remove-storage --show-candidates command to detect additional storages that should be removed.

Related Issue

https://github.com/owncloud/enterprise/issues/6063

Motivation and Context

How Has This Been Tested?

Manually tested.

  1. Setup a external storage and allow sharing in it
  2. Share the external storage with another user
  3. Check the new user can access to the storage through the share
  4. Remove the external storage
  5. The new user can't access to the share
  6. The occ files:remove-storage --show-candidates now shows the external storage

Step 5 might be required in order for ownCloud to notice the unavailable storage.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@update-docs
Copy link

update-docs bot commented Oct 6, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@pako81
Copy link

pako81 commented Oct 6, 2023

Tested following scenarios on 10.13.2 RC1.

  • Share the whole mount and then delete the external storage

occ files:remove-storage --show-candidates correctly finds the storage candidate 👍

  • Share multiple files and folders in the mount and delete the external storage

occ files:remove-storage --show-candidates correctly finds the storage candidate 👍

  • Share some files in the mount and remove the user sharing those files

occ files:remove-storage --show-candidates correctly finds the storage candidate 👍

  • The external storage becomes temporary unavailable

occ files:remove-storage --show-candidates does not display the storage as candidate 👍

@pako81 pako81 self-requested a review October 6, 2023 09:43
@pako81
Copy link

pako81 commented Oct 6, 2023

JP if we could had tests and changelog that would be great

@pako81
Copy link

pako81 commented Oct 6, 2023

I guess we should switch this to the release-10.13.2 branch.

@jnweiger jnweiger changed the base branch from master to release-10.13.2 October 6, 2023 10:47
@pako81
Copy link

pako81 commented Oct 6, 2023

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@jvillafanez
Copy link
Member Author

Maybe we should cherry-pick the commits instead of switching the target branch. There are changes in the Japanese translations that I don't know if they should be there or not.

@jnweiger
Copy link
Contributor

jnweiger commented Oct 6, 2023

@phil-davis https://drone.owncloud.com/owncloud/core/39225/171/15 and https://drone.owncloud.com/owncloud/core/39225/173/15 fail but for instance https://drone.owncloud.com/owncloud/core/39225/170/15 goes through. Maybe you have an idea.

I've re-triggerd drone. Those errors are purely sporadic, I got a clean CI run now.

@jnweiger jnweiger changed the base branch from release-10.13.2 to master October 6, 2023 14:00
@jnweiger
Copy link
Contributor

jnweiger commented Oct 6, 2023

Maybe we should cherry-pick the commits instead of switching the target branch. There are changes in the Japanese translations that I don't know if they should be there or not.

You are right. The Translation changes in this PR don't look good. Lots of deletions. Let's merge into master and cherry pick from there.

@jnweiger jnweiger merged commit 7706791 into master Oct 6, 2023
@delete-merged-branch delete-merged-branch bot deleted the not_mount_failed_shares branch October 6, 2023 14:02
jnweiger pushed a commit that referenced this pull request Oct 6, 2023
* Don't mount shares with failed underlying storages

* Add and adjust unit tests

* Add changelog entry
@phil-davis
Copy link
Contributor

You are right. The Translation changes in this PR don't look good. Lots of deletions. Let's merge into master and cherry pick from there.

FYI #40332 - that has been a problem for a long time. @DeepDiver1975 hopes to have fixed it yesterday. The Japanese translations should come back into master in a few hours, and not disappear any more.

netgate-git-updates pushed a commit to pfsense/FreeBSD-ports that referenced this pull request Nov 1, 2023
- Bump PORTREVISION for package change

Upstream rerolled the tarball to include a bugfix "Do not mount shared storage
which are failing".

Reference:	owncloud/core#41014
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.

4 participants