Address items from feedback. Make MountConfig use values rather than

pointers and change how config is read to compensate.
This commit is contained in:
Jeff Mitchell 2015-09-09 15:24:45 -04:00
parent c460ff10ca
commit ace611d56d
10 changed files with 151 additions and 167 deletions

View File

@ -2,9 +2,9 @@ package api
import (
"fmt"
"time"
"github.com/fatih/structs"
"github.com/hashicorp/vault/vault"
)
func (c *Sys) ListMounts() (map[string]*Mount, error) {
@ -79,15 +79,12 @@ func (c *Sys) Remount(from, to string) error {
return err
}
func (c *Sys) TuneMount(path string, config vault.MountConfig) error {
func (c *Sys) TuneMount(path string, config APIMountConfig) error {
if err := c.checkMountPath(path); err != nil {
return err
}
body := map[string]interface{}{
"config": config,
}
body := structs.Map(config)
r := c.c.NewRequest("POST", fmt.Sprintf("/v1/sys/mounts/%s/tune", path))
if err := r.SetJSONBody(body); err != nil {
return err
@ -123,7 +120,12 @@ func (c *Sys) checkMountPath(path string) error {
}
type Mount struct {
Type string `json:"type" structs:"type"`
Description string `json:"description" structs:"description"`
Config vault.MountConfig `json:"config" structs:"config"`
Type string `json:"type" structs:"type"`
Description string `json:"description" structs:"description"`
Config APIMountConfig `json:"config" structs:"config"`
}
type APIMountConfig struct {
DefaultLeaseTTL *time.Duration `json:"default_lease_ttl" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"`
MaxLeaseTTL *time.Duration `json:"max_lease_ttl" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"`
}

View File

@ -6,7 +6,6 @@ import (
"time"
"github.com/hashicorp/vault/api"
"github.com/hashicorp/vault/vault"
)
// MountCommand is a Command that mounts a new mount.
@ -51,7 +50,7 @@ func (c *MountCommand) Run(args []string) int {
mountInfo := &api.Mount{
Type: mountType,
Description: description,
Config: vault.MountConfig{},
Config: api.APIMountConfig{},
}
if defaultLeaseTTL != "" {

View File

@ -5,7 +5,7 @@ import (
"strings"
"time"
"github.com/hashicorp/vault/vault"
"github.com/hashicorp/vault/api"
)
// MountTuneCommand is a Command that remounts a mounted secret backend
@ -34,7 +34,7 @@ func (c *MountTuneCommand) Run(args []string) int {
path := args[0]
mountConfig := vault.MountConfig{}
mountConfig := api.APIMountConfig{}
if defaultLeaseTTL != "" {
defTTL, err := time.ParseDuration(defaultLeaseTTL)
if err != nil {

View File

@ -253,49 +253,37 @@ func TestSysTuneMount(t *testing.T) {
// Shorter than system default
resp = testHttpPost(t, token, addr+"/v1/sys/mounts/foo/tune", map[string]interface{}{
"config": map[string]interface{}{
"default_lease_ttl": time.Duration(time.Hour * 72),
},
"default_lease_ttl": time.Duration(time.Hour * 72),
})
testResponseStatus(t, resp, 204)
// Longer than system default
// Longer than system max
resp = testHttpPost(t, token, addr+"/v1/sys/mounts/foo/tune", map[string]interface{}{
"config": map[string]interface{}{
"default_lease_ttl": time.Duration(time.Hour * 72000),
},
"default_lease_ttl": time.Duration(time.Hour * 72000),
})
testResponseStatus(t, resp, 400)
// Longer than system default
resp = testHttpPost(t, token, addr+"/v1/sys/mounts/foo/tune", map[string]interface{}{
"config": map[string]interface{}{
"max_lease_ttl": time.Duration(time.Hour * 72000),
},
"max_lease_ttl": time.Duration(time.Hour * 72000),
})
testResponseStatus(t, resp, 204)
// Longer than backend max
resp = testHttpPost(t, token, addr+"/v1/sys/mounts/foo/tune", map[string]interface{}{
"config": map[string]interface{}{
"default_lease_ttl": time.Duration(time.Hour * 72001),
},
"default_lease_ttl": time.Duration(time.Hour * 72001),
})
testResponseStatus(t, resp, 400)
// Shorter than backend default
resp = testHttpPost(t, token, addr+"/v1/sys/mounts/foo/tune", map[string]interface{}{
"config": map[string]interface{}{
"max_lease_ttl": time.Duration(time.Hour * 1),
},
"max_lease_ttl": time.Duration(time.Hour * 1),
})
testResponseStatus(t, resp, 400)
// Shorter than backend max, longer than system max
resp = testHttpPost(t, token, addr+"/v1/sys/mounts/foo/tune", map[string]interface{}{
"config": map[string]interface{}{
"default_lease_ttl": time.Duration(time.Hour * 71999),
},
"default_lease_ttl": time.Duration(time.Hour * 71999),
})
testResponseStatus(t, resp, 204)
@ -338,10 +326,8 @@ func TestSysTuneMount(t *testing.T) {
resp = testHttpGet(t, token, addr+"/v1/sys/mounts/foo/tune")
actual = map[string]interface{}{}
expected = map[string]interface{}{
"config": map[string]interface{}{
"default_lease_ttl": float64(time.Duration(time.Hour * 71999)),
"max_lease_ttl": float64(time.Duration(time.Hour * 72000)),
},
"default_lease_ttl": float64(time.Duration(time.Hour * 71999)),
"max_lease_ttl": float64(time.Duration(time.Hour * 72000)),
}
testResponseStatus(t, resp, 200)
@ -352,20 +338,16 @@ func TestSysTuneMount(t *testing.T) {
// Set a low max
resp = testHttpPost(t, token, addr+"/v1/sys/mounts/secret/tune", map[string]interface{}{
"config": map[string]interface{}{
"default_lease_ttl": time.Duration(time.Second * 40),
"max_lease_ttl": time.Duration(time.Second * 80),
},
"default_lease_ttl": time.Duration(time.Second * 40),
"max_lease_ttl": time.Duration(time.Second * 80),
})
testResponseStatus(t, resp, 204)
resp = testHttpGet(t, token, addr+"/v1/sys/mounts/secret/tune")
actual = map[string]interface{}{}
expected = map[string]interface{}{
"config": map[string]interface{}{
"default_lease_ttl": float64(time.Duration(time.Second * 40)),
"max_lease_ttl": float64(time.Duration(time.Second * 80)),
},
"default_lease_ttl": float64(time.Duration(time.Second * 40)),
"max_lease_ttl": float64(time.Duration(time.Second * 80)),
}
testResponseStatus(t, resp, 200)

View File

@ -43,11 +43,11 @@ func (d dynamicSystemView) fetchTTLs() (def, max time.Duration, retErr error) {
def = d.core.defaultLeaseTTL
max = d.core.maxLeaseTTL
if me.Config.DefaultLeaseTTL != nil && *me.Config.DefaultLeaseTTL != 0 {
def = *me.Config.DefaultLeaseTTL
if me.Config.DefaultLeaseTTL != 0 {
def = me.Config.DefaultLeaseTTL
}
if me.Config.MaxLeaseTTL != nil && *me.Config.MaxLeaseTTL != 0 {
max = *me.Config.MaxLeaseTTL
if me.Config.MaxLeaseTTL != 0 {
max = me.Config.MaxLeaseTTL
}
return

View File

@ -53,9 +53,13 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) logical.Backend
Type: framework.TypeString,
Description: strings.TrimSpace(sysHelp["mount_path"][0]),
},
"config": &framework.FieldSchema{
Type: framework.TypeMap,
Description: strings.TrimSpace(sysHelp["mount_config"][0]),
"default_lease_ttl": &framework.FieldSchema{
Type: framework.TypeDurationSecond,
Description: strings.TrimSpace(sysHelp["tune_default_lease_ttl"][0]),
},
"max_lease_ttl": &framework.FieldSchema{
Type: framework.TypeDurationSecond,
Description: strings.TrimSpace(sysHelp["tune_max_lease_ttl"][0]),
},
},
@ -415,13 +419,6 @@ func (b *SystemBackend) handleMount(
Config: config,
}
if me.Config.DefaultLeaseTTL == nil {
me.Config.DefaultLeaseTTL = new(time.Duration)
}
if me.Config.MaxLeaseTTL == nil {
me.Config.MaxLeaseTTL = new(time.Duration)
}
// Attempt mount
if err := b.Core.mount(me); err != nil {
b.Backend.Logger().Printf("[ERR] sys: mount %#v failed: %v", me, err)
@ -513,14 +510,10 @@ func (b *SystemBackend) handleMountConfig(
return handleError(err)
}
config := MountConfig{
DefaultLeaseTTL: &def,
MaxLeaseTTL: &max,
}
resp := &logical.Response{
Data: map[string]interface{}{
"config": config,
"default_lease_ttl": def,
"max_lease_ttl": max,
},
}
@ -541,27 +534,45 @@ func (b *SystemBackend) handleMountTune(
path += "/"
}
var config MountConfig
configMap := data.Get("config").(map[string]interface{})
if configMap == nil || len(configMap) == 0 {
return logical.ErrorResponse(
"invalid parameters; 'config' empty or not supplied"),
logical.ErrInvalidRequest
// Prevent protected paths from being changed
for _, p := range protectedMounts {
if strings.HasPrefix(path, p) {
err := fmt.Errorf("[ERR] core: cannot tune '%s'", path)
b.Backend.Logger().Print(err)
return handleError(err)
}
}
err := mapstructure.Decode(configMap, &config)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf(
"unable to convert given mount config information: %s", err)),
logical.ErrInvalidRequest
}
// Attempt tune
if err := b.Core.tuneMount(path, config); err != nil {
b.Backend.Logger().Printf("[ERR] sys: tune of path '%s' failed: %v", path, err)
mountEntry := b.Core.router.MatchingMountEntry(path)
if mountEntry == nil {
err := fmt.Errorf("[ERR] sys: tune of path '%s' failed: no mount entry found", path)
b.Backend.Logger().Print(err)
return handleError(err)
}
newMountConfig := mountEntry.Config
// Timing configuration parameters
{
var needTTLTune bool
defTTLInt, ok := data.GetOk("default_lease_ttl")
if ok {
newMountConfig.DefaultLeaseTTL = time.Duration(defTTLInt.(int))
needTTLTune = true
}
maxTTLInt, ok := data.GetOk("max_lease_ttl")
if ok {
newMountConfig.MaxLeaseTTL = time.Duration(maxTTLInt.(int))
needTTLTune = true
}
if needTTLTune {
if err := b.tuneMountTTLs(path, &mountEntry.Config, &newMountConfig); err != nil {
b.Backend.Logger().Printf("[ERR] sys: tune of path '%s' failed: %v", path, err)
return handleError(err)
}
}
}
return nil, nil
}
@ -972,6 +983,14 @@ west coast.
and max_lease_ttl.`,
},
"tune_default_lease_ttl": {
`The default lease TTL for this mount.`,
},
"tune_max_lease_ttl": {
`The max lease TTL for this mount.`,
},
"remount": {
"Move the mount point of an already-mounted backend.",
`

View File

@ -0,0 +1,51 @@
package vault
import (
"errors"
"fmt"
)
// tuneMount is used to set config on a mount point
func (b *SystemBackend) tuneMountTTLs(path string, meConfig, newConfig *MountConfig) error {
if meConfig.MaxLeaseTTL == newConfig.MaxLeaseTTL &&
meConfig.DefaultLeaseTTL == newConfig.DefaultLeaseTTL {
return nil
}
if meConfig.DefaultLeaseTTL != 0 {
if newConfig.MaxLeaseTTL < meConfig.DefaultLeaseTTL {
if newConfig.DefaultLeaseTTL == 0 {
return fmt.Errorf("New backend max lease TTL of %d less than backend default lease TTL of %d",
newConfig.MaxLeaseTTL, meConfig.DefaultLeaseTTL)
}
if newConfig.MaxLeaseTTL < newConfig.DefaultLeaseTTL {
return fmt.Errorf("New backend max lease TTL of %d less than new backend default lease TTL of %d",
newConfig.MaxLeaseTTL, newConfig.DefaultLeaseTTL)
}
}
}
if meConfig.MaxLeaseTTL == 0 {
if newConfig.DefaultLeaseTTL > b.Core.maxLeaseTTL {
return fmt.Errorf("New backend default lease TTL of %d greater than system max lease TTL of %d",
newConfig.DefaultLeaseTTL, b.Core.maxLeaseTTL)
}
} else {
if meConfig.MaxLeaseTTL < newConfig.DefaultLeaseTTL {
return fmt.Errorf("New backend default lease TTL of %d greater than backend max lease TTL of %d",
newConfig.DefaultLeaseTTL, meConfig.MaxLeaseTTL)
}
}
meConfig.MaxLeaseTTL = newConfig.MaxLeaseTTL
meConfig.DefaultLeaseTTL = newConfig.DefaultLeaseTTL
// Update the mount table
if err := b.Core.persistMounts(b.Core.mounts); err != nil {
return errors.New("failed to update mount table")
}
b.Core.logger.Printf("[INFO] core: tuned '%s'", path)
return nil
}

View File

@ -47,16 +47,16 @@ func TestSystemBackend_mounts(t *testing.T) {
"type": "generic",
"description": "generic secret storage",
"config": map[string]interface{}{
"default_lease_ttl": resp.Data["secret/"].(map[string]interface{})["config"].(map[string]interface{})["default_lease_ttl"].(*time.Duration),
"max_lease_ttl": resp.Data["secret/"].(map[string]interface{})["config"].(map[string]interface{})["max_lease_ttl"].(*time.Duration),
"default_lease_ttl": resp.Data["secret/"].(map[string]interface{})["config"].(map[string]interface{})["default_lease_ttl"].(time.Duration),
"max_lease_ttl": resp.Data["secret/"].(map[string]interface{})["config"].(map[string]interface{})["max_lease_ttl"].(time.Duration),
},
},
"sys/": map[string]interface{}{
"type": "system",
"description": "system endpoints used for control, policy and debugging",
"config": map[string]interface{}{
"default_lease_ttl": resp.Data["sys/"].(map[string]interface{})["config"].(map[string]interface{})["default_lease_ttl"].(*time.Duration),
"max_lease_ttl": resp.Data["sys/"].(map[string]interface{})["config"].(map[string]interface{})["max_lease_ttl"].(*time.Duration),
"default_lease_ttl": resp.Data["sys/"].(map[string]interface{})["config"].(map[string]interface{})["default_lease_ttl"].(time.Duration),
"max_lease_ttl": resp.Data["sys/"].(map[string]interface{})["config"].(map[string]interface{})["max_lease_ttl"].(time.Duration),
},
},
}

View File

@ -118,8 +118,8 @@ type MountEntry struct {
// MountConfig is used to hold settable options
type MountConfig struct {
DefaultLeaseTTL *time.Duration `json:"default_lease_ttl" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"` // Override for global default
MaxLeaseTTL *time.Duration `json:"max_lease_ttl" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"` // Override for global default
DefaultLeaseTTL time.Duration `json:"default_lease_ttl" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"` // Override for global default
MaxLeaseTTL time.Duration `json:"max_lease_ttl" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"` // Override for global default
}
// Returns a deep copy of the mount entry
@ -330,7 +330,6 @@ func (c *Core) remount(src, dst string) error {
// Update the entry in the mount table
newTable := c.mounts.Clone()
for _, ent := range newTable.Entries {
//FIXME: Update systemview in each logical backend
if ent.Path == src {
ent.Path = dst
ent.Tainted = false
@ -358,66 +357,6 @@ func (c *Core) remount(src, dst string) error {
return nil
}
// tuneMount is used to set config on a mount point
func (c *Core) tuneMount(path string, config MountConfig) error {
// Ensure we end the path in a slash
if !strings.HasSuffix(path, "/") {
path += "/"
}
// Prevent protected paths from being changed
for _, p := range protectedMounts {
if strings.HasPrefix(path, p) {
return fmt.Errorf("[ERR] core: cannot tune '%s'", path)
}
}
me := c.router.MatchingMountEntry(path)
if me == nil {
return fmt.Errorf("[ERR] core: no matching mount at '%s'", path)
}
if config.MaxLeaseTTL != nil {
if *me.Config.DefaultLeaseTTL != 0 {
if *config.MaxLeaseTTL < *me.Config.DefaultLeaseTTL {
return fmt.Errorf("Given backend max lease TTL of %d less than backend default lease TTL of %d",
*config.MaxLeaseTTL, *me.Config.DefaultLeaseTTL)
}
}
if *config.MaxLeaseTTL == 0 {
*me.Config.MaxLeaseTTL = 0
} else {
me.Config.MaxLeaseTTL = config.MaxLeaseTTL
}
}
if config.DefaultLeaseTTL != nil {
if *me.Config.MaxLeaseTTL == 0 {
if *config.DefaultLeaseTTL > c.maxLeaseTTL {
return fmt.Errorf("Given default lease TTL of %d greater than system default lease TTL of %d",
*config.DefaultLeaseTTL, c.maxLeaseTTL)
}
} else {
if *me.Config.MaxLeaseTTL != 0 && *me.Config.MaxLeaseTTL < *config.DefaultLeaseTTL {
return fmt.Errorf("Given default lease TTL of %d greater than backend max lease TTL of %d",
*config.DefaultLeaseTTL, *me.Config.MaxLeaseTTL)
}
}
if *config.DefaultLeaseTTL == 0 {
*me.Config.DefaultLeaseTTL = 0
} else {
me.Config.DefaultLeaseTTL = config.DefaultLeaseTTL
}
}
// Update the mount table
if err := c.persistMounts(c.mounts); err != nil {
return errors.New("failed to update mount table")
}
c.logger.Printf("[INFO] core: tuned '%s'", path)
return nil
}
// loadMounts is invoked as part of postUnseal to load the mount table
func (c *Core) loadMounts() error {
// Load the existing mount table
@ -563,20 +502,12 @@ func defaultMountTable() *MountTable {
Type: "generic",
Description: "generic secret storage",
UUID: uuid.GenerateUUID(),
Config: MountConfig{
DefaultLeaseTTL: new(time.Duration),
MaxLeaseTTL: new(time.Duration),
},
}
sysMount := &MountEntry{
Path: "sys/",
Type: "system",
Description: "system endpoints used for control, policy and debugging",
UUID: uuid.GenerateUUID(),
Config: MountConfig{
DefaultLeaseTTL: new(time.Duration),
MaxLeaseTTL: new(time.Duration),
},
}
table.Entries = append(table.Entries, genericMount)
table.Entries = append(table.Entries, sysMount)

View File

@ -81,10 +81,8 @@ description: |-
```javascript
{
"config": {
"default_lease_ttl": 0,
"max_lease_ttl": 0
}
"default_lease_ttl": 0,
"max_lease_ttl": 0
}
```
@ -152,14 +150,16 @@ description: |-
<dd>
<ul>
<li>
<span class="param">config</span>
<span class="param-flags">required</span>
Config options for this mount. This is an object with
two possible values: `default_lease_ttl` and
`max_lease_ttl`. These control the default and
maximum lease time-to-live, respectively. If set
on a specific mount, this overrides the global
defaults.
<span class="param">default_lease_ttl</span>
<span class="param-flags">optional</span>
The default time-to-live. If set on a specific mount,
overrides the global default.
</li>
<li>
<span class="param">max_lease_ttl</span>
<span class="param-flags">optional</span>
The maximum time-to-live. If set on a specific mount,
overrides the global default.
</li>
</ul>
</dd>