Consistent consul secrets parameter names

[robmonte]

Description

With the Consul secrets engine improvements, several new parameters were added. Roles for example, but since Vault uses the term roles already, the parameter was named consul_roles. Similarly, the parameter for Consul namespaces was named consul_namespace. For example:

$ vault write consul/roles/my-role consul_roles="role1"
and
$ vault write -namespace="ns1" consul/roles/my-role consul_namespace="ns2"

This has created an inconsistent naming convention for the parameters. A counterexample to the above is policies, as both Consul and Vault use them, yet the parameter is simply named policies without the "consul_" prefix:
$ vault write consul/roles/my-role policies="global-management"

Solution

We could consider renaming the parameters to be consistent, and if so decide which parameters would get renamed.
This draft PR changes most of the remaining parameters for configuring Vault roles to use with Consul. They are listed below:

• local -> consul_local
• policies -> consul_policies
• partition -> consul_partition
• ttl -> consul_ttl
• max_ttl -> consul_max_ttl
• service_identities -> consul_service_identities
• node_identities -> consul_node_identities

As you can see, some of these become particularly long-winded which may not be good UX:
$ vault write consul/roles/my-role consul_local=true consul_ttl=1h consul_max_ttl=2h consul_service_identities="myserv-1:dc1"

Perhaps only a subset of these could be changed yet still make it feel more consistent?

[robmonte]

After taking more time to consider the UX with these changes yesterday, I am backtracking on which parameters could be changed to feel "consistent". I also made a mistake with TTL.

• TTL fields are Vault token TTLs, not Consul fields. This definitely shouldn't be changed.
• The local field changes if Consul SecretIDs are replicated or not. While Vault has a local field, it applies to the secret engine itself and cannot be applied to individual roles or tokens, so I don't consider this conflicting.
• Node and service identity is unique enough so using the "consul_" prefix is superfluous.
• The isolation provided by Consul admin partitions is already provided by Vault namespaces, so Vault is unlikely to adopt the terminology in the future.

 

The current changes in the PR now are:

• A new parameter called consul_policies that is a clone of policies.
• The policies parameter's description states to use consul_policies instead. Should this have a deprecated tag?
• The policy parameter and token_type parameter have Deprecated: true added.
• Removed the "lease" field from the role read response because it is duplicate data and deprecated.

With this implementation, Vault will accept Consul policies via either name policies or consul_policies by passing any values given from the first one into the second.

[austingebauer]

Generally looks good! I agree with your assessment of the various fields in the comment. Left a few small suggestions.

[fairclothjm]

Nice! Looking good! Should we add some unit tests to cover the new fields?