Skip to content

fix(apiserver): ArkConfig is unserved in postgres mode, so cluster-wide defaults silently don't apply #3333

Description

@pontino

Problem

In postgres mode, ArkConfig does not exist as an API resource, and the admission path that would consume it passes a nil lookup. Every cluster-wide default routed through ArkConfig silently does nothing in postgres mode — no error, no warning, just etcd/postgres behavioral divergence.

  • ark/internal/apiserver/server.go:94-95 puts ArkConfig/ArkConfigList in the Scheme, but V1Alpha1Resources (ark/internal/apiserver/resources.go:21) omits them → no REST endpoint. Postgres mode skips CRDs entirely, so the resource is uncreatable, and ark-api's arkconfig.py endpoints fail underneath.
  • ark/internal/apiserver/admission.go:31,47,61 all call validation.ApplyDefaults(ctx, obj, nil), so DefaultQuery never injects spec.ttl from ArkConfig.spec.queryTTL.
  • The gap is mechanical: WebhookLookup implements ArkConfigLookup.GetArkConfig; StorageLookup (postgres mode) does not — hence the nil.

Why this matters more after #3307

#3307 makes ArkConfig the injection mechanism for the cluster default Memory, through the same DefaultQuery/ArkConfigLookup path. Once it merges, postgres mode won't just miss queryTTL — the whole default-memory feature will silently not exist there. That settles the old "wire it vs delete it" question from the #2627 review thread: wire it.

Proposed scope

  1. Serve ArkConfig from the aggregated apiserver: add it to V1Alpha1Resources. It would be the first cluster-scoped kind there (ResourceDef has no scope field; the registry assumes namespaced), so this includes a ClusterScoped flag threaded through GenericStorage/discovery. I'll spike this piece first and report back if it's bigger than it looks.
  2. Implement StorageLookup.GetArkConfig via Backend.Get(ctx, "ArkConfig", "", "default"), honoring the existing contract (any error ⇒ fall back to hardcoded defaults, never block admission).
  3. Replace the three nils in admission.go with the lookup — after which defaulting behaves identically in both modes, including feat(controller): add ArkConfig.defaultMemory and inject spec.memory at admission #3307's defaultMemory once merged.
  4. Separate commit: delete the dead storage.postgresql.* block from ark/dist/chart/values.yaml — consumed by zero templates (only .Values.storage.backend is read; the real knobs live on chart-apiserver). Documented as dead in the feat(apiserver): support Postgres TLS verification and custom CA bundle #2627 review; this closes that loop.

I'll pick this up. cc @Nab-0 @skazinka @CDimonaco

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions