Skip to content

Commit

Permalink
perf: Reuse 1Password client (#548)
Browse files Browse the repository at this point in the history
I noticed that the 1Password providers were very slow, and the amount of time
it took to evaluate a YAML file with 1Password references increased
approximately linearly with the number of 1Password references, which suggested
to me that work was being unnecessarily repeated.

I found that the providers have a `client` field, and its assigned to, but we
never check if its set, so we always create a new client. This PR updates the
providers so that if the `client` is not set one will be created, and if one
has been set then it'll be reused.

For my YAML file with 26 1Password references, this change reduced the time it
took to evaluate the file from 29 seconds to 11 seconds. There is still room
for improvement (e.g. by parallelizing the evaluation of each ref, improved
caching, etc.), but this seems like a simple and worthwhile change on its own.

Signed-off-by: Will Da Silva <[email protected]>
  • Loading branch information
WillDaSilva authored Oct 16, 2024
1 parent 4891c14 commit 0c849cf
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 16 deletions.
23 changes: 13 additions & 10 deletions pkg/providers/onepassword/onepassword.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,21 @@ func (p *provider) GetString(key string) (string, error) {
var err error

ctx := context.Background()
token := os.Getenv("OP_SERVICE_ACCOUNT_TOKEN")

client, err := onepassword.NewClient(
ctx,
onepassword.WithServiceAccountToken(token),
onepassword.WithIntegrationInfo("Vals op integration", "v1.0.0"),
)
if err != nil {
return "", fmt.Errorf("storage.NewClient: %v", err)
}
if p.client == nil {
token := os.Getenv("OP_SERVICE_ACCOUNT_TOKEN")

p.client = client
client, err := onepassword.NewClient(
ctx,
onepassword.WithServiceAccountToken(token),
onepassword.WithIntegrationInfo("Vals op integration", "v1.0.0"),
)
if err != nil {
return "", fmt.Errorf("storage.NewClient: %v", err)
}

p.client = client
}

prefixedKey := fmt.Sprintf("op://%s", key)
item, err := p.client.Secrets.Resolve(ctx, prefixedKey)
Expand Down
14 changes: 8 additions & 6 deletions pkg/providers/onepasswordconnect/onepasswordconnect.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,16 @@ func (p *provider) GetString(key string) (string, error) {
return "", fmt.Errorf("invalid URI: %v", errors.New("vault or item missing"))
}

client, err := connect.NewClientFromEnvironment()
if err != nil {
return "", fmt.Errorf("storage.NewClient: %v", err)
}
if p.client == nil {
client, err := connect.NewClientFromEnvironment()
if err != nil {
return "", fmt.Errorf("storage.NewClient: %v", err)
}

p.client = client
p.client = client
}

item, err := client.GetItem(splits[1], splits[0])
item, err := p.client.GetItem(splits[1], splits[0])
if err != nil {
return "", fmt.Errorf("error retrieving item: %v", err)
}
Expand Down

0 comments on commit 0c849cf

Please sign in to comment.