From 0c849cf399ebbea6eb3d7a790c46a06045d09ee5 Mon Sep 17 00:00:00 2001 From: Will Da Silva Date: Wed, 16 Oct 2024 19:43:28 -0400 Subject: [PATCH] perf: Reuse 1Password client (#548) 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 --- pkg/providers/onepassword/onepassword.go | 23 +++++++++++-------- .../onepasswordconnect/onepasswordconnect.go | 14 ++++++----- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/pkg/providers/onepassword/onepassword.go b/pkg/providers/onepassword/onepassword.go index 0ec9159..9996b8e 100644 --- a/pkg/providers/onepassword/onepassword.go +++ b/pkg/providers/onepassword/onepassword.go @@ -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) diff --git a/pkg/providers/onepasswordconnect/onepasswordconnect.go b/pkg/providers/onepasswordconnect/onepasswordconnect.go index 63a05a2..be7f45b 100644 --- a/pkg/providers/onepasswordconnect/onepasswordconnect.go +++ b/pkg/providers/onepasswordconnect/onepasswordconnect.go @@ -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) }