luxolus/open-consul
2
0
Fork 0
Code Issues 1 Pull requests Packages Releases Wiki Activity

making ACLToken.ExpirationTime a *time.Time value instead of time.Time (#5663)

Browse source 
This is mainly to avoid having the API return "0001-01-01T00:00:00Z" as
a value for the ExpirationTime field when it is not set. Unfortunately
time.Time doesn't respect the json marshalling "omitempty" directive.
This commit is contained in:
R.B. Boyer 2019-04-15 13:35:55 -05:00 committed by R.B. Boyer
parent b3956e511c
commit f43bc981e9
7 changed files with 59 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
agent/consul/acl_endpoint.go
View file

@ -358,16 +358,16 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs.
if token.ExpirationTTL < 0 {
return fmt.Errorf("Token Expiration TTL '%s' should be > 0", token.ExpirationTTL)
}
if !token.ExpirationTime.IsZero() {
if token.HasExpirationTime() {
return fmt.Errorf("Token Expiration TTL and Expiration Time cannot both be set")
}
token.ExpirationTime = token.CreateTime.Add(token.ExpirationTTL)
token.ExpirationTime = timePointer(token.CreateTime.Add(token.ExpirationTTL))
token.ExpirationTTL = 0
}
if !token.ExpirationTime.IsZero() {
if token.CreateTime.After(token.ExpirationTime) {
if token.HasExpirationTime() {
if token.CreateTime.After(*token.ExpirationTime) {
return fmt.Errorf("ExpirationTime cannot be before CreateTime")
}
@ -417,7 +417,15 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs.
return fmt.Errorf("cannot toggle local mode of %s", token.AccessorID)
}
if token.ExpirationTTL != 0 || !token.ExpirationTime.Equal(existing.ExpirationTime) {
if token.ExpirationTTL != 0 {
return fmt.Errorf("Cannot change expiration time of %s", token.AccessorID)
}
if !token.HasExpirationTime() {
token.ExpirationTime = existing.ExpirationTime
} else if !existing.HasExpirationTime() {
return fmt.Errorf("Cannot change expiration time of %s", token.AccessorID)
} else if !token.ExpirationTime.Equal(*existing.ExpirationTime) {
return fmt.Errorf("Cannot change expiration time of %s", token.AccessorID)
}
@ -1041,3 +1049,7 @@ func (a *ACL) ReplicationStatus(args *structs.DCSpecificRequest,
a.srv.aclReplicationStatusLock.RUnlock()
return nil
}
func timePointer(t time.Time) *time.Time {
return &t
}

28
agent/consul/acl_endpoint_test.go
View file

@ -1054,7 +1054,7 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
Description: "foobar",
Policies: nil,
Local: false,
ExpirationTime: time.Now().Add(test.offset),
ExpirationTime: timePointer(time.Now().Add(test.offset)),
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
@ -1099,7 +1099,7 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
Description: "foobar",
Policies: nil,
Local: false,
ExpirationTime: time.Now().Add(4 * time.Second),
ExpirationTime: timePointer(time.Now().Add(4 * time.Second)),
ExpirationTTL: 4 * time.Second,
},
WriteRequest: structs.WriteRequest{Token: "root"},
@ -1138,7 +1138,7 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
require.NotNil(t, token.AccessorID)
require.Equal(t, token.Description, "foobar")
require.Equal(t, token.AccessorID, resp.AccessorID)
requireTimeEquals(t, expectExpTime, resp.ExpirationTime)
requireTimeEquals(t, &expectExpTime, resp.ExpirationTime)
tokenID = token.AccessorID
})
@ -1152,7 +1152,7 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
Description: "foobar",
Policies: nil,
Local: false,
ExpirationTime: expTime,
ExpirationTime: &expTime,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
@ -1170,7 +1170,7 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
require.NotNil(t, token.AccessorID)
require.Equal(t, token.Description, "foobar")
require.Equal(t, token.AccessorID, resp.AccessorID)
requireTimeEquals(t, expTime, resp.ExpirationTime)
requireTimeEquals(t, &expTime, resp.ExpirationTime)
tokenID = token.AccessorID
})
@ -1183,7 +1183,7 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
ACLToken: structs.ACLToken{
Description: "new-description",
AccessorID: tokenID,
ExpirationTime: expTime.Add(-1 * time.Second),
ExpirationTime: timePointer(expTime.Add(-1 * time.Second)),
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
@ -1202,7 +1202,7 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
ACLToken: structs.ACLToken{
Description: "new-description",
AccessorID: tokenID,
ExpirationTime: expTime,
ExpirationTime: &expTime,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
@ -1220,7 +1220,7 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
require.NotNil(t, token.AccessorID)
require.Equal(t, token.Description, "new-description")
require.Equal(t, token.AccessorID, resp.AccessorID)
requireTimeEquals(t, expTime, resp.ExpirationTime)
requireTimeEquals(t, &expTime, resp.ExpirationTime)
})
t.Run("cannot update a token that is past its expiration time", func(t *testing.T) {
@ -2217,10 +2217,16 @@ func retrieveTestPolicy(codec rpc.ClientCodec, masterToken string, datacenter st
return &out, nil
}
func requireTimeEquals(t *testing.T, expect, got time.Time) {
func requireTimeEquals(t *testing.T, expect, got *time.Time) {
t.Helper()
if !expect.Equal(got) {
t.Fatalf("expected=%q != got=%q", expect, got)
if expect == nil && got == nil {
return
} else if expect == nil && got != nil {
t.Fatalf("expected=NIL != got=%q", *got)
} else if expect != nil && got == nil {
t.Fatalf("expected=%q != got=NIL", *expect)
} else if !expect.Equal(*got) {
t.Fatalf("expected=%q != got=%q", *expect, *got)
}
}

11
agent/consul/state/acl.go
View file

@ -79,14 +79,14 @@ func (s *TokenExpirationIndex) FromObject(obj interface{}) (bool, []byte, error)
if s.LocalFilter != token.Local {
return false, nil, nil
}
if token.ExpirationTime.IsZero() {
if !token.HasExpirationTime() {
return false, nil, nil
}
if token.ExpirationTime.Unix() < 0 {
return false, nil, fmt.Errorf("token expiration time cannot be before the unix epoch: %s", token.ExpirationTime)
}
buf := s.encodeTime(token.ExpirationTime)
buf := s.encodeTime(*token.ExpirationTime)
return true, buf, nil
}
@ -669,10 +669,10 @@ func (s *Store) ACLTokenMinExpirationTime(local bool) (time.Time, error) {
token := item.(*structs.ACLToken)
return token.ExpirationTime, nil
return *token.ExpirationTime, nil
}
// ACLTokenListExpires lists tokens that are expires as of the provided time.
// ACLTokenListExpires lists tokens that are expired as of the provided time.
// The returned set will be no larger than the max value provided.
func (s *Store) ACLTokenListExpired(local bool, asOf time.Time, max int) (structs.ACLTokens, <-chan struct{}, error) {
tx := s.db.Txn(false)
@ -689,8 +689,7 @@ func (s *Store) ACLTokenListExpired(local bool, asOf time.Time, max int) (struct
)
for raw := iter.Next(); raw != nil; raw = iter.Next() {
token := raw.(*structs.ACLToken)
if !token.ExpirationTime.Before(asOf) {
if token.ExpirationTime != nil && !token.ExpirationTime.Before(asOf) {
return tokens, nil, nil
}

2
agent/consul/state/acl_test.go
View file

@ -1741,7 +1741,7 @@ func TestTokenPoliciesIndex(t *testing.T) {
SecretID: newUUID(),
Description: desc,
Local: local,
ExpirationTime: expTime,
ExpirationTime: &expTime,
CreateTime: baseTime,
RaftIndex: structs.RaftIndex{
CreateIndex: 9,

16
agent/structs/acl.go
View file

@ -218,7 +218,11 @@ type ACLToken struct {
// ExpirationTime represents the point after which a token should be
// considered revoked and is eligible for destruction. The zero value
// represents NO expiration.
ExpirationTime time.Time `json:",omitempty"`
//
// This is a pointer value so that the zero value is omitted properly
// during json serialization. time.Time does not respect json omitempty
// directives unfortunately.
ExpirationTime *time.Time `json:",omitempty"`
// ExpirationTTL is a convenience field for helping set ExpirationTime to a
// value of CreateTime+ExpirationTTL. This can only be set during
@ -293,17 +297,21 @@ func (t *ACLToken) ServiceIdentityList() []*ACLServiceIdentity {
}
func (t *ACLToken) IsExpired(asOf time.Time) bool {
if asOf.IsZero() || t.ExpirationTime.IsZero() {
if asOf.IsZero() || !t.HasExpirationTime() {
return false
}
return t.ExpirationTime.Before(asOf)
}
func (t *ACLToken) HasExpirationTime() bool {
return t.ExpirationTime != nil && !t.ExpirationTime.IsZero()
}
func (t *ACLToken) UsesNonLegacyFields() bool {
return len(t.Policies) > 0 ||
len(t.ServiceIdentities) > 0 ||
t.Type == "" ||
!t.ExpirationTime.IsZero() ||
t.HasExpirationTime() ||
t.ExpirationTTL != 0
}
@ -405,7 +413,7 @@ type ACLTokenListStub struct {
Policies []ACLTokenPolicyLink `json:",omitempty"`
ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
Local bool
ExpirationTime time.Time `json:",omitempty"`
ExpirationTime *time.Time `json:",omitempty"`
CreateTime time.Time `json:",omitempty"`
Hash []byte
CreateIndex uint64

4
api/acl.go
View file

@ -31,7 +31,7 @@ type ACLToken struct {
ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
Local bool
ExpirationTTL time.Duration `json:",omitempty"`
ExpirationTime time.Time `json:",omitempty"`
ExpirationTime *time.Time `json:",omitempty"`
CreateTime time.Time `json:",omitempty"`
Hash []byte `json:",omitempty"`
@ -48,7 +48,7 @@ type ACLTokenListEntry struct {
Policies []*ACLTokenPolicyLink `json:",omitempty"`
ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
Local bool
ExpirationTime time.Time `json:",omitempty"`
ExpirationTime *time.Time `json:",omitempty"`
CreateTime time.Time
Hash []byte
Legacy bool

8
command/acl/acl_helpers.go
View file

@ -15,8 +15,8 @@ func PrintToken(token *api.ACLToken, ui cli.Ui, showMeta bool) {
ui.Info(fmt.Sprintf("Description: %s", token.Description))
ui.Info(fmt.Sprintf("Local: %t", token.Local))
ui.Info(fmt.Sprintf("Create Time: %v", token.CreateTime))
if !token.ExpirationTime.IsZero() {
ui.Info(fmt.Sprintf("Expiration Time: %v", token.ExpirationTime))
if token.ExpirationTime != nil && !token.ExpirationTime.IsZero() {
ui.Info(fmt.Sprintf("Expiration Time: %v", *token.ExpirationTime))
}
if showMeta {
ui.Info(fmt.Sprintf("Hash: %x", token.Hash))
@ -46,8 +46,8 @@ func PrintTokenListEntry(token *api.ACLTokenListEntry, ui cli.Ui, showMeta bool)
ui.Info(fmt.Sprintf("Description: %s", token.Description))
ui.Info(fmt.Sprintf("Local: %t", token.Local))
ui.Info(fmt.Sprintf("Create Time: %v", token.CreateTime))
if !token.ExpirationTime.IsZero() {
ui.Info(fmt.Sprintf("Expiration Time: %v", token.ExpirationTime))
if token.ExpirationTime != nil && !token.ExpirationTime.IsZero() {
ui.Info(fmt.Sprintf("Expiration Time: %v", *token.ExpirationTime))
}
ui.Info(fmt.Sprintf("Legacy: %t", token.Legacy))
if showMeta {