Skip to content

Replace spec.toolPrefix with spec.instance and unify singleton/family code paths #668

@QuentinBisson

Description

@QuentinBisson

Update 2026-05-19 — original framing (rename-only) was wrong: the rename
in isolation is a regression in clarity once you look at what
toolPrefix actually does after #670 landed. Rewriting the issue to
propose the rename together with unifying the singleton and family
code paths.

Summary

Replace spec.toolPrefix on the MCPServer CRD with spec.instance, and
have the aggregator use instance (defaulting to metadata.name) as the
family-enum value. Treat singletons as size-1 implicit families internally
so a single code path handles both modes.

Problem

After #670, spec.toolPrefix plays two unrelated roles depending on
whether spec.family is set:

spec.family Effect of toolPrefix
unset Drives exposed tool name x_<toolPrefix>_<tool> (falls back to metadata.name).
set Nothing. GetAllTools builds contributions with serverName: serverName (internal/aggregator/registry.go:388); the family bucket appends c.serverName (registry.go:466); injectInstanceEnum (registry.go:503) writes those bare serverName values into the enum. The family instance identifier callers pass is metadata.name, never toolPrefix.

So today toolPrefix is half a prefix knob, half a vestigial field, and
the family enum has no override knob at all. Renaming the field to
instance without changing behaviour just shifts which half of the
field's job the name lies about.

Concrete bite: the gazelle muster runs 26 kubernetes + 26 prometheus
MCPServer CRs named <cluster>-mcp-<type> for kustomize / label hygiene.
Callers have to pass management_cluster=agama-mcp-kubernetes because
the family enum is hard-wired to metadata.name. They want to pass
management_cluster=agama. There is no field that lets them.

Proposal

  1. Add spec.instance^[a-zA-Z][a-zA-Z0-9_-]*$, optional,
    defaulting to metadata.name.
  2. Aggregator changes (internal/aggregator/registry.go):
    • Non-family servers: exposed tool name = x_<instance>_<tool>
      (today's behaviour, with instance substituted for toolPrefix).
    • Family servers: injectInstanceEnum writes instance values
      (not raw serverName). family.name still drives the tool-name
      prefix.
    • Internally, when spec.family is unset, treat the server as a
      size-1 implicit family with family.name defaulting to instance.
      Callers see no selector (size-1 → omit/default:), but the code
      path is one.
  3. Deprecate spec.toolPrefix — keep as an alias for spec.instance
    for one minor release. Admission warning when both are set or when
    only toolPrefix is used. Remove the release after.
  4. CHANGELOG: Added spec.instance. and Deprecated spec.toolPrefix.
    under the matching Keep a Changelog H3 sections.

Why this is the right shape

  • One field, one honest meaning in both modes. instance describes a
    member of a family (explicit or singleton).
  • Adds the override the gazelle case needs — instance: agama
    management_cluster=agama at call sites.
  • Unifies the two code paths, which removes the "vestigial field"
    gotcha and sets up the singleton-ergonomics fix (default: instead
    of required: for size-1 families) cleanly.

Why not rename-only

Calling the field instance is honest only if every server is an
instance of a family. Without the model + aggregator change, instance
lies about the singleton role (the field is literally a tool-name prefix
there, not an instance identifier) and lies about the family role (the
field has no effect today). The alternatives without behavioural change
all clunk:

Name Honest in singleton mode Honest in family mode
toolPrefix yes no (vestigial)
instance no yes
alias generic generic
displayName partial partial

So either ship this issue, or drop the rename and instead add CRD
validation !(has(self.toolPrefix) && has(self.family)) to make the
vestigial-field gotcha go away — that's the only other coherent shape.

Out of scope

  • Removing auth.type: teleport (tracked in Remove Teleport authentication support #687).
  • Singleton-schema softening (default: instead of required: for
    size-1 families) — separate follow-up that becomes trivial once
    this lands.
  • Any change to family.name validation or instanceArg semantics
    beyond the default.

Acceptance criteria

  • MCPServerSpec.Instance field added with kubebuilder validation;
    optional; pattern ^[a-zA-Z][a-zA-Z0-9_-]*$.
  • MCPServerSpec.ToolPrefix marked deprecated in godoc, CRD doc,
    and CHANGELOG; both fields readable; admission warning when both
    are set.
  • assembleExposedTools uses instance (default metadata.name)
    for both the non-family tool-name prefix and the family enum
    value; no remaining reads of bare serverName for those paths.
  • BDD scenario: a family of three servers with distinct instance
    values exposes those values (not the long CR names) in the enum;
    omitting instance falls back to metadata.name.
  • helm/muster/crds/muster.giantswarm.io_mcpservers.yaml
    regenerated; pkg/apis/muster/v1alpha1/zz_generated.deepcopy.go
    regenerated.
  • CHANGELOG entry under ### Added and ### Deprecated.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions