Skip to content

feat: KapeTool memory — support external user-managed database URL - #143

Merged
dzungtr merged 2 commits into
mainfrom
feat/12-kapetool-external-db
Jul 19, 2026
Merged

dzungtr merged 2 commits into
mainfrom
feat/12-kapetool-external-db

Conversation

@dzungtr

@dzungtr dzungtr commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds ExternalMemorySpec type with URL (required) and SecretRef (optional) fields to operator/infra/api/v1alpha1/kapetool_types.go
  • Adds External *ExternalMemorySpec field to MemorySpec (json:"external,omitempty")
  • Reconciler branches in reconcileMemory: when external is set, skips Qdrant StatefulSet/Service/PVC provisioning and publishes the external URL into status.QdrantEndpoint; when external is nil, existing provisioning path is unchanged
  • zz_generated.deepcopy.go and crds/kape.io_kapetools.yaml regenerated via make generate
  • CRD reference doc updated with external field description and example YAML

Test plan

  • TestToolReconciler_ExternalMemory_SetsReadyWithExternalURL — skips StatefulSet, sets QdrantEndpoint from URL, Ready=True/ExternalDatabase
  • TestToolReconciler_ExternalMemory_WithSecretRef_SetsReady — same with SecretRef present
  • TestToolReconciler_ProvisionedMemory_ExternalNil_StillProvisions — confirms provisioning path unchanged when external is nil
  • All existing operator tests pass (go test ./operator/...)

Closes #12

🤖 Generated with Claude Code

Adds an optional `external` field to `MemorySpec` so users can point a
KapeTool at a pre-existing Qdrant instance instead of having KAPE
provision one. When `spec.memory.external` is set, the reconciler skips
StatefulSet/Service/PVC provisioning and publishes the external URL
directly into `status.QdrantEndpoint` (which the TOML renderer already
reads for handler pod wiring). The provisioning path is unchanged when
`external` is absent.

Closes #12

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dzungtr
dzungtr force-pushed the feat/12-kapetool-external-db branch from 606fefd to 21e9b13 Compare July 19, 2026 13:17
@dzungtr

dzungtr commented Jul 19, 2026

Copy link
Copy Markdown
Owner Author

Review — requesting changes (admin cannot use the request-changes API on own PR, so posting findings here)

Thanks for this — the overall structure is clean and tests pass locally (go build, go vet, go test ./operator/... all green after rebasing onto current main). Two findings against issue #12's acceptance criteria before this can merge:

Finding 1 — external.secretRef is declared but never wired (correctness gap)

ExternalMemorySpec.SecretRef is added to the type and CRD, and the field doc states:

"The operator injects the key value as the QDRANT_API_KEY environment variable in the handler pod."

But the implementation does not do this. reconcileExternalMemory (operator/controller/reconcile/tool.go) only publishes ext.URL into status.QdrantEndpoint — it never reads ext.SecretRef. The handler deployment builder (operator/infra/k8s/deployment.go) also has no reference to SecretRef; it only mounts the provisioned-path kape-tool-<name>-conn Secret. There are zero SecretRef reads in operator/controller/ or operator/infra/k8s/.

Issue #12 acceptance criterion: "set → skip provisioning, pass URL (and optional secret) into the handler's QDRANT_URL / API key env." The URL path works; the secret path is a no-op. A user who sets secretRef will get no error and no injected key — the field silently does nothing, which is worse than rejecting it.

Options: (a) wire SecretRef into the handler pod env (e.g. EnvVar with SecretKeySelector → QDRANT_API_KEY) in the external branch so the documented behavior is real, or (b) if this is deferred to a follow-up, drop SecretRef from this PR's API surface and remove the doc claim, and leave #12 open (do not close it) until the secret path lands.

Finding 2 — external URL has no format validation

ExternalMemorySpec.URL only carries +kubebuilder:validation:MinLength=1. There is no Pattern / CEL rule requiring a scheme. A value like "my-qdrant" (no http(s)://) would be accepted at admission and silently published into status.QdrantEndpoint, producing a broken handler config with no operator-side signal. The review brief calls out format validation as in-scope (reachability is out of scope, format is not).

Suggested: add +kubebuilder:validation:Pattern="^https?://.+"onURL(and mirror it incrds/kape.io_kapetools.yamlviamake generate), or a CEL x-kubernetes-validations` rule. Add a test case for a malformed URL asserting admission/reconcile rejection.

Notes on what is already correct

  • Status field naming (QdrantEndpoint) is consistent across both branches — external path reuses the same field the provisioned path writes.
  • No secret values leak into status: only the URL is published to status.QdrantEndpoint, and the URL is expected to be credential-free given the SecretRef design. ✅
  • The rebase conflict in tool_test.go was resolved cleanly — duplicate helper definitions (findCondition, fakeConfigLoader, var _ ctrl.Result) were dropped since they already live in testhelpers_test.go on main; only the 3 new test functions were kept.
  • Test coverage exists for both external-URL and provisioned paths. ✅

Verdict: FIX-ROUND-NEEDED — please address Finding 1 (wire SecretRef or drop it + leave #12 open) and Finding 2 (URL format validation + a negative test).

…143)

Finding 1: ExternalMemorySpec.SecretRef was declared but never wired into
the handler pod. The deployment builder now injects a QDRANT_API_KEY env
var with a SecretKeySelector referencing the user-specified Secret
(name=ext.SecretRef.Name, key=apiKey default) when the chosen memory tool
points at an external database with a SecretRef.

Finding 2: ExternalMemorySpec.URL only had MinLength=1; a value without an
http(s):// scheme would be silently published into status.QdrantEndpoint.
Added a +kubebuilder:validation:Pattern on URL (regenerated CRD) and a
defense-in-depth scheme check in reconcileExternalMemory that sets
Ready=False / Reason=InvalidExternalURL and returns an error.

Tests:
- TestBuildDeployment_ExternalMemorySecretRef_InjectsAPIKeyEnv
- TestBuildDeployment_ExternalMemoryNoSecretRef_OmitsAPIKeyEnv
- TestToolReconciler_ExternalMemory_MalformedURL_ReportsError

Refs issue #12.
@dzungtr
dzungtr merged commit e347102 into main Jul 19, 2026
1 check passed
@dzungtr
dzungtr deleted the feat/12-kapetool-external-db branch July 19, 2026 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KapeTool memory: support pointing at an existing user-managed database URL

1 participant