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

Return ACLRemoteError from cache and test it correctly

This commit is contained in:
Kyle Havlovitz 2022-05-03 09:53:27 -07:00
parent f84ed5f70b
commit 3bd001fb29
3 changed files with 21 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/12885.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
acl: Fixed a bug where the ACL down policy wasn't being applied on remote errors from the primary datacenter.
```

6
agent/consul/acl.go
View File

@ -402,6 +402,9 @@ func (r *ACLResolver) resolveIdentityFromToken(token string) (structs.ACLIdentit
cacheEntry := r.cache.GetIdentity(tokenSecretCacheID(token)) cacheEntry := r.cache.GetIdentity(tokenSecretCacheID(token))
if cacheEntry != nil && cacheEntry.Age() <= r.config.ACLTokenTTL { if cacheEntry != nil && cacheEntry.Age() <= r.config.ACLTokenTTL {
metrics.IncrCounter([]string{"acl", "token", "cache_hit"}, 1) metrics.IncrCounter([]string{"acl", "token", "cache_hit"}, 1)
if cacheEntry.Error != nil && !acl.IsErrNotFound(cacheEntry.Error) {
return cacheEntry.Identity, ACLRemoteError{Err: cacheEntry.Error}
}
return cacheEntry.Identity, cacheEntry.Error return cacheEntry.Identity, cacheEntry.Error
} }
@ -416,6 +419,9 @@ func (r *ACLResolver) resolveIdentityFromToken(token string) (structs.ACLIdentit
waitForResult := cacheEntry == nil || r.config.ACLDownPolicy != "async-cache" waitForResult := cacheEntry == nil || r.config.ACLDownPolicy != "async-cache"
if !waitForResult { if !waitForResult {
// waitForResult being false requires the cacheEntry to not be nil // waitForResult being false requires the cacheEntry to not be nil
if cacheEntry.Error != nil && !acl.IsErrNotFound(cacheEntry.Error) {
return cacheEntry.Identity, ACLRemoteError{Err: cacheEntry.Error}
}
return cacheEntry.Identity, cacheEntry.Error return cacheEntry.Identity, cacheEntry.Error
} }

15
agent/consul/acl_test.go
View File

@ -1269,6 +1269,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
foundToken := rawToken.(*structs.ACLToken) foundToken := rawToken.(*structs.ACLToken)
secretID := foundToken.SecretID secretID := foundToken.SecretID
remoteErr := fmt.Errorf("network error")
delegate := &ACLResolverTestDelegate{ delegate := &ACLResolverTestDelegate{
enabled: true, enabled: true,
datacenter: "dc1", datacenter: "dc1",
@ -1276,11 +1277,11 @@ func TestACLResolver_DownPolicy(t *testing.T) {
localTokens: false, localTokens: false,
localPolicies: false, localPolicies: false,
tokenReadFn: func(_ *structs.ACLTokenGetRequest, reply *structs.ACLTokenResponse) error { tokenReadFn: func(_ *structs.ACLTokenGetRequest, reply *structs.ACLTokenResponse) error {
return acl.ErrPermissionDenied return remoteErr
}, },
} }
r := newTestACLResolver(t, delegate, func(config *ACLResolverConfig) { r := newTestACLResolver(t, delegate, func(config *ACLResolverConfig) {
config.Config.ACLDownPolicy = "extend-cache" config.Config.ACLDownPolicy = "deny"
}) })
// Attempt to resolve the token - this should set up the cache entry with a nil // Attempt to resolve the token - this should set up the cache entry with a nil
@ -1290,7 +1291,15 @@ func TestACLResolver_DownPolicy(t *testing.T) {
require.NotNil(t, authz) require.NotNil(t, authz)
entry := r.cache.GetIdentity(tokenSecretCacheID(secretID)) entry := r.cache.GetIdentity(tokenSecretCacheID(secretID))
require.Nil(t, entry.Identity) require.Nil(t, entry.Identity)
require.Equal(t, acl.ErrPermissionDenied, entry.Error) require.Equal(t, remoteErr, entry.Error)
// Attempt to resolve again to pull from the cache.
authz, err = r.ResolveToken(secretID)
require.NoError(t, err)
require.NotNil(t, authz)
entry = r.cache.GetIdentity(tokenSecretCacheID(secretID))
require.Nil(t, entry.Identity)
require.Equal(t, remoteErr, entry.Error)
}) })
t.Run("PolicyResolve-TokenNotFound", func(t *testing.T) { t.Run("PolicyResolve-TokenNotFound", func(t *testing.T) {