Mount conflict inconsistency

[otakup0pe]

We stumbled across something interesting with regards to how generic mountpoint conflicts are handled. We are not sure if this is the desired behavior or not. We understand that "sub mounts" are not supported, however this constraint seems to be only enforced in one direction.

This particular set of commands exhibits the behavior we expect.

$ vault mount -path foo generic
Successfully mounted 'generic' at 'foo'!
$ vault mount -path foo/bar generic
Mount error: Error making API request.

URL: POST http://127.0.0.1:8200/v1/sys/mounts/foo/bar
Code: 400. Errors:

* existing mount at foo/

The next set is what we find confusing. One might (possibly naively) expect that the second command, a "sub mount", would conflict with the first. Instead what we see is that the second mount works fine, with the third behaving consistently (as compared to the first example).

$ vault mount -path fom/bar generic
Successfully mounted 'generic' at 'fom/bar'!
$ vault mount -path fom generic
Successfully mounted 'generic' at 'fom'!
$ vault mount -path fom/baz generic
Mount error: Error making API request.

URL: POST http://127.0.0.1:8200/v1/sys/mounts/fom/baz
Code: 400. Errors:

* existing mount at fom/

If this is considered a bug, I'm totally willing to submit a pull request to fix it. If it is not, then I am totally willing to to submit a pull request to clarify the documentation.

[jefferai]

Hi Jonathan,

It's a bug. We should not only check to see if the new value is a sub mount, of an existing mount, but if it would also be a parent mount to an existing mount. Looking forward to the PR!

[otakup0pe]

Sounds good! To confirm - the desired behavior is detection of any potential sub-path conflict?

[jefferai]

Yep!

Thinking about it there are two ways to solve this. One is traveling up the path hierarchy and looking for conflicts along the way. The other is to simply only allow top level mounts. It'd potentially be a breaking change but I wonder if there is a point in allowing non top level mounts if you can't sub mount.

[otakup0pe]

I'm leaning towards the first option. Disallowing non top-level mounts seems way too opinionated for a piece of software like Vault...

[jefferai]

I mean, mounts are Vault fakery in the first place :-) But I agree.

[jefferai]

@otakup0pe Just wanted to check, are you still intending to work up a PR for this?

[otakup0pe]

I've started poking at it, I'll make sure to reference this issue if I get a PR up...

[jefferai]

Sure. I think it's not too bad actually. We're not worried about speed in this path so really all you have to do is start with the given mount path, then in a loop knock one character off each iteration and check if that path exists.