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

storage uuids are not randomized with init #5064

Closed
butonic opened this issue Nov 16, 2022 · 8 comments · Fixed by #5091
Closed

storage uuids are not randomized with init #5064

butonic opened this issue Nov 16, 2022 · 8 comments · Fixed by #5091
Assignees
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug

Comments

@butonic
Copy link
Member

butonic commented Nov 16, 2022

Currently every deployment uses tha same uuid for the users storage ... that is a non starter for a federated deployment.

We have to initialize the Storageid for the users storage (which in fact currently also is our project storage ... so it is already misused, but that is a separate issue) so that it is possible to determine the storageprovider and distinguish an which instance a space is located ...

"providerid": "1284d238-aa92-42ce-bdc4-0b0000009157",
needs to be made configurabe and set to a random uuid with init

Breaking Change

All WebDAV urls will change

The providerID is the first part of the ID string in the url. It will change from the existing hardcoded uuid (1284d238-aa92-42ce-bdc4-0b0000009157) to a new random uuid.

- "webDavUrl": "https://localhost:9200/dav/spaces/1284d238-aa92-42ce-bdc4-0b0000009157$4c510ada-c86b-4815-8820-42cdf82c3d51"
+ "webDavUrl": "https://localhost:9200/dav/spaces/bc631809-54a9-4b1e-9d6e-d9bdf5088eab$4c510ada-c86b-4815-8820-42cdf82c3d51"

Consequences for Clients

  • Web: Can handle it
  • iOS: Can handle it
  • Android: not known
  • Desktop: All sync connections need to be recreated

Alternative Solution

  • Fallback to the existing mount ID
  • This is not a good approach IMO, we really risk to have a lot of storage providers out there with the same ID
  • Later migrations will be even more painful
@butonic butonic changed the title storage ids are not initialized with init storage uuids are not randomized with init Nov 16, 2022
@micbar micbar added the Priority:p2-high Escalation, on top of current planning, release blocker label Nov 16, 2022
@micbar micbar added this to the 2.0.0 General Availability milestone Nov 16, 2022
@micbar micbar self-assigned this Nov 22, 2022
@micbar
Copy link
Contributor

micbar commented Nov 22, 2022

@michaelstingl @TheOneRing This would become a problem for all existing sync connections for the desktop because all WebDAV Urls will change.

@micbar
Copy link
Contributor

micbar commented Nov 22, 2022

@TheOneRing I updated the top post

@michaelstingl
Copy link
Contributor

@micbar could you post the full information of a Space before and after this change from the Graph API?

@wkloucek
Copy link
Contributor

All references to a fileid will be broken after a storage uuid change. This affects:

  • private shares
  • public shares
  • bookmarks (Web)
  • thumbnails cache

@wkloucek
Copy link
Contributor

wkloucek commented Nov 23, 2022

My proposal would be:

  • Don't change the hardcoded UUID for now
  • Introduce support for multiple storage providers in oCIS. Because then we need also support the move of spaces from one to another storage.
  • Then we have a migration path "for free" because we can move all spaces from the "well-known / non unique uuid" storage to a new random uuid storage, that we later can use for federation.

This proposal assumes that we will do multi-storage oCIS instances before we will do federated oCIS instances.

@micbar
Copy link
Contributor

micbar commented Nov 23, 2022

@wkloucek what is the gain of value?

If we do it now, only a few people need to recreate the desktop sync.

If we do nothing now, we would have thousands of storage providers out there which all have the same provider id. This will explode and cause a lot of painful migrations.

@wkloucek
Copy link
Contributor

If we do it now, only a few people need to recreate the desktop sync.

At least I have no numbers. So I don't know how many installations are out there already.
But introducing a that huge breaking change between a rc.1 and rc.2 sounds odd.

If we do nothing now, we would have thousands of storage providers out there which all have the same provider id. This will explode and cause a lot of painful migrations.

If we find a way to use a random UUID only for new storages, we would have "only a few people" that need a migration later on. And the migration is needed when these instances will be using federation.

@butonic
Copy link
Member Author

butonic commented Nov 23, 2022

The problem I see with the hardcoded UUID for the storageprovider is that we will get collisions of storage provider ids that exist in multiple ocis deployments. hmmm ... clients will have to deal with that anyway because there might be malicious instances that could reuse the storageid of a resoucre to try and confuse clients to send them data ...

the current user storage uuid was handcrafted by me to contain the port number at the end ... to make debugging easier ... but the idea was always to initialize with a unique identifier.

while cern is using human readable names I am strongly recommending uuids because we will run into merge scenarios. And for those provider id collisions ... suck big time. Maybe I overthought this. But I have been ther and I don't want to start rolling out ocis deployments with the same fallback uuid everywhere.

I would change the uuid now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants