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

[full-ci] read-only storagehome and storageusers #2230

Merged
merged 1 commit into from
Jul 12, 2021
Merged

[full-ci] read-only storagehome and storageusers #2230

merged 1 commit into from
Jul 12, 2021

Conversation

micbar
Copy link
Contributor

@micbar micbar commented Jun 29, 2021

Description

For migration scenarios it can be handy to use a storage provider in read-only mode. For example if we use the ownCloud SQL driver to connect to a ownCloud Classic, readonly mode is a vital step in the migration process.

How it works

  • The read-only interceptor uses a list of known request types which are allowed.
  • Known request types which change data on the storage are blocked and return a grpc error which will be forwarded to WebDAV.
  • Unknown Request Types will be blocked and throw a GRPC error
  • The interceptor changes the grants on the resources which will cause WebDAV to return read-only permissions.

WebUI

  • The webUI reflects the webdav permissions and "greys out" or hides all actions that would change the files on the storage

Config

  • To enable the interceptor, use STORAGE_HOME_READ_ONLY=true and STORAGE_USERS_READ_ONLY=true
  • alternative: use OCIS_STORAGE_READ_ONLY=true

Known Issue

  • Currently we need to add the interceptor to storagehome and storageusers which share the same physical storage (which is IMO weird and will be changed in the future)

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • 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:

@update-docs
Copy link

update-docs bot commented Jun 29, 2021

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.

@micbar micbar changed the title [WIP] read-only storagehome [WIP] read-only storagehome and storageusers Jun 30, 2021
@micbar micbar force-pushed the read-only branch 2 times, most recently from 8bb09c9 to d0e727d Compare July 9, 2021 07:03
@micbar micbar requested review from butonic and C0rby July 9, 2021 07:04
@micbar micbar marked this pull request as ready for review July 9, 2021 07:07
@micbar micbar force-pushed the read-only branch 2 times, most recently from 6f71e74 to efe7c15 Compare July 9, 2021 09:18
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

1 similar comment
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@micbar micbar changed the title [WIP] read-only storagehome and storageusers [WIP][full-ci] read-only storagehome and storageusers Jul 9, 2021
@micbar micbar force-pushed the read-only branch 2 times, most recently from fde16ff to 0e22703 Compare July 9, 2021 13:31
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 9, 2021

Kudos, SonarCloud Quality Gate passed!

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

0.0% 0.0% Coverage
7.4% 7.4% Duplication

@micbar micbar changed the title [WIP][full-ci] read-only storagehome and storageusers [full-ci] read-only storagehome and storageusers Jul 11, 2021
@@ -142,6 +142,12 @@ func storageHomeConfigFromStruct(c *cli.Context, cfg *config.Config) map[string]
},
},
}
if cfg.Reva.StorageHome.ReadOnly {
gcfg := rcfg["grpc"].(map[string]interface{})
gcfg["interceptors"] = map[string]interface{}{
Copy link
Member

Choose a reason for hiding this comment

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

This would replace an existing map. You will have to check if it exists and create a new map or append to the existing one. Move that to a function with a name that explains what it does, eg. 'appendInterceptor()' then we can use it to add arbitrary interceptors... Or maybe even generic map[string]interface{} things 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. But the whole struct is created in L113. I don't see a case where there is already another interceptor.

I was digging into it but let us merge now. We will see what is needed when we have a real use case for more interceptors.

if cfg.Reva.StorageUsers.ReadOnly {
gcfg := rcfg["grpc"].(map[string]interface{})
gcfg["interceptors"] = map[string]interface{}{
"readonly": map[string]interface{}{},
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same answer.

@micbar micbar merged commit 98563bb into master Jul 12, 2021
@delete-merged-branch delete-merged-branch bot deleted the read-only branch July 12, 2021 06:43
ownclouders pushed a commit that referenced this pull request Jul 12, 2021
Merge: d59e828 de5aefb
Author: Michael Barz <[email protected]>
Date:   Mon Jul 12 08:43:10 2021 +0200

    Merge pull request #2230 from owncloud/read-only

    [full-ci] read-only storagehome and storageusers
@micbar micbar mentioned this pull request Jul 13, 2021
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants