Skip to content

Support agent deployments for only Broker API#7118

Closed
matheuscscp wants to merge 1 commit into
spiffe:mainfrom
matheuscscp:broker-api-only
Closed

Support agent deployments for only Broker API#7118
matheuscscp wants to merge 1 commit into
spiffe:mainfrom
matheuscscp:broker-api-only

Conversation

@matheuscscp

@matheuscscp matheuscscp commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Superseded by: #7122, #7123

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new agent.workload_api.enabled configuration knob (defaulting to enabled) to allow running spire-agent in a “Broker API only” mode by disabling the Workload API/SDS endpoints, while updating startup/health-check behavior and documentation to match.

Changes:

  • Add agent.workload_api.enabled parsing/validation (requires experimental.broker when disabled) and plumb it through to disable the agent Workload API/SDS bind address.
  • Update agent startup to conditionally start Workload API endpoints and to gate health-check readiness on either Workload API or Broker API listening, depending on mode.
  • Document the new configuration and update the full reference agent config accordingly.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/agent/config.go Clarifies that a nil Workload API bind address disables Workload API/SDS endpoints.
pkg/agent/broker/endpoints.go Adds a listening hook mechanism for Broker API readiness signaling.
pkg/agent/agent.go Conditionally starts Workload API endpoints; adjusts readiness gating and health behavior for broker-only mode.
pkg/agent/agent_test.go Adds coverage for CheckHealth behavior when Workload API is disabled.
doc/spire_agent.md Documents workload_api.enabled and broker-only agent mode, plus related socket-path notes.
conf/agent/agent_full.conf Adds reference configuration documentation for workload_api { enabled = ... }.
cmd/spire-agent/cli/run/run.go Parses/validates workload_api.enabled, avoids resolving bind address when disabled, and tracks unknown keys.
cmd/spire-agent/cli/run/run_test.go Adds tests covering disabling Workload API with/without broker config.
cmd/spire-agent/cli/run/run_posix.go Skips socket-path directory constraints and UDS prep when Workload API is disabled.
cmd/spire-agent/cli/run/run_posix_test.go Adds POSIX tests for broker/workload socket directory constraints with Workload API enabled/disabled.

Comment thread pkg/agent/broker/endpoints.go
Comment thread doc/spire_agent.md Outdated
@kfox1111

kfox1111 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

This pr generally looks good to me.

@matheuscscp matheuscscp mentioned this pull request Jul 4, 2026
Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>

// Use the agent pod UID instead of the node UID when generating the agent SPIFFE ID
UsePodUIDForAgentID bool `hcl:"use_pod_uid_for_agent_id"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#7001 (comment)

Suggested we template the agent id rather then make a bool. I think templating it would be a better idea as there could be other options we need in the future, and follows an established convention.

@kfox1111

kfox1111 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

This pr is now doing multiple things. I suggest we break it up into feature prs.

  • allow disabling workload api
  • k8s_psat agent id templating

@kfox1111 kfox1111 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To safely implement registration entries to target the correct agents in spire-controller-manager+dynamic registration, at minimum, the agent id needs to be able to include:

cluster name
service account namespace
service account name
pod uid
So just appending in the pod uid wont be sufficient. Please add agent id templating.

@sorindumitru

Copy link
Copy Markdown
Member

cluster name
service account namespace
service account name
pod uid
So just appending in the pod uid wont be sufficient. Please add agent id templating.

The cluster name and pod uid should uniquely identify the agent within a multi-cluster deployment. The namespace and service account do not need to be part of the agent SPIFFE ID.

I would say that from a UX point of view, it's preferable to provide options for the SPIFFE ID template instead of putting it on the user to choose one. If there's a clear need for it, we can look at doing that in the future.

I agree about splitting this PR into two, there are two independent changes here and it's nicer to review them in isolation. @matheuscscp, would you mind doing that.

@kfox1111

kfox1111 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

cluster name
service account namespace
service account name
pod uid
So just appending in the pod uid wont be sufficient. Please add agent id templating.

The cluster name and pod uid should uniquely identify the agent within a multi-cluster deployment. The namespace and service account do not need to be part of the agent SPIFFE ID.

There is a difference between sufficent from spire-server's perspective, and from the overall system.

Please give me an end to end example. Specifically, what do you set the spire-controller-manager parentid template to, to be able to use your proposed solution? I dont think you can, so I dont think the proposed solution is currently valid.

Its not about uniquely identifying an agent, just to spire-server or the purposes of having a unique name. you are correct there, that is unique. but not enough for the rest of the system.

How do you uniquely identify agent 1 and agent 2 on the same kubernetes node such that, you can bind registration entries to just agent 1 without leaking them to agent 2, and visa vesa in the overall system. In particular, the spire-controller-manager needs a reliable way to get a parent id for every ClusterSPIFFEID that is generated for the cluster binding only to agent 1. I think this is being ignored in designs but will be critical to having a working solution.

There needs to be a bridge built, between agent 1's svid, and the ClusterSPIFFEID's parent id templates. I only know how to do this with the dynamic registration functionality. Using dynamic registration will require the properties above I believe. To be really safe, the dynamic registration needs a to be able to limit to the k8s cluster and service account only using the spiffeid, so it needs that info to be in there.

If you continue down the path your going, its going to take a major overhaul of the spire-controller-manager to support what you suggest will work. It will take significantly more time to get end to end support in for this method.

Trust me, I'm trying to help make this a viable solution sooner.

I would say that from a UX point of view, it's preferable to provide options for the SPIFFE ID template instead of putting it on the user to choose one. If there's a clear need for it, we can look at doing that in the future.

I'm having a bit of difficulty parsing that. Are you for or against template values?

IMO, we should make good defaults for the user so they don't have to make many decisions. but give them options when they do need to make choices so they dont have to come back for patch after patch adding little bits of functionality over and over (I've hit this a lot and its really offputting to be told repeatedly, I dont need something when I do)

@matheuscscp

Copy link
Copy Markdown
Contributor Author

@sorindumitru Split done: #7122, #7123

@matheuscscp matheuscscp closed this Jul 6, 2026
@matheuscscp matheuscscp deleted the broker-api-only branch July 6, 2026 23:42
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.

4 participants