Marking local storage as non-writeable doesn't reflect in files app configuration


#1

Observed Behavior:
Setup local file storage. Perhaps upload a file >1MB to use the storage. Make the path non-writeable by removing permissions (in my case I changed the owner of the path configured in storage.local-disk.path to make writes fail).

Go to Applications -> Click Configure beside Files -> Read Storage Engines table, look at “local-disk” row and “Writeable” column: it still says yes.

Even if the setup issue discovering the local file storage is not configured, it will still say yes.

Expected Behavior:
Storage Engines table should reflect the reality of the storage engine (is it writeable or not).

Phabricator Version:
abc26aa96f4cfa31ecf6ffda83e4c3a948f21eb4 (Feb 20)

Reproduction Steps:
Setup local file storage. Perhaps upload a file >1MB to use the storage. Make the path non-writeable by removing permissions (in my case I changed the owner of the path configured in storage.local-disk.path to make writes fail).

Go to Applications -> Click Configure beside Files -> Read Storage Engines table, look at “local-disk” row and “Writeable” column.


#2

The settings inside the Files application are there to configure how Phabricator will attempt to use those storage engines. Marking a storage engine as “writeable” just tells Phabricator that it can attempt to make writes using that engine, and does not (and can not) reflect the underlying writability reality.


#3

How does one mark a storage as “not-writeable” in that case? Since the only way I could see the “writeable” column go to “Yes” is by configuring a storage engine properly. But configuring it improperly (for lack of a better phrase) doesn’t do the opposite.


#4

My mistake; I was confusing Phabricator aspirations with Phabricator reality. See https://secure.phabricator.com/T13256 for plans to improve the configurability of storage engines, including adding the ability to explicitly mark storage engines as read-only.

Regardless, the “Writeable” column on that config page only reflects whether or not Phabricator has been configured sufficiently to enable writes to that engine. There is no way for Phabricator to know if a configured storage engine is, in reality, writeable or not.


#5

How does one mark a storage as “not-writeable” in that case?

Remove the configured value of storage.local-disk.path, likely with:

phabricator/ $ ./bin/config delete storage.local-disk.path

#6

T13256 discusses the ability to mark an engine as “read-only”, but I don’t think that’s what you’re trying to do. My understanding is that you’re trying to disable the engine completely, and you expected configuring storage.local-disk.path to point somewhere but then making that path non-writable would disable the engine.

Phabricator does not interpret this as “disable the engine”. It interprets any value set in storage.local-disk.path to mean “enable the engine”, and assumes that value to be a directory you expect to be a valid, writable directory, because you could simply delete the value if you want to stop writes and disable the engine.

That is, the way to stop writes is to remove the value that says “write here”, not to change it to point somewhere invalid. If Phabricator interpreted “value is configured, but invalid” as an acceptable configuration meaning “the user wants to disable writes”, it could not raise errors to guide users through the case where this actually means “the user made a mistake in their configuration”, which is overwhelmingly the more likely reason that the value is configured but not valid.


#7

Actually, T13256 is exactly what I wanted, but I didn’t want to submit some giant feature request when I thought the functionality was exposed in a less obvious manner.

My big problems arise when I’m trying to understand the behaviour of Phabricator (and its components). I wanted to understand what happens when I upload a chunked file (which storage engine does that use) vs an LFS object etc etc, so I wanted to get ways to disable storage (but I didn’t want to de-configure it completely like you suggested, since that will break all the files I am not ready to migrate).

Anyways, T13256 is exactly it.

Also, I learned (happily) that git lfs will store in S3 when it is properly configured, which may seem obvious to you, but for us it’s a huge quality of life improvement since we can reduce our storage pain on our repo servers without sacrificing availability.

This can be closed then, I think T13256 hits my goals on the head. Thank you for the information.


closed #8