From aadf0393689d7d875e2c7536e3791c0610fb7f50 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 1 Sep 2015 18:29:30 -0400 Subject: [PATCH] Add DynamicSystemView. This uses a pointer to a pointer to always have up-to-date information. This allows remount to be implemented with the same source and dest, allowing mount options to be changed on the fly. If/when Vault gains the ability to HUP its configuration, this should just work for the global values as well. Need specific unit tests for this functionality. --- api/sys_mounts.go | 16 ++--- builtin/logical/pki/backend_test.go | 8 +-- command/mount.go | 37 +++++----- command/remount.go | 43 ++++++----- logical/framework/field_data.go | 4 +- logical/system_view.go | 27 +++++-- vault/logical_system.go | 57 +++++++-------- vault/logical_system_test.go | 13 ++-- vault/mount.go | 76 +++++++++++++++++--- vault/mount_test.go | 6 +- website/source/docs/http/sys-mounts.html.md | 26 ++++++- website/source/docs/http/sys-remount.html.md | 15 +++- 12 files changed, 225 insertions(+), 103 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index f22afd1920..ee4848b3d8 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -54,7 +54,7 @@ func (c *Sys) Unmount(path string) error { return err } -func (c *Sys) Remount(from, to string, config *vault.MountConfig) error { +func (c *Sys) Remount(from, to string, config vault.MountConfig) error { if err := c.checkMountPath(from); err != nil { return err } @@ -63,11 +63,9 @@ func (c *Sys) Remount(from, to string, config *vault.MountConfig) error { } body := map[string]interface{}{ - "from": from, - "to": to, - } - if config != nil { - body["config"] = *config + "from": from, + "to": to, + "config": config, } r := c.c.NewRequest("POST", "/v1/sys/remount") @@ -91,7 +89,7 @@ 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 vault.MountConfig `json:"config" structs:"config"` } diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index f90142a855..ffaafe4a33 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -28,8 +28,8 @@ func TestBackend_basic(t *testing.T) { b, err := Factory(&logical.BackendConfig{ Logger: nil, System: &logical.StaticSystemView{ - DefaultLeaseTTLVal: &defaultLeaseTTLVal, - MaxLeaseTTLVal: &maxLeaseTTLVal, + DefaultLeaseTTLVal: defaultLeaseTTLVal, + MaxLeaseTTLVal: maxLeaseTTLVal, }, }) if err != nil { @@ -56,8 +56,8 @@ func TestBackend_roles(t *testing.T) { b, err := Factory(&logical.BackendConfig{ Logger: nil, System: &logical.StaticSystemView{ - DefaultLeaseTTLVal: &defaultLeaseTTLVal, - MaxLeaseTTLVal: &maxLeaseTTLVal, + DefaultLeaseTTLVal: defaultLeaseTTLVal, + MaxLeaseTTLVal: maxLeaseTTLVal, }, }) if err != nil { diff --git a/command/mount.go b/command/mount.go index dbe06f6698..61842af63c 100644 --- a/command/mount.go +++ b/command/mount.go @@ -19,8 +19,8 @@ func (c *MountCommand) Run(args []string) int { flags := c.Meta.FlagSet("mount", FlagSetDefault) flags.StringVar(&description, "description", "", "") flags.StringVar(&path, "path", "", "") - flags.StringVar(&defaultLeaseTTL, "default_lease_ttl", "", "") - flags.StringVar(&maxLeaseTTL, "max_lease_ttl", "", "") + flags.StringVar(&defaultLeaseTTL, "default-lease-ttl", "", "") + flags.StringVar(&maxLeaseTTL, "max-lease-ttl", "", "") flags.Usage = func() { c.Ui.Error(c.Help()) } if err := flags.Parse(args); err != nil { return 1 @@ -51,31 +51,26 @@ func (c *MountCommand) Run(args []string) int { mountInfo := &api.Mount{ Type: mountType, Description: description, - Config: &vault.MountConfig{}, + Config: vault.MountConfig{}, } - var passConfig bool if defaultLeaseTTL != "" { - mountInfo.Config.DefaultLeaseTTL, err = time.ParseDuration(defaultLeaseTTL) + defTTL, err := time.ParseDuration(defaultLeaseTTL) if err != nil { c.Ui.Error(fmt.Sprintf( "Error parsing default lease TTL duration: %s", err)) return 2 } - passConfig = true + mountInfo.Config.DefaultLeaseTTL = &defTTL } if maxLeaseTTL != "" { - mountInfo.Config.MaxLeaseTTL, err = time.ParseDuration(maxLeaseTTL) + maxTTL, err := time.ParseDuration(maxLeaseTTL) if err != nil { c.Ui.Error(fmt.Sprintf( "Error parsing max lease TTL duration: %s", err)) return 2 } - passConfig = true - } - - if !passConfig { - mountInfo.Config = nil + mountInfo.Config.MaxLeaseTTL = &maxTTL } if err := client.Sys().Mount(path, mountInfo); err != nil { @@ -110,11 +105,21 @@ General Options: Mount Options: - -description= Human-friendly description of the purpose for the - mount. This shows up in the mounts command. + -description= Human-friendly description of the purpose for + the mount. This shows up in the mounts command. - -path= Mount point for the logical backend. This defaults - to the type of the mount. + -path= Mount point for the logical backend. This + defauls to the type of the mount. + + -default-lease-ttl= Default lease time-to-live for this backend. + If not specified, uses the global default, or + the previously set value. Set to '0' to + explicitly set it to use the global default. + + -max-lease-ttl= Max lease time-to-live for this backend. + If not specified, uses the global default, or + the previously set value. Set to '0' to + explicitly set it to use the global default. ` return strings.TrimSpace(helpText) diff --git a/command/remount.go b/command/remount.go index c1480ff207..491c1075bf 100644 --- a/command/remount.go +++ b/command/remount.go @@ -17,8 +17,8 @@ type RemountCommand struct { func (c *RemountCommand) Run(args []string) int { var defaultLeaseTTL, maxLeaseTTL string flags := c.Meta.FlagSet("remount", FlagSetDefault) - flags.StringVar(&defaultLeaseTTL, "default_lease_ttl", "", "") - flags.StringVar(&maxLeaseTTL, "max_lease_ttl", "", "") + flags.StringVar(&defaultLeaseTTL, "default-lease-ttl", "", "") + flags.StringVar(&maxLeaseTTL, "max-lease-ttl", "", "") flags.Usage = func() { c.Ui.Error(c.Help()) } if err := flags.Parse(args); err != nil { return 1 @@ -35,30 +35,24 @@ func (c *RemountCommand) Run(args []string) int { from := args[0] to := args[1] - mountConfig := &vault.MountConfig{} - var err error - var passConfig bool + mountConfig := vault.MountConfig{} if defaultLeaseTTL != "" { - mountConfig.DefaultLeaseTTL, err = time.ParseDuration(defaultLeaseTTL) + defTTL, err := time.ParseDuration(defaultLeaseTTL) if err != nil { c.Ui.Error(fmt.Sprintf( "Error parsing default lease TTL duration: %s", err)) - return 1 + return 2 } - passConfig = true + mountConfig.DefaultLeaseTTL = &defTTL } if maxLeaseTTL != "" { - mountConfig.MaxLeaseTTL, err = time.ParseDuration(maxLeaseTTL) + maxTTL, err := time.ParseDuration(maxLeaseTTL) if err != nil { c.Ui.Error(fmt.Sprintf( "Error parsing max lease TTL duration: %s", err)) - return 1 + return 2 } - passConfig = true - } - - if !passConfig { - mountConfig = nil + mountConfig.MaxLeaseTTL = &maxTTL } client, err := c.Client() @@ -95,10 +89,27 @@ Usage: vault remount [options] from to the Vault data associated with the backend will be preserved (such as configuration data). + If the 'from' and 'to' values of the same, performs an in-place + remount. This allows you to change mount options. + Example: vault remount secret/ generic/ General Options: - ` + generalOptionsUsage() + ` + generalOptionsUsage() + ` + +Mount Options: + + -default-lease-ttl= Default lease time-to-live for this backend. + If not specified, uses the global default, or + the previously set value. Set to '0' to + explicitly set it to use the global default. + + -max-lease-ttl= Max lease time-to-live for this backend. + If not specified, uses the global default, or + the previously set value. Set to '0' to + explicitly set it to use the global default. + +` return strings.TrimSpace(helpText) } diff --git a/logical/framework/field_data.go b/logical/framework/field_data.go index 2dec5be6ba..d88c7afafe 100644 --- a/logical/framework/field_data.go +++ b/logical/framework/field_data.go @@ -34,11 +34,11 @@ func (d *FieldData) Validate() error { case TypeBool, TypeInt, TypeMap, TypeDurationSecond, TypeString: _, _, err := d.getPrimitive(field, schema) if err != nil { - return fmt.Errorf("Error converting input %v for field %s", value, field) + return fmt.Errorf("Error converting input %v for field %s: %s", value, field, err) } default: return fmt.Errorf("unknown field type %s for field %s", - schema.Type, field) + schema.Type, field) } } diff --git a/logical/system_view.go b/logical/system_view.go index f404ea7548..f1eac8d2a5 100644 --- a/logical/system_view.go +++ b/logical/system_view.go @@ -15,14 +15,33 @@ type SystemView interface { } type StaticSystemView struct { - DefaultLeaseTTLVal *time.Duration - MaxLeaseTTLVal *time.Duration + DefaultLeaseTTLVal time.Duration + MaxLeaseTTLVal time.Duration } func (d *StaticSystemView) DefaultLeaseTTL() time.Duration { - return *d.DefaultLeaseTTLVal + return d.DefaultLeaseTTLVal } func (d *StaticSystemView) MaxLeaseTTL() time.Duration { - return *d.MaxLeaseTTLVal + return d.MaxLeaseTTLVal +} + +type DynamicSystemView struct { + DefaultLeaseTTLVal **time.Duration + MaxLeaseTTLVal **time.Duration +} + +func (d *DynamicSystemView) DefaultLeaseTTL() time.Duration { + if *d.DefaultLeaseTTLVal == nil { + return 0 + } + return **d.DefaultLeaseTTLVal +} + +func (d *DynamicSystemView) MaxLeaseTTL() time.Duration { + if *d.MaxLeaseTTLVal == nil { + return 0 + } + return **d.MaxLeaseTTLVal } diff --git a/vault/logical_system.go b/vault/logical_system.go index 625ebae109..3ae8e82cf5 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -99,6 +99,10 @@ func NewSystemBackend(core *Core) logical.Backend { Type: framework.TypeString, Description: strings.TrimSpace(sysHelp["remount_to"][0]), }, + "config": &framework.FieldSchema{ + Type: framework.TypeMap, + Description: strings.TrimSpace(sysHelp["mount_config"][0]), + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -363,23 +367,16 @@ func (b *SystemBackend) handleMount( path := data.Get("path").(string) logicalType := data.Get("type").(string) description := data.Get("description").(string) - var config *MountConfig - configInt, ok := data.GetOk("config") - if ok { - configMap, ok := configInt.(map[string]interface{}) - if !ok { + + var config MountConfig + configMap := data.Get("config").(map[string]interface{}) + if configMap != nil && len(configMap) != 0 { + err := mapstructure.Decode(configMap, &config) + if err != nil { return logical.ErrorResponse( - "cannot convert mount config information into proper values"), + "unable to convert given mount config information"), logical.ErrInvalidRequest } - if configMap != nil && len(configMap) != 0 { - err := mapstructure.Decode(configMap, config) - if err != nil { - return logical.ErrorResponse( - "unable to convert given mount config information"), - logical.ErrInvalidRequest - } - } } if logicalType == "" { @@ -393,9 +390,14 @@ func (b *SystemBackend) handleMount( Path: path, Type: logicalType, Description: description, + Config: config, } - if config != nil { - me.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 @@ -446,23 +448,16 @@ func (b *SystemBackend) handleRemount( "both 'from' and 'to' path must be specified as a string"), logical.ErrInvalidRequest } - var config *MountConfig - configInt, ok := data.GetOk("config") - if ok { - configMap, ok := configInt.(map[string]interface{}) - if !ok { - return logical.ErrorResponse( - "cannot convert mount config information into proper values"), + + var config MountConfig + configMap := data.Get("config").(map[string]interface{}) + if configMap != nil && len(configMap) != 0 { + err := mapstructure.Decode(configMap, &config) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf( + "unable to convert given mount config information: %s", err)), logical.ErrInvalidRequest } - if configMap != nil && len(configMap) != 0 { - err := mapstructure.Decode(configMap, config) - if err != nil { - return logical.ErrorResponse( - "unable to convert given mount config information"), - logical.ErrInvalidRequest - } - } } // Attempt remount diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 1190d72001..5b794a78fe 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/fatih/structs" "github.com/hashicorp/vault/audit" "github.com/hashicorp/vault/logical" ) @@ -39,21 +40,23 @@ func TestSystemBackend_mounts(t *testing.T) { t.Fatalf("err: %v", err) } + // We can't know the pointer address ahead of time so simply + // copy what's given exp := map[string]interface{}{ "secret/": map[string]interface{}{ "type": "generic", "description": "generic secret storage", "config": map[string]interface{}{ - "default_lease_ttl": time.Duration(0), - "max_lease_ttl": time.Duration(0), + "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": time.Duration(0), - "max_lease_ttl": time.Duration(0), + "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), }, }, } @@ -123,6 +126,7 @@ func TestSystemBackend_remount(t *testing.T) { req := logical.TestRequest(t, logical.WriteOperation, "remount") req.Data["from"] = "secret" req.Data["to"] = "foo" + req.Data["config"] = structs.Map(MountConfig{}) resp, err := b.HandleRequest(req) if err != nil { t.Fatalf("err: %v", err) @@ -138,6 +142,7 @@ func TestSystemBackend_remount_invalid(t *testing.T) { req := logical.TestRequest(t, logical.WriteOperation, "remount") req.Data["from"] = "unknown" req.Data["to"] = "foo" + req.Data["config"] = structs.Map(MountConfig{}) resp, err := b.HandleRequest(req) if err != logical.ErrInvalidRequest { t.Fatalf("err: %v", err) diff --git a/vault/mount.go b/vault/mount.go index b9e811ff23..e2800ca5a8 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -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"` // Override for global default - MaxLeaseTTL time.Duration `json:"max_lease_ttl" structs:"max_lease_ttl"` // Override for global default + DefaultLeaseTTL *time.Duration `json:"default_lease_ttl" structs:"default_lease_ttl"` // Override for global default + MaxLeaseTTL *time.Duration `json:"max_lease_ttl" structs:"max_lease_ttl"` // Override for global default } // Returns a deep copy of the mount entry @@ -283,7 +283,7 @@ func (c *Core) taintMountEntry(path string) error { } // Remount is used to remount a path at a new mount point. -func (c *Core) remount(src, dst string, config *MountConfig) error { +func (c *Core) remount(src, dst string, config MountConfig) error { c.mounts.Lock() defer c.mounts.Unlock() @@ -302,6 +302,11 @@ func (c *Core) remount(src, dst string, config *MountConfig) error { } } + inPlace := false + if src == dst { + inPlace = true + } + // Verify exact match of the route match := c.router.MatchingMount(src) if match == "" || src != match { @@ -309,6 +314,36 @@ func (c *Core) remount(src, dst string, config *MountConfig) error { } // Verify there is no conflicting mount + if inPlace { + for _, ent := range c.mounts.Entries { + if ent.Path == src { + if config.DefaultLeaseTTL != nil { + if *config.DefaultLeaseTTL == 0 { + ent.Config.DefaultLeaseTTL = &c.defaultLeaseTTL + } else { + ent.Config.DefaultLeaseTTL = config.DefaultLeaseTTL + } + } + if config.MaxLeaseTTL != nil { + if *config.MaxLeaseTTL == 0 { + ent.Config.MaxLeaseTTL = &c.maxLeaseTTL + } else { + ent.Config.MaxLeaseTTL = config.MaxLeaseTTL + } + } + break + } + } + + // 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: remounted '%s' in-place", src) + return nil + } + if match := c.router.MatchingMount(dst); match != "" { return fmt.Errorf("existing mount at '%s'", match) } @@ -339,8 +374,19 @@ func (c *Core) remount(src, dst string, config *MountConfig) error { if ent.Path == src { ent.Path = dst ent.Tainted = false - if config != nil { - ent.Config = *config + if config.DefaultLeaseTTL != nil { + if *config.DefaultLeaseTTL == 0 { + ent.Config.DefaultLeaseTTL = &c.defaultLeaseTTL + } else { + ent.Config.DefaultLeaseTTL = config.DefaultLeaseTTL + } + } + if config.MaxLeaseTTL != nil { + if *config.MaxLeaseTTL == 0 { + ent.Config.MaxLeaseTTL = &c.maxLeaseTTL + } else { + ent.Config.MaxLeaseTTL = config.MaxLeaseTTL + } } break } @@ -503,15 +549,17 @@ func (c *Core) MountEntrySysView(me *MountEntry) (logical.SystemView, error) { return nil, fmt.Errorf("[ERR] core: nil MountEntry when generating SystemView") } - sysView := &logical.StaticSystemView{ - DefaultLeaseTTLVal: &c.defaultLeaseTTL, - MaxLeaseTTLVal: &c.maxLeaseTTL, + sysDefaultLeaseTTLPtr := &c.defaultLeaseTTL + sysMaxLeaseTTLPtr := &c.maxLeaseTTL + sysView := &logical.DynamicSystemView{ + DefaultLeaseTTLVal: &sysDefaultLeaseTTLPtr, + MaxLeaseTTLVal: &sysMaxLeaseTTLPtr, } - if me.Config.DefaultLeaseTTL != 0 { + if me.Config.DefaultLeaseTTL != nil && *me.Config.DefaultLeaseTTL != 0 { sysView.DefaultLeaseTTLVal = &me.Config.DefaultLeaseTTL } - if me.Config.MaxLeaseTTL != 0 { + if me.Config.MaxLeaseTTL != nil && *me.Config.MaxLeaseTTL != 0 { sysView.MaxLeaseTTLVal = &me.Config.MaxLeaseTTL } @@ -545,12 +593,20 @@ 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) diff --git a/vault/mount_test.go b/vault/mount_test.go index dcded5d09a..c577611c21 100644 --- a/vault/mount_test.go +++ b/vault/mount_test.go @@ -192,7 +192,7 @@ func TestCore_Unmount_Cleanup(t *testing.T) { func TestCore_Remount(t *testing.T) { c, key, _ := TestCoreUnsealed(t) - err := c.remount("secret", "foo", nil) + err := c.remount("secret", "foo", MountConfig{}) if err != nil { t.Fatalf("err: %v", err) } @@ -280,7 +280,7 @@ func TestCore_Remount_Cleanup(t *testing.T) { } // Remount, this should cleanup - if err := c.remount("test/", "new/", nil); err != nil { + if err := c.remount("test/", "new/", MountConfig{}); err != nil { t.Fatalf("err: %v", err) } @@ -309,7 +309,7 @@ func TestCore_Remount_Cleanup(t *testing.T) { func TestCore_Remount_Protected(t *testing.T) { c, _, _ := TestCoreUnsealed(t) - err := c.remount("sys", "foo", nil) + err := c.remount("sys", "foo", MountConfig{}) if err.Error() != "cannot remount 'sys/'" { t.Fatalf("err: %v", err) } diff --git a/website/source/docs/http/sys-mounts.html.md b/website/source/docs/http/sys-mounts.html.md index 0bdb4f9cd5..b34e5c64d2 100644 --- a/website/source/docs/http/sys-mounts.html.md +++ b/website/source/docs/http/sys-mounts.html.md @@ -13,7 +13,9 @@ description: |-
Description
- Lists all the mounted secret backends. + Lists all the mounted secret backends. `default_lease_ttl` + or `max_lease_ttl` values of `0` mean that the system + defaults are used by this backend.
Method
@@ -31,12 +33,20 @@ description: |- { "aws": { "type": "aws", - "description": "AWS keys" + "description": "AWS keys", + "config": { + "default_lease_ttl": 0, + "max_lease_ttl": 0 + } }, "sys": { "type": "system", - "description": "system endpoint" + "description": "system endpoint", + "config": { + "default_lease_ttl": 0, + "max_lease_ttl": 0 + } } } ``` @@ -71,6 +81,16 @@ description: |- optional A human-friendly description of the mount. +
  • + config + optional + 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. +
    • diff --git a/website/source/docs/http/sys-remount.html.md b/website/source/docs/http/sys-remount.html.md index 231e770920..22e5141dd6 100644 --- a/website/source/docs/http/sys-remount.html.md +++ b/website/source/docs/http/sys-remount.html.md @@ -28,8 +28,21 @@ description: |-
  • to required - The new mount point. + The new mount point. This can be the same + as `from` if you simply want to change + backend configuration with the `config` + parameter.
    • +
  • + config + optional + 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. +