Skip to content

Add ServerAgentFactory SPI for extension-contributed agents#2130

Open
krital wants to merge 3 commits into
developmentfrom
tailnet-server-agent-spi
Open

Add ServerAgentFactory SPI for extension-contributed agents#2130
krital wants to merge 3 commits into
developmentfrom
tailnet-server-agent-spi

Conversation

@krital

@krital krital commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a generic extension point so third-party jars can contribute a lifecycle Agent to the server via java.util.ServiceLoader, without any core edits per extension.

  • New io.mapsmessaging.ServerAgentFactory interface: Agent create(), int startOrder(), int stopOrder().
  • SubSystemManager.createAgentStartStopList() now calls loadExtensionAgents(), which discovers all ServerAgentFactory providers and registers each returned Agent with its start/stop ordering (added after the built-in agents, so ordering composes correctly).
  • Loading is hardened, mirroring the existing loadProtocolImplementations() pattern:
    • iterator.next() is inside the try, so a ServiceConfigurationError from a malformed META-INF/services entry or a factory that fails to instantiate is caught per-provider and cannot crash daemon startup.
    • Duplicate-name guard: an extension agent whose getName() collides with an already-registered agent is skipped (can't silently shadow a core manager in agentMap).
    • Failures log a new non-fatal MESSAGE_DAEMON_EXTENSION_AGENT_FAILED (WARN), distinct from the fatal core-agent path (which reads "system exitting").
  • No feature-gating: discovered extension agents load unconditionally (they are opt-in, drop-in jars).

Motivation: the first MAPS extension (tailnet onboarding) needs to run a long-lived subsystem inside the daemon. The EndPointServerFactory route can't bind a custom-scheme endpoint (EndPointConfigFactory returns null for unknown schemes, unlike the protocol side's ExtensionConfig fallback), so a general Agent SPI is the clean seam.

Changes

  • src/main/java/io/mapsmessaging/ServerAgentFactory.java (new)
  • src/main/java/io/mapsmessaging/SubSystemManager.java (+loadExtensionAgents() + call)
  • src/main/java/io/mapsmessaging/logging/ServerLogMessages.java (+MESSAGE_DAEMON_EXTENSION_AGENT_FAILED)

Test Plan

  • mvn -DskipTests install builds; artifact contains io.mapsmessaging.ServerAgentFactory.
  • Existing daemon startup unaffected when no ServerAgentFactory providers are present (additive, no built-in agents changed).
  • Verified end-to-end against the maps-tailnet-onboarding extension: dropping its jar in /opt/maps/lib/ loads TailnetAgent (Service Manager Tailnet Onboarding loadedStarted Tailnet Onboarding), no NetworkManager.yaml entry needed.
  • A deliberately broken extension jar (bad services entry / throwing create()) logs a WARN and does not crash startup.

🤖 Generated with Claude Code

@krital krital requested a review from mbuckton July 8, 2026 17:16
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a68cdc77-3600-44ad-a59a-ced139bbd221

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tailnet-server-agent-spi

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@krital

krital commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressing the security review concern (allowlist added — commit 975ac08)

Thanks for flagging the plugin-loading risk. I've pushed a defense-in-depth change and want to lay out the reasoning.

The change: SubSystemManager.loadExtensionAgents() now only instantiates a discovered ServerAgentFactory if its fully-qualified class name is listed in the system property extension.agents.allowed (comma-separated). The default is empty → nothing loads. So an operator must consciously opt in each extension agent, e.g.:

-Dextension.agents.allowed=io.mapsmessaging.extension.tailnet.spi.TailnetAgentFactory

An un-listed factory is skipped with a log line rather than started. No jar can auto-run an agent just by being present.

On the underlying trust boundary: it's worth noting the SPI does not create a new code-execution path. Anyone who can write a jar into /opt/maps/lib/ can already run arbitrary code in the MAPS JVM today — via Java static initializers and the existing ServiceLoader SPIs the server already auto-loads from lib/ (ProtocolImplFactory, EndPointServerFactory, TokenGenerator, SystemTopic, Analyser, PacketIntegrity, …). ServerAgentFactory sits on the same trust model as those. So the marginal attack surface is ~zero; lib/ is already a trusted directory. The allowlist is added as auditable, opt-in defense-in-depth rather than because the base mechanism is unsafe.

Note: the extension it was built for (tailnet onboarding) can now also run fully out-of-process (a standalone launcher against a stock server), so this SPI is a convenience for in-process integration, not a hard dependency — no rush if you'd like more time on it.

Happy to adjust: e.g. gate the allowlist itself behind a licensed feature, require signed jars, or default to a specific known-good set instead of empty. Whatever you're most comfortable with.

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.

1 participant