From e69df0e947318bb0cde5bf855cab2b4f06690256 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 16 Mar 2015 13:29:51 -0700 Subject: [PATCH] all: Removing fields from Lease --- api/secret.go | 9 +++-- api/secret_test.go | 14 ++++---- http/logical.go | 10 +++--- http/logical_test.go | 7 ++-- logical/request.go | 1 + logical/response.go | 20 ++++------- vault/expiration.go | 56 ++++++++++++++++++------------- vault/expiration_test.go | 7 ++-- vault/logical_passthrough.go | 7 ++-- vault/logical_passthrough_test.go | 7 ++-- 10 files changed, 64 insertions(+), 74 deletions(-) diff --git a/api/secret.go b/api/secret.go index 774a0e6e90..152c82650e 100644 --- a/api/secret.go +++ b/api/secret.go @@ -7,11 +7,10 @@ import ( // Secret is the structure returned for every secret within Vault. type Secret struct { - VaultId string `json:"vault_id"` - Renewable bool - LeaseDuration int `json:"lease_duration"` - LeaseDurationMax int `json:"lease_duration_max"` - Data map[string]interface{} `json:"data"` + VaultId string `json:"vault_id"` + Renewable bool + LeaseDuration int `json:"lease_duration"` + Data map[string]interface{} `json:"data"` } // ParseSecret is used to parse a secret value from JSON from an io.Reader. diff --git a/api/secret_test.go b/api/secret_test.go index 92767316ac..a27856af4f 100644 --- a/api/secret_test.go +++ b/api/secret_test.go @@ -12,8 +12,9 @@ func TestParseSecret(t *testing.T) { "vault_id": "foo", "renewable": true, "lease_duration": 10, - "lease_duration_max": 100, - "key": "value" + "data": { + "key": "value" + } }`) secret, err := ParseSecret(strings.NewReader(raw)) @@ -22,15 +23,14 @@ func TestParseSecret(t *testing.T) { } expected := &Secret{ - VaultId: "foo", - Renewable: true, - LeaseDuration: 10, - LeaseDurationMax: 100, + VaultId: "foo", + Renewable: true, + LeaseDuration: 10, Data: map[string]interface{}{ "key": "value", }, } if !reflect.DeepEqual(secret, expected) { - t.Fatalf("bad: %#v", secret) + t.Fatalf("bad: %#v %#v", secret, expected) } } diff --git a/http/logical.go b/http/logical.go index 41cf3c0d3b..da42b9f25f 100644 --- a/http/logical.go +++ b/http/logical.go @@ -64,7 +64,6 @@ func handleLogical(core *vault.Core) http.Handler { logicalResp.VaultId = resp.Lease.VaultID logicalResp.Renewable = resp.Lease.Renewable logicalResp.LeaseDuration = int(resp.Lease.Duration.Seconds()) - logicalResp.LeaseDurationMax = int(resp.Lease.MaxDuration.Seconds()) } httpResp = logicalResp @@ -76,9 +75,8 @@ func handleLogical(core *vault.Core) http.Handler { } type LogicalResponse struct { - VaultId string `json:"vault_id"` - Renewable bool `json:"renewable"` - LeaseDuration int `json:"lease_duration"` - LeaseDurationMax int `json:"lease_duration_max"` - Data map[string]interface{} `json:"data"` + VaultId string `json:"vault_id"` + Renewable bool `json:"renewable"` + LeaseDuration int `json:"lease_duration"` + Data map[string]interface{} `json:"data"` } diff --git a/http/logical_test.go b/http/logical_test.go index 7a10570ae7..0be1ff58ba 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -25,10 +25,9 @@ func TestLogical(t *testing.T) { var actual map[string]interface{} expected := map[string]interface{}{ - "vault_id": "", - "renewable": false, - "lease_duration": float64(0), - "lease_duration_max": float64(0), + "vault_id": "", + "renewable": false, + "lease_duration": float64(0), "data": map[string]interface{}{ "data": "bar", }, diff --git a/logical/request.go b/logical/request.go index 4832f5d438..17635752ad 100644 --- a/logical/request.go +++ b/logical/request.go @@ -49,6 +49,7 @@ const ( DeleteOperation = "delete" ListOperation = "list" RevokeOperation = "revoke" + RenewOperation = "renew" HelpOperation = "help" ) diff --git a/logical/response.go b/logical/response.go index 682374efae..ebef4228a6 100644 --- a/logical/response.go +++ b/logical/response.go @@ -21,12 +21,10 @@ type Response struct { // Lease is used to provide more information about the lease type Lease struct { - VaultID string // VaultID is the unique identifier used for renewal and revocation - Renewable bool // Is the VaultID renewable - Revokable bool // Is the secret revokable. Must support 'Revoke' operation. - Duration time.Duration // Current lease duration - MaxDuration time.Duration // Maximum lease duration - MaxIncrement time.Duration // Maximum increment to lease duration + VaultID string // VaultID is the unique identifier used for renewal and revocation + Renewable bool // Is the VaultID renewable + Duration time.Duration // Current lease duration + GracePeriod time.Duration // Lease revocation grace period (Duration+GracePeriod=RevokePeriod) } // Validate is used to sanity check a lease @@ -34,14 +32,8 @@ func (l *Lease) Validate() error { if l.Duration <= 0 { return fmt.Errorf("lease duration must be greater than zero") } - if l.MaxDuration <= 0 { - return fmt.Errorf("maximum lease duration must be greater than zero") - } - if l.Duration > l.MaxDuration { - return fmt.Errorf("lease duration cannot be greater than maximum lease duration") - } - if l.MaxIncrement < 0 { - return fmt.Errorf("maximum lease increment cannot be negative") + if l.GracePeriod < 0 { + return fmt.Errorf("grace period cannot be less than zero") } return nil } diff --git a/vault/expiration.go b/vault/expiration.go index 31cb011843..a4aa18a551 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -206,7 +206,7 @@ func (m *ExpirationManager) RevokePrefix(prefix string) error { // Renew is used to renew a secret using the given vaultID // and a renew interval. The increment may be ignored. -func (m *ExpirationManager) Renew(vaultID string, increment time.Duration) (*logical.Lease, error) { +func (m *ExpirationManager) Renew(vaultID string, increment time.Duration) (*logical.Response, error) { // Load the entry le, err := m.loadEntry(vaultID) if err != nil { @@ -218,31 +218,31 @@ func (m *ExpirationManager) Renew(vaultID string, increment time.Duration) (*log return nil, fmt.Errorf("lease not found") } - // Determine the remaining lease time - end := le.IssueTime.Add(le.Lease.MaxDuration) - now := time.Now().UTC() - remain := end.Sub(now) - if remain < 0 { + // Determine if the lease is expired + if le.ExpireTime.Before(time.Now().UTC()) { return nil, fmt.Errorf("lease expired") } - // Set the default increment if none provided - if increment <= 0 { - increment = le.Lease.Duration + // Attempt to renew the entry + resp, err := m.renewEntry(le) + if err != nil { + return nil, err } - // Determine the maximum lease renew - if increment > le.Lease.MaxIncrement && le.Lease.MaxIncrement != 0 { - increment = le.Lease.MaxIncrement + // Fast-path if there is no lease + if resp == nil || resp.Lease == nil || !resp.IsSecret { + return resp, nil } - // Restrict the increment to avoid exceeding maximum duration - if increment > remain { - increment = remain + // Validate the lease + if err := resp.Lease.Validate(); err != nil { + return nil, err } // Update the lease entry - le.ExpireTime = now.Add(increment) + le.Data = resp.Data + le.Lease = resp.Lease + le.ExpireTime = time.Now().UTC().Add(resp.Lease.Duration) if err := m.persistEntry(le); err != nil { return nil, err } @@ -250,16 +250,12 @@ func (m *ExpirationManager) Renew(vaultID string, increment time.Duration) (*log // Update the expiration time m.pendingLock.Lock() if timer, ok := m.pending[vaultID]; ok { - timer.Reset(increment) + timer.Reset(resp.Lease.Duration) } m.pendingLock.Unlock() - // Return a new lease with updated durations - lease := new(logical.Lease) - *lease = *le.Lease - lease.Duration = increment - lease.MaxDuration = remain - return lease, nil + // Return the response + return resp, nil } // Register is used to take a request and response with an associated @@ -341,6 +337,20 @@ func (m *ExpirationManager) revokeEntry(le *leaseEntry) error { return nil } +// renewEntry is used to attempt renew of an internal entry +func (m *ExpirationManager) renewEntry(le *leaseEntry) (*logical.Response, error) { + req := &logical.Request{ + Operation: logical.RenewOperation, + Path: le.Path, + Data: le.Data, + } + resp, err := m.router.Route(req) + if err != nil { + return nil, fmt.Errorf("failed to renew entry: %v", err) + } + return resp, nil +} + // loadEntry is used to read a lease entry func (m *ExpirationManager) loadEntry(vaultID string) (*leaseEntry, error) { out, err := m.view.Get(vaultID) diff --git a/vault/expiration_test.go b/vault/expiration_test.go index ea48a04d9e..a13bc78475 100644 --- a/vault/expiration_test.go +++ b/vault/expiration_test.go @@ -62,8 +62,7 @@ func TestExpiration_Register(t *testing.T) { resp := &logical.Response{ IsSecret: true, Lease: &logical.Lease{ - Duration: time.Hour, - MaxDuration: time.Hour, + Duration: time.Hour, }, Data: map[string]interface{}{ "access_key": "xyz", @@ -93,9 +92,7 @@ func TestLeaseEntry(t *testing.T) { "testing": true, }, Lease: &logical.Lease{ - Renewable: true, - Duration: time.Minute, - MaxDuration: time.Hour, + Duration: time.Minute, }, IssueTime: time.Now(), ExpireTime: time.Now(), diff --git a/vault/logical_passthrough.go b/vault/logical_passthrough.go index a4a7dbe781..5ffda9c1c6 100644 --- a/vault/logical_passthrough.go +++ b/vault/logical_passthrough.go @@ -70,11 +70,8 @@ func (b *PassthroughBackend) handleRead( leaseDuration, err := time.ParseDuration(leaseVal) if err == nil { lease = &logical.Lease{ - Renewable: false, - Revokable: false, - Duration: leaseDuration, - MaxDuration: leaseDuration, - MaxIncrement: 0, + Renewable: false, + Duration: leaseDuration, } } } diff --git a/vault/logical_passthrough_test.go b/vault/logical_passthrough_test.go index fb1074546f..a777ea1691 100644 --- a/vault/logical_passthrough_test.go +++ b/vault/logical_passthrough_test.go @@ -60,11 +60,8 @@ func TestPassthroughBackend_Read(t *testing.T) { expected := &logical.Response{ IsSecret: true, Lease: &logical.Lease{ - Renewable: false, - Revokable: false, - Duration: time.Hour, - MaxDuration: time.Hour, - MaxIncrement: 0, + Renewable: false, + Duration: time.Hour, }, Data: map[string]interface{}{ "raw": "test",