if mount<backend> is called, confirm that the backend isn't mounted yet before mounting it.

[davepgreene]

This PR makes mounting a previously mounted backend a no-op. This resolves #94.

Previously if a backend (like transit) is mounted before Vaulted is used, a call to mountTransit would throw an error because Vault would complain that the backend was already mounted. A side effect of this behavior is that the API specs for that backend wouldn't be loaded.

Now, the mount<backend> function is short-circuited if the requested backend is already mounted. Instead, we emit the mount event to load the API specs, then return the list of mounts which matches the previous behavior.

[kenjones-cisco]

To achieve the same thing with an extra line of code:

Update prepare method

Vaulted.prototype.prepare = function prepare(vault_token) {
  var self = this;
  return internal.loadState(self, vault_token).then(function () {
    internal.syncMounts(self);
  });
};

When a vault completes the unseal, there is an event emitted that will load all of the state and synchronize the mounts (all of them).

Because the vault is already unsealed when vaulted is instantiated the last step of performing the sync is never performed. By updating the prepare to include the sync as well, all the API endpoints will get loaded based on the existing state that was discovered from the vault.

[davepgreene]

To achieve the same thing with an extra line of code:

Update prepare method

Just to be clear, do you want me to add this to my PR or instead of?

[kenjones-cisco]

My thoughts are instead of so that create behaves like a normal a create and the loading of existing state stays within .prepare.

@chiefy thoughts?

[davepgreene]

That makes sense if Vaulted is the canonical source of truth for how Vault is operated, but the reality is that in our case (and I'm sure plenty of others) Vault is managed entirely out of band and Vaulted is used as a swanky wrapper over doing a bunch of Promisified http.requests.

We ran into this issue in working on https://github.com/rapid7/tokend/pull/58#pullrequestreview-2360938 where we had to manually mount the transit API specs because attempting decryptTransitCipherText failed internally with a Error: Could not find endpoint: transit/decrypt/:id in API defintions. Attempting to mount the transit backend resolved as an error from Vault as one would expect.

In my opinion, there are two solutions.

1. Vaulted should take a cue from the type of request being made (e.g Vaulted.decryptTransitCipherText) and confirm that the backend has been mounted and if so, the API specs have been loaded before the request is made.
2. Vaulted should load all API specs on instantiation and allow Vault to return it's "This backend has not been mounted" error.

@chiefy if you have an approach you'd prefer let me know and I'll either close this PR and open a new one or make the relevant changes here.

[kenjones-cisco]

That is the reason that prepare was created so that a token can be provided directly and the existing state of Vault can be loaded into vaulted.

The thought process is to support those that want to use vaulted to setup and configure Vault, and those that want to interact with Vault.

For those doing the setups via vaulted they would not use prepare as they need to do initialize, unseal, mounting storage backend(s), and auth backend(s).

For the more typical use case where Vault comes setup already, they just instantiate vaulted and call prepare that will orchestrate discovering status and mounts.

The missing step which was an oversight was to load up the endpoints associated with the loaded mounts.

[chiefy]

I agree with @kenjones-cisco that if you have an already initialized/unsealed Vault somewhere, you should be using prepare and we should change that method to add the syncMounts as @kenjones-cisco has above.

[davepgreene]

Makes sense. I'll close this PR and open a new one with the proposed fix in the prepare method.

[davepgreene]

I'll add, as it just occurred to me, I'm not sure I understand the design decision of having API specs load selectively. I suppose it allows requests to unmounted backends to be short-circuited but really, what does that save you but a single HTTP request?

@chiefy Can you explain the rationale?

[chiefy]

@davepgreene the rationale is exactly what you state: saving an HTTP request. If you have a pre-initialized vault, and we fix the prepare method, I don't really see why this is a huge issue?

[davepgreene]

Oh no, it's not! 😄 I just wanted a little more insight into your design.