Skip to content

Commit

Permalink
Allow only 1 call to RegisterResolveCallback, fix cache usage
Browse files Browse the repository at this point in the history
  • Loading branch information
dustmop committed Dec 18, 2023
1 parent 3c76359 commit 2f7c012
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 13 deletions.
3 changes: 0 additions & 3 deletions comp/core/secrets/secretsimpl/fetch_secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,6 @@ func (r *secretResolver) fetchSecret(secretsHandle []string) (map[string]string,
if v.Value == "" {
return nil, fmt.Errorf("resolved secret for '%s' is empty", sec)
}

// add it to the cache
r.cache[sec] = v.Value
res[sec] = v.Value
}
return res, nil
Expand Down
10 changes: 4 additions & 6 deletions comp/core/secrets/secretsimpl/fetch_secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ func TestFetchSecret(t *testing.T) {
// some dummy value to check the cache is not purge
resolver.cache["test"] = "yes"
resolver.commandHookFunc = func(string) ([]byte, error) {
res := []byte("{\"handle1\":{\"value\":\"p1\"},")
res = append(res, []byte("\"handle2\":{\"value\":\"p2\"},")...)
res = append(res, []byte("\"handle3\":{\"value\":\"p3\"}}")...)
res := []byte(`{"handle1":{"value":"p1"},
"handle2":{"value":"p2"},
"handle3":{"value":"p3"}}`)
return res, nil
}
resp, err := resolver.fetchSecret(secrets)
Expand All @@ -249,9 +249,7 @@ func TestFetchSecret(t *testing.T) {
"handle2": "p2",
}, resp)
assert.Equal(t, map[string]string{
"test": "yes",
"handle1": "p1",
"handle2": "p2",
"test": "yes",
}, resolver.cache)
}

Expand Down
12 changes: 8 additions & 4 deletions comp/core/secrets/secretsimpl/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ func (r *secretResolver) RegisterResolveCallback(data []byte, origin string, cb
r.lock.Lock()
defer r.lock.Unlock()

if r.refreshData != nil {
return fmt.Errorf("can only register a callback once per secret resolver")
}

// enable refreshing data by storing required parameters
r.refreshData = data
r.refreshOrigin = origin
Expand Down Expand Up @@ -318,16 +322,16 @@ func (r *secretResolver) resolve(data []byte, origin string, notifyCb secrets.Re
if r.fetchHookFunc != nil {
// hook used only for tests
secrets, err = r.fetchHookFunc(newHandles)
// add results to the cache
for handle, secretValue := range secrets {
r.cache[handle] = secretValue
}
} else {
secrets, err = r.fetchSecret(newHandles)
}
if err != nil {
return nil, err
}
// add results to the cache
for handle, secretValue := range secrets {
r.cache[handle] = secretValue
}

w.resolver = func(yamlPath []string, value string) (string, error) {
if ok, handle := isEnc(value); ok {
Expand Down
27 changes: 27 additions & 0 deletions comp/core/secrets/secretsimpl/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,3 +511,30 @@ func TestResolveThenRefresh(t *testing.T) {
assert.Equal(t, testConfNestedOriginMultiple, resolver.origin)
assert.Equal(t, []string{"some/encoded/third_level", "top_level"}, keysResolved)
}

func TestResolveCantRegisterTwice(t *testing.T) {
resolver := newEnabledSecretResolver()
resolver.backendCommand = "some_command"
resolver.cache = map[string]string{}
resolver.fetchHookFunc = func(secrets []string) (map[string]string, error) {
return map[string]string{
"pass1": "password1",
"pass2": "password2",
}, nil
}

err := resolver.RegisterResolveCallback(
testConf,
"test",
func(yamlPath []string, value any) bool { return true },
)
require.NoError(t, err)

err = resolver.RegisterResolveCallback(
testConf,
"test",
func(yamlPath []string, value any) bool { return true },
)
require.Error(t, err)
assert.Equal(t, err.Error(), "can only register a callback once per secret resolver")
}

0 comments on commit 2f7c012

Please sign in to comment.