perf: reduce redundant bitwarden CLI calls
This commit is contained in:
parent
98bac84ab8
commit
2920f5980a
4 changed files with 89 additions and 31 deletions
|
|
@ -1537,6 +1537,7 @@ import (
|
|||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"gitea.lclr.dev/AI/mcp-framework/bootstrap"
|
||||
"gitea.lclr.dev/AI/mcp-framework/cli"
|
||||
|
|
@ -1816,14 +1817,16 @@ func (r Runtime) runConfigTest(ctx context.Context, inv bootstrap.Invocation) er
|
|||
stdout = os.Stdout
|
||||
}
|
||||
|
||||
secretStoreFactory := memoizeSecretStoreFactory(r.openSecretStore)
|
||||
|
||||
report := cli.RunDoctor(ctx, cli.DoctorOptions{
|
||||
ConfigCheck: cli.NewConfigCheck(r.ConfigStore),
|
||||
SecretStoreCheck: cli.SecretStoreAvailabilityCheck(r.openSecretStore),
|
||||
SecretStoreCheck: cli.SecretStoreAvailabilityCheck(secretStoreFactory),
|
||||
SecretBackendPolicy: r.activeBackendPolicy(),
|
||||
RequiredSecrets: []cli.DoctorSecret{
|
||||
{Name: r.SecretName, Label: "API token"},
|
||||
},
|
||||
SecretStoreFactory: r.openSecretStore,
|
||||
SecretStoreFactory: secretStoreFactory,
|
||||
ManifestCheck: r.manifestDoctorCheck(),
|
||||
BitwardenOptions: cli.BitwardenDoctorOptions{
|
||||
LookupEnv: os.LookupEnv,
|
||||
|
|
@ -1841,6 +1844,25 @@ func (r Runtime) runConfigTest(ctx context.Context, inv bootstrap.Invocation) er
|
|||
return nil
|
||||
}
|
||||
|
||||
func memoizeSecretStoreFactory(factory func() (secretstore.Store, error)) func() (secretstore.Store, error) {
|
||||
if factory == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
var (
|
||||
once sync.Once
|
||||
store secretstore.Store
|
||||
err error
|
||||
)
|
||||
|
||||
return func() (secretstore.Store, error) {
|
||||
once.Do(func() {
|
||||
store, err = factory()
|
||||
})
|
||||
return store, err
|
||||
}
|
||||
}
|
||||
|
||||
func (r Runtime) runUpdate(ctx context.Context, inv bootstrap.Invocation) error {
|
||||
stdout := inv.Stdout
|
||||
if stdout == nil {
|
||||
|
|
|
|||
|
|
@ -76,6 +76,8 @@ func TestGenerateCreatesRecommendedSkeleton(t *testing.T) {
|
|||
"ManifestSource",
|
||||
"ManifestCheck: r.manifestDoctorCheck()",
|
||||
"SecretBackendPolicy: r.activeBackendPolicy()",
|
||||
"secretStoreFactory := memoizeSecretStoreFactory(r.openSecretStore)",
|
||||
"func memoizeSecretStoreFactory(factory func() (secretstore.Store, error)) func() (secretstore.Store, error) {",
|
||||
"cli.WriteSetupSecretVerified",
|
||||
} {
|
||||
if !strings.Contains(string(appGo), snippet) {
|
||||
|
|
|
|||
|
|
@ -211,7 +211,7 @@ func detectShellFlavor(shellHint string) string {
|
|||
|
||||
func (s *bitwardenStore) SetSecret(name, label, secret string) error {
|
||||
secretName := s.scopedName(name)
|
||||
item, err := s.findItem(secretName, name)
|
||||
item, payload, err := s.findItem(secretName, name)
|
||||
switch {
|
||||
case errors.Is(err, ErrNotFound):
|
||||
template, err := s.itemTemplate()
|
||||
|
|
@ -239,11 +239,6 @@ func (s *bitwardenStore) SetSecret(name, label, secret string) error {
|
|||
return err
|
||||
}
|
||||
|
||||
payload, err := s.itemByID(item.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
setBitwardenSecretPayload(payload, s.serviceName, name, secretName, label, secret)
|
||||
encoded, err := s.encodePayload(payload)
|
||||
if err != nil {
|
||||
|
|
@ -266,12 +261,7 @@ func (s *bitwardenStore) SetSecret(name, label, secret string) error {
|
|||
|
||||
func (s *bitwardenStore) GetSecret(name string) (string, error) {
|
||||
secretName := s.scopedName(name)
|
||||
item, err := s.findItem(secretName, name)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
payload, err := s.itemByID(item.ID)
|
||||
_, payload, err := s.findItem(secretName, name)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
|
@ -286,7 +276,7 @@ func (s *bitwardenStore) GetSecret(name string) (string, error) {
|
|||
|
||||
func (s *bitwardenStore) DeleteSecret(name string) error {
|
||||
secretName := s.scopedName(name)
|
||||
item, err := s.findItem(secretName, name)
|
||||
item, _, err := s.findItem(secretName, name)
|
||||
if errors.Is(err, ErrNotFound) {
|
||||
return nil
|
||||
}
|
||||
|
|
@ -311,7 +301,12 @@ func (s *bitwardenStore) scopedName(name string) string {
|
|||
return fmt.Sprintf("%s/%s", s.serviceName, name)
|
||||
}
|
||||
|
||||
func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenListItem, error) {
|
||||
type bitwardenResolvedItem struct {
|
||||
item bitwardenListItem
|
||||
payload map[string]any
|
||||
}
|
||||
|
||||
func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenListItem, map[string]any, error) {
|
||||
output, err := s.execute(
|
||||
fmt.Sprintf("list bitwarden items for secret %q", secretName),
|
||||
nil,
|
||||
|
|
@ -321,15 +316,15 @@ func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenLi
|
|||
secretName,
|
||||
)
|
||||
if err != nil {
|
||||
return bitwardenListItem{}, err
|
||||
return bitwardenListItem{}, nil, err
|
||||
}
|
||||
if strings.TrimSpace(string(output)) == "" {
|
||||
return bitwardenListItem{}, ErrNotFound
|
||||
return bitwardenListItem{}, nil, ErrNotFound
|
||||
}
|
||||
|
||||
var items []bitwardenListItem
|
||||
if err := json.Unmarshal(output, &items); err != nil {
|
||||
return bitwardenListItem{}, fmt.Errorf("decode bitwarden items for secret %q: %w", secretName, err)
|
||||
return bitwardenListItem{}, nil, fmt.Errorf("decode bitwarden items for secret %q: %w", secretName, err)
|
||||
}
|
||||
|
||||
matches := make([]bitwardenListItem, 0, len(items))
|
||||
|
|
@ -344,42 +339,47 @@ func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenLi
|
|||
}
|
||||
|
||||
if len(matches) == 0 {
|
||||
return bitwardenListItem{}, ErrNotFound
|
||||
return bitwardenListItem{}, nil, ErrNotFound
|
||||
}
|
||||
|
||||
markedMatches := make([]bitwardenListItem, 0, len(matches))
|
||||
legacyMatches := make([]bitwardenListItem, 0, len(matches))
|
||||
markedMatches := make([]bitwardenResolvedItem, 0, len(matches))
|
||||
legacyMatches := make([]bitwardenResolvedItem, 0, len(matches))
|
||||
for _, item := range matches {
|
||||
payload, err := s.itemByID(item.ID)
|
||||
if err != nil {
|
||||
return bitwardenListItem{}, err
|
||||
return bitwardenListItem{}, nil, err
|
||||
}
|
||||
|
||||
resolved := bitwardenResolvedItem{
|
||||
item: item,
|
||||
payload: payload,
|
||||
}
|
||||
|
||||
if bitwardenItemMatchesMarkers(payload, s.serviceName, rawSecretName) {
|
||||
markedMatches = append(markedMatches, item)
|
||||
markedMatches = append(markedMatches, resolved)
|
||||
continue
|
||||
}
|
||||
legacyMatches = append(legacyMatches, item)
|
||||
legacyMatches = append(legacyMatches, resolved)
|
||||
}
|
||||
|
||||
switch len(markedMatches) {
|
||||
case 0:
|
||||
switch len(legacyMatches) {
|
||||
case 0:
|
||||
return bitwardenListItem{}, ErrNotFound
|
||||
return bitwardenListItem{}, nil, ErrNotFound
|
||||
case 1:
|
||||
return legacyMatches[0], nil
|
||||
return legacyMatches[0].item, legacyMatches[0].payload, nil
|
||||
default:
|
||||
return bitwardenListItem{}, fmt.Errorf(
|
||||
return bitwardenListItem{}, nil, fmt.Errorf(
|
||||
"multiple legacy bitwarden items match secret %q for service %q",
|
||||
secretName,
|
||||
s.serviceName,
|
||||
)
|
||||
}
|
||||
case 1:
|
||||
return markedMatches[0], nil
|
||||
return markedMatches[0].item, markedMatches[0].payload, nil
|
||||
default:
|
||||
return bitwardenListItem{}, fmt.Errorf(
|
||||
return bitwardenListItem{}, nil, fmt.Errorf(
|
||||
"multiple bitwarden items share marker for secret %q and service %q",
|
||||
secretName,
|
||||
s.serviceName,
|
||||
|
|
|
|||
|
|
@ -337,6 +337,38 @@ func TestBitwardenStoreFallsBackToSingleLegacyItemWithoutMarkers(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestBitwardenStoreGetSecretReadsSelectedItemOnlyOnce(t *testing.T) {
|
||||
withBitwardenSession(t)
|
||||
fakeCLI := newFakeBitwardenCLI("bw")
|
||||
fakeCLI.itemsByID["item-1"] = fakeBitwardenItem{
|
||||
ID: "item-1",
|
||||
Name: "email-mcp/api-token",
|
||||
Secret: "secret-v1",
|
||||
MarkerService: "email-mcp",
|
||||
MarkerSecretName: "api-token",
|
||||
}
|
||||
withBitwardenRunner(t, fakeCLI.run)
|
||||
|
||||
store, err := Open(Options{
|
||||
ServiceName: "email-mcp",
|
||||
BackendPolicy: BackendBitwardenCLI,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Open returned error: %v", err)
|
||||
}
|
||||
|
||||
value, err := store.GetSecret("api-token")
|
||||
if err != nil {
|
||||
t.Fatalf("GetSecret returned error: %v", err)
|
||||
}
|
||||
if value != "secret-v1" {
|
||||
t.Fatalf("GetSecret = %q, want secret-v1", value)
|
||||
}
|
||||
if fakeCLI.getItemCalls != 1 {
|
||||
t.Fatalf("bw get item count = %d, want 1", fakeCLI.getItemCalls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBitwardenFindItemTreatsEmptyListOutputAsNotFound(t *testing.T) {
|
||||
store := &bitwardenStore{command: "bw", serviceName: "email-mcp"}
|
||||
withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) {
|
||||
|
|
@ -346,7 +378,7 @@ func TestBitwardenFindItemTreatsEmptyListOutputAsNotFound(t *testing.T) {
|
|||
return nil, fmt.Errorf("unexpected args: %v", args)
|
||||
})
|
||||
|
||||
_, err := store.findItem("email-mcp/api-token", "api-token")
|
||||
_, _, err := store.findItem("email-mcp/api-token", "api-token")
|
||||
if !errors.Is(err, ErrNotFound) {
|
||||
t.Fatalf("error = %v, want ErrNotFound", err)
|
||||
}
|
||||
|
|
@ -527,6 +559,7 @@ type fakeBitwardenCLI struct {
|
|||
status string
|
||||
versionChecked bool
|
||||
statusChecked bool
|
||||
getItemCalls int
|
||||
}
|
||||
|
||||
type fakeBitwardenItem struct {
|
||||
|
|
@ -568,6 +601,7 @@ func (f *fakeBitwardenCLI) run(command string, stdin []byte, args ...string) ([]
|
|||
case len(args) == 4 && args[0] == "list" && args[1] == "items" && args[2] == "--search":
|
||||
return f.handleListItems(args[3])
|
||||
case len(args) == 3 && args[0] == "get" && args[1] == "item":
|
||||
f.getItemCalls++
|
||||
return f.handleGetItem(args[2])
|
||||
case len(args) == 3 && args[0] == "get" && args[1] == "template" && args[2] == "item":
|
||||
return []byte(`{"type":2,"name":"","notes":"","secureNote":{"type":0},"fields":[]}`), nil
|
||||
|
|
|
|||
Loading…
Reference in a new issue