From 1464bded65f1d0de9960f098e20dc73b4678e0eb Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 8 Jul 2026 01:01:43 +0100 Subject: [PATCH 1/2] Introduce flags for disabling Workload and SDS APIs Signed-off-by: Matheus Pimenta --- cmd/spire-agent/cli/run/run.go | 20 ++- cmd/spire-agent/cli/run/run_posix.go | 42 +++--- cmd/spire-agent/cli/run/run_posix_test.go | 55 +++++++ cmd/spire-agent/cli/run/run_test.go | 99 ++++++++++++ cmd/spire-agent/cli/run/run_windows_test.go | 41 +++++ conf/agent/agent_full.conf | 16 +- doc/spire_agent.md | 12 +- pkg/agent/agent.go | 29 +++- pkg/agent/api/health/v1/service.go | 17 ++- pkg/agent/api/health/v1/service_test.go | 11 +- pkg/agent/config.go | 8 +- pkg/agent/endpoints/config.go | 4 + pkg/agent/endpoints/endpoints.go | 76 +++++++--- pkg/agent/endpoints/endpoints_test.go | 159 ++++++++++++++++++++ 14 files changed, 524 insertions(+), 65 deletions(-) diff --git a/cmd/spire-agent/cli/run/run.go b/cmd/spire-agent/cli/run/run.go index b5165ced04..81c905b677 100644 --- a/cmd/spire-agent/cli/run/run.go +++ b/cmd/spire-agent/cli/run/run.go @@ -83,6 +83,8 @@ type agentConfig struct { ServerAddress string `hcl:"server_address"` ServerPort int `hcl:"server_port"` SocketPath string `hcl:"socket_path"` + DisableWorkloadAPI bool `hcl:"disable_workload_api"` + DisableSDSAPI bool `hcl:"disable_sds_api"` WorkloadX509SVIDKeyType string `hcl:"workload_x509_svid_key_type"` TrustBundleFormat string `hcl:"trust_bundle_format"` TrustBundlePath string `hcl:"trust_bundle_path"` @@ -483,6 +485,8 @@ func parseFlags(name string, args []string, output io.Writer) (*agentConfig, err flags.StringVar(&c.ServerAddress, "serverAddress", "", "IP address or DNS name of the SPIRE server") flags.IntVar(&c.ServerPort, "serverPort", 0, "Port number of the SPIRE server") flags.StringVar(&c.TrustDomain, "trustDomain", "", "The trust domain that this agent belongs to") + flags.BoolVar(&c.DisableWorkloadAPI, "disableWorkloadAPI", false, "Disable the SPIFFE Workload API") + flags.BoolVar(&c.DisableSDSAPI, "disableSDSAPI", false, "Disable the Envoy SDS API") flags.StringVar(&c.TrustBundlePath, "trustBundle", "", "Path to the SPIRE server CA bundle") flags.StringVar(&c.TrustBundleURL, "trustBundleUrl", "", "URL to download the SPIRE server CA bundle") flags.StringVar(&c.TrustBundleFormat, "trustBundleFormat", "", fmt.Sprintf("Format of the bootstrap trust bundle, %q or %q", trustbundlesources.BundleFormatPEM, trustbundlesources.BundleFormatSPIFFE)) @@ -524,6 +528,10 @@ func mergeInput(fileInput *Config, cliInput *agentConfig) (*Config, error) { return c, nil } +func (c *agentConfig) endpointEnabled() bool { + return !c.DisableWorkloadAPI || !c.DisableSDSAPI +} + func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool) (*agent.Config, error) { ac := &agent.Config{} @@ -632,11 +640,15 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool) } ac.TrustDomain = td - addr, err := c.Agent.getAddr() - if err != nil { - return nil, err + ac.DisableWorkloadAPI = c.Agent.DisableWorkloadAPI + ac.DisableSDSAPI = c.Agent.DisableSDSAPI + if c.Agent.endpointEnabled() { + addr, err := c.Agent.getAddr() + if err != nil { + return nil, err + } + ac.BindAddress = addr } - ac.BindAddress = addr if c.Agent.hasAdminAddr() { adminAddr, err := c.Agent.getAdminAddr() diff --git a/cmd/spire-agent/cli/run/run_posix.go b/cmd/spire-agent/cli/run/run_posix.go index 8a63a48e97..0ecf35894d 100644 --- a/cmd/spire-agent/cli/run/run_posix.go +++ b/cmd/spire-agent/cli/run/run_posix.go @@ -30,17 +30,19 @@ func (c *agentConfig) getAddr() (net.Addr, error) { } func (c *agentConfig) getAdminAddr() (net.Addr, error) { - socketPathAbs, err := filepath.Abs(c.SocketPath) - if err != nil { - return nil, fmt.Errorf("failed to get absolute path for socket_path: %w", err) - } adminSocketPathAbs, err := filepath.Abs(c.AdminSocketPath) if err != nil { return nil, fmt.Errorf("failed to get absolute path for admin_socket_path: %w", err) } - if strings.HasPrefix(adminSocketPathAbs, filepath.Dir(socketPathAbs)+"/") { - return nil, errors.New("admin socket cannot be in the same directory or a subdirectory as that containing the Workload API socket") + if c.endpointEnabled() { + socketPathAbs, err := filepath.Abs(c.SocketPath) + if err != nil { + return nil, fmt.Errorf("failed to get absolute path for socket_path: %w", err) + } + if strings.HasPrefix(adminSocketPathAbs, filepath.Dir(socketPathAbs)+"/") { + return nil, errors.New("admin socket cannot be in the same directory or a subdirectory as that containing the Workload API socket") + } } return &net.UnixAddr{ @@ -56,17 +58,19 @@ func (c *agentConfig) hasAdminAddr() bool { // brokerSocketAddr resolves the UDS branch of broker bind-address selection. // Returns the platform-agnostic TCP branch is handled in brokerBindAddr. func (c *agentConfig) brokerSocketAddr() (net.Addr, error) { - socketPathAbs, err := filepath.Abs(c.SocketPath) - if err != nil { - return nil, fmt.Errorf("failed to get absolute path for socket_path: %w", err) - } brokerSocketPathAbs, err := filepath.Abs(c.Experimental.Broker.SocketPath) if err != nil { return nil, fmt.Errorf("failed to get absolute path for experimental.broker.socket_path: %w", err) } - if strings.HasPrefix(brokerSocketPathAbs, filepath.Dir(socketPathAbs)+"/") { - return nil, errors.New("broker socket cannot be in the same directory or a subdirectory as that containing the Workload API socket") + if c.endpointEnabled() { + socketPathAbs, err := filepath.Abs(c.SocketPath) + if err != nil { + return nil, fmt.Errorf("failed to get absolute path for socket_path: %w", err) + } + if strings.HasPrefix(brokerSocketPathAbs, filepath.Dir(socketPathAbs)+"/") { + return nil, errors.New("broker socket cannot be in the same directory or a subdirectory as that containing the Workload API socket") + } } return &net.UnixAddr{ @@ -87,12 +91,14 @@ func (c *agentConfig) validateOS() error { } func prepareEndpoints(c *agent.Config) error { - // Create uds dir and parents if not exists - dir := filepath.Dir(c.BindAddress.String()) - if _, statErr := os.Stat(dir); os.IsNotExist(statErr) { - c.Log.WithField("dir", dir).Infof("Creating spire agent UDS directory") - if err := os.MkdirAll(dir, 0755); err != nil { - return err + if c.BindAddress != nil { + // Create uds dir and parents if not exists + dir := filepath.Dir(c.BindAddress.String()) + if _, statErr := os.Stat(dir); os.IsNotExist(statErr) { + c.Log.WithField("dir", dir).Infof("Creating spire agent UDS directory") + if err := os.MkdirAll(dir, 0755); err != nil { + return err + } } } diff --git a/cmd/spire-agent/cli/run/run_posix_test.go b/cmd/spire-agent/cli/run/run_posix_test.go index 8767f9a681..b0a0b2cabd 100644 --- a/cmd/spire-agent/cli/run/run_posix_test.go +++ b/cmd/spire-agent/cli/run/run_posix_test.go @@ -151,6 +151,8 @@ func TestParseFlagsGood(t *testing.T) { "-serverAddress=127.0.0.1", "-serverPort=8081", "-socketPath=/tmp/spire-agent/public/api.sock", + "-disableWorkloadAPI", + "-disableSDSAPI", "-trustBundle=conf/agent/dummy_root_ca.crt", "-trustBundleUrl=https://test.url", "-trustDomain=example.org", @@ -162,6 +164,8 @@ func TestParseFlagsGood(t *testing.T) { assert.Equal(t, c.ServerAddress, "127.0.0.1") assert.Equal(t, c.ServerPort, 8081) assert.Equal(t, c.SocketPath, "/tmp/spire-agent/public/api.sock") + assert.True(t, c.DisableWorkloadAPI) + assert.True(t, c.DisableSDSAPI) assert.Equal(t, c.TrustBundlePath, "conf/agent/dummy_root_ca.crt") assert.Equal(t, c.TrustBundleURL, "https://test.url") assert.Equal(t, c.TrustDomain, "example.org") @@ -296,6 +300,20 @@ func newAgentConfigCasesOS(t *testing.T) []newAgentConfigCase { require.Equal(t, "unix", c.AdminBindAddress.Network()) }, }, + { + msg: "admin_socket_path can share disabled public endpoint socket directory", + input: func(c *Config) { + c.Agent.SocketPath = "/tmp/workload.sock" + c.Agent.DisableWorkloadAPI = true + c.Agent.DisableSDSAPI = true + c.Agent.AdminSocketPath = "/tmp/admin.sock" + }, + test: func(t *testing.T, c *agent.Config) { + require.Nil(t, c.BindAddress) + require.Equal(t, "/tmp/admin.sock", c.AdminBindAddress.String()) + require.Equal(t, "unix", c.AdminBindAddress.Network()) + }, + }, { msg: "admin_socket_path configured with similar folder that socket_path", input: func(c *Config) { @@ -331,6 +349,18 @@ func newAgentConfigCasesOS(t *testing.T) []newAgentConfigCase { require.Nil(t, c) }, }, + { + msg: "admin_socket_path same folder as socket_path when only workload API is disabled", + expectError: true, + input: func(c *Config) { + c.Agent.SocketPath = "/tmp/workload.sock" + c.Agent.DisableWorkloadAPI = true + c.Agent.AdminSocketPath = "/tmp/admin.sock" + }, + test: func(t *testing.T, c *agent.Config) { + require.Nil(t, c) + }, + }, { msg: "admin_socket_path configured with subfolder socket_path", expectError: true, @@ -353,6 +383,31 @@ func newAgentConfigCasesOS(t *testing.T) []newAgentConfigCase { require.Nil(t, c) }, }, + { + msg: "broker socket can share disabled public endpoint socket directory", + input: func(c *Config) { + c.Agent.SocketPath = "/tmp/workload.sock" + c.Agent.DisableWorkloadAPI = true + c.Agent.DisableSDSAPI = true + c.Agent.Experimental.Broker = &brokerHCLConfig{ + SocketPath: "/tmp/broker.sock", + Brokers: []brokerHCLEntry{ + { + ID: "spiffe://example.org/broker", + AllowedReferenceTypes: []brokerAllowedReferenceTypeHCLEntry{ + {TypeURL: "type.googleapis.com/spiffe.broker.WorkloadPIDReference"}, + }, + }, + }, + } + }, + test: func(t *testing.T, c *agent.Config) { + require.Nil(t, c.BindAddress) + require.Len(t, c.Broker.BindAddresses, 1) + require.Equal(t, "/tmp/broker.sock", c.Broker.BindAddresses[0].String()) + require.Equal(t, "unix", c.Broker.BindAddresses[0].Network()) + }, + }, { msg: "admin_socket_path not provided", input: func(c *Config) { diff --git a/cmd/spire-agent/cli/run/run_test.go b/cmd/spire-agent/cli/run/run_test.go index 45f265e92f..da768125ba 100644 --- a/cmd/spire-agent/cli/run/run_test.go +++ b/cmd/spire-agent/cli/run/run_test.go @@ -553,6 +553,62 @@ func TestMergeInput(t *testing.T) { require.Equal(t, "bar", c.Agent.TrustDomain) }, }, + { + msg: "disable_workload_api should default to false", + fileInput: func(c *Config) {}, + cliInput: func(c *agentConfig) {}, + test: func(t *testing.T, c *Config) { + require.False(t, c.Agent.DisableWorkloadAPI) + }, + }, + { + msg: "disable_workload_api should be configurable by file", + fileInput: func(c *Config) { + c.Agent.DisableWorkloadAPI = true + }, + cliInput: func(c *agentConfig) {}, + test: func(t *testing.T, c *Config) { + require.True(t, c.Agent.DisableWorkloadAPI) + }, + }, + { + msg: "disable_workload_api should be configurable by CLI flag", + fileInput: func(c *Config) {}, + cliInput: func(c *agentConfig) { + c.DisableWorkloadAPI = true + }, + test: func(t *testing.T, c *Config) { + require.True(t, c.Agent.DisableWorkloadAPI) + }, + }, + { + msg: "disable_sds_api should default to false", + fileInput: func(c *Config) {}, + cliInput: func(c *agentConfig) {}, + test: func(t *testing.T, c *Config) { + require.False(t, c.Agent.DisableSDSAPI) + }, + }, + { + msg: "disable_sds_api should be configurable by file", + fileInput: func(c *Config) { + c.Agent.DisableSDSAPI = true + }, + cliInput: func(c *agentConfig) {}, + test: func(t *testing.T, c *Config) { + require.True(t, c.Agent.DisableSDSAPI) + }, + }, + { + msg: "disable_sds_api should be configurable by CLI flag", + fileInput: func(c *Config) {}, + cliInput: func(c *agentConfig) { + c.DisableSDSAPI = true + }, + test: func(t *testing.T, c *Config) { + require.True(t, c.Agent.DisableSDSAPI) + }, + }, { msg: "require_pq_kem should be configurable by file", fileInput: func(c *Config) { @@ -993,6 +1049,49 @@ func TestNewAgentConfig(t *testing.T) { assert.True(t, c.DisableSPIFFECertValidation) }, }, + { + msg: "public endpoint is enabled by default", + input: func(c *Config) {}, + test: func(t *testing.T, c *agent.Config) { + require.NotNil(t, c.BindAddress) + require.False(t, c.DisableWorkloadAPI) + require.False(t, c.DisableSDSAPI) + }, + }, + { + msg: "disable_workload_api keeps public endpoint enabled", + input: func(c *Config) { + c.Agent.DisableWorkloadAPI = true + }, + test: func(t *testing.T, c *agent.Config) { + require.NotNil(t, c.BindAddress) + require.True(t, c.DisableWorkloadAPI) + require.False(t, c.DisableSDSAPI) + }, + }, + { + msg: "disable_sds_api keeps public endpoint enabled", + input: func(c *Config) { + c.Agent.DisableSDSAPI = true + }, + test: func(t *testing.T, c *agent.Config) { + require.NotNil(t, c.BindAddress) + require.False(t, c.DisableWorkloadAPI) + require.True(t, c.DisableSDSAPI) + }, + }, + { + msg: "disable_workload_api and disable_sds_api disable public endpoint", + input: func(c *Config) { + c.Agent.DisableWorkloadAPI = true + c.Agent.DisableSDSAPI = true + }, + test: func(t *testing.T, c *agent.Config) { + require.Nil(t, c.BindAddress) + require.True(t, c.DisableWorkloadAPI) + require.True(t, c.DisableSDSAPI) + }, + }, { msg: "allowed_foreign_jwt_claims no provided", input: func(c *Config) { diff --git a/cmd/spire-agent/cli/run/run_windows_test.go b/cmd/spire-agent/cli/run/run_windows_test.go index 99d416451f..2b3ccf3d40 100644 --- a/cmd/spire-agent/cli/run/run_windows_test.go +++ b/cmd/spire-agent/cli/run/run_windows_test.go @@ -137,6 +137,8 @@ func TestParseFlagsGood(t *testing.T) { "-serverAddress=127.0.0.1", "-serverPort=8081", "-namedPipeName=\\spire-agent\\public\\api", + "-disableWorkloadAPI", + "-disableSDSAPI", "-trustBundle=conf/agent/dummy_root_ca.crt", "-trustBundleUrl=https://test.url", "-trustDomain=example.org", @@ -148,6 +150,8 @@ func TestParseFlagsGood(t *testing.T) { assert.Equal(t, "127.0.0.1", c.ServerAddress) assert.Equal(t, 8081, c.ServerPort) assert.Equal(t, "\\spire-agent\\public\\api", c.Experimental.NamedPipeName) + assert.True(t, c.DisableWorkloadAPI) + assert.True(t, c.DisableSDSAPI) assert.Equal(t, "conf/agent/dummy_root_ca.crt", c.TrustBundlePath) assert.Equal(t, "https://test.url", c.TrustBundleURL) assert.Equal(t, "example.org", c.TrustDomain) @@ -271,6 +275,43 @@ func newAgentConfigCasesOS(*testing.T) []newAgentConfigCase { require.Equal(t, "pipe", c.BindAddress.(*namedpipe.Addr).Network()) }, }, + { + msg: "disable_workload_api keeps named pipe enabled", + input: func(c *Config) { + c.Agent.Experimental.NamedPipeName = "foo" + c.Agent.DisableWorkloadAPI = true + }, + test: func(t *testing.T, c *agent.Config) { + require.Equal(t, "\\\\.\\pipe\\foo", c.BindAddress.String()) + require.True(t, c.DisableWorkloadAPI) + require.False(t, c.DisableSDSAPI) + }, + }, + { + msg: "disable_sds_api keeps named pipe enabled", + input: func(c *Config) { + c.Agent.Experimental.NamedPipeName = "foo" + c.Agent.DisableSDSAPI = true + }, + test: func(t *testing.T, c *agent.Config) { + require.Equal(t, "\\\\.\\pipe\\foo", c.BindAddress.String()) + require.False(t, c.DisableWorkloadAPI) + require.True(t, c.DisableSDSAPI) + }, + }, + { + msg: "disable_workload_api and disable_sds_api disable named pipe", + input: func(c *Config) { + c.Agent.Experimental.NamedPipeName = "foo" + c.Agent.DisableWorkloadAPI = true + c.Agent.DisableSDSAPI = true + }, + test: func(t *testing.T, c *agent.Config) { + require.Nil(t, c.BindAddress) + require.True(t, c.DisableWorkloadAPI) + require.True(t, c.DisableSDSAPI) + }, + }, { msg: "admin_named_pipe_name not provided", input: func(c *Config) { diff --git a/conf/agent/agent_full.conf b/conf/agent/agent_full.conf index 2dd5be64fc..e840c1b790 100644 --- a/conf/agent/agent_full.conf +++ b/conf/agent/agent_full.conf @@ -61,9 +61,19 @@ agent { # server_port: Port number of the SPIRE server. server_port = "8081" - # socket_path: Location to bind the workload API socket. Default: /tmp/spire-agent/public/api.sock. + # socket_path: Location to bind the Workload API and SDS socket. The socket is + # exposed unless both disable_workload_api and disable_sds_api are true. + # Default: /tmp/spire-agent/public/api.sock. socket_path = "/tmp/spire-agent/public/api.sock" + # disable_workload_api: Disable the SPIFFE Workload API. The public socket or named + # pipe remains exposed if SDS is enabled. Default: false. + # disable_workload_api = false + + # disable_sds_api: Disable the Envoy SDS API. The public socket or named pipe remains + # exposed if the Workload API is enabled. Default: false. + # disable_sds_api = false + # trust_bundle_path: Path to the SPIRE server CA bundle. trust_bundle_path = "./conf/agent/dummy_root_ca.crt" @@ -115,7 +125,9 @@ agent { # experimental: The experimental options that are subject to change or removal # experimental { - # # named_pipe_name: Pipe name to bind the SPIRE Agent API named pipe (Windows only). + # # named_pipe_name: Pipe name to bind the SPIRE Agent Workload API and SDS named + # # pipe (Windows only). The named pipe is exposed unless both + # # disable_workload_api and disable_sds_api are true. # # Default: \spire-agent\public\api # named_pipe_name = "\\spire-agent\\public\\api" diff --git a/doc/spire_agent.md b/doc/spire_agent.md index 091ce15c4b..b83012f877 100644 --- a/doc/spire_agent.md +++ b/doc/spire_agent.md @@ -66,7 +66,9 @@ This may be useful for templating configuration files, for example across differ | `profiling_port` | Port number of the [net/http/pprof](https://pkg.go.dev/net/http/pprof) endpoint. Only used when `profiling_enabled` is `true`. | | | `server_address` | DNS name or IP address of the SPIRE server | | | `server_port` | Port number of the SPIRE server | | -| `socket_path` | Location to bind the SPIRE Agent API socket (Unix only) | /tmp/spire-agent/public/api.sock | +| `socket_path` | Location to bind the SPIRE Agent Workload API and SDS socket (Unix only). The socket is exposed unless both `disable_workload_api` and `disable_sds_api` are `true`. | /tmp/spire-agent/public/api.sock | +| `disable_workload_api` | Disable the SPIFFE Workload API. The public socket or named pipe remains exposed if SDS is enabled. | false | +| `disable_sds_api` | Disable the Envoy SDS API. The public socket or named pipe remains exposed if the Workload API is enabled. | false | | `sds` | Optional SDS configuration section | | | `trust_bundle_path` | Path to the SPIRE server CA bundle | | | `trust_bundle_url` | URL to download the initial SPIRE server trust bundle | | @@ -80,7 +82,7 @@ This may be useful for templating configuration files, for example across differ | experimental | Description | Default | | :---------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------- | -| `named_pipe_name` | Pipe name to bind the SPIRE Agent API named pipe (Windows only) | \spire-agent\public\api | +| `named_pipe_name` | Pipe name to bind the SPIRE Agent Workload API and SDS named pipe (Windows only). The named pipe is exposed unless both `disable_workload_api` and `disable_sds_api` are `true`. | \spire-agent\public\api | | `sync_interval` | Sync interval with SPIRE server with exponential backoff | 5 sec | | `use_sync_authorized_entries` | Use SyncAuthorizedEntries API for periodically synchronization of authorized entries | true | | `require_pq_kem` | Require use of a post-quantum-safe key exchange method for TLS handshakes | false | @@ -307,9 +309,11 @@ the following flags are available: | `-logFile` | File to write logs to | | | `-logFormat` | Format of logs, <text|json> | | | `-logLevel` | DEBUG, INFO, WARN or ERROR | | +| `-disableWorkloadAPI` | Disable the SPIFFE Workload API | false | +| `-disableSDSAPI` | Disable the Envoy SDS API | false | | `-serverAddress` | IP address or DNS name of the SPIRE server | | | `-serverPort` | Port number of the SPIRE server | | -| `-socketPath` | Location to bind the workload API socket | | +| `-socketPath` | Location to bind the Workload API and SDS socket | | | `-trustBundle` | Path to the SPIRE server CA bundle | | | `-trustBundleUrl` | URL to download the SPIRE server CA bundle | | | `-trustDomain` | The trust domain that this agent belongs to (should be no more than 255 characters) | | @@ -682,7 +686,7 @@ gRPC layer. ## Envoy SDS Support SPIRE agent has support for the [Envoy](https://envoyproxy.io) [Secret Discovery Service](https://www.envoyproxy.io/docs/envoy/latest/configuration/security/secret) (SDS). -SDS is served over the same Unix domain socket as the Workload API. Envoy processes connecting to SDS are attested as workloads. +SDS is served over the same public SPIRE Agent socket or named pipe as the Workload API. Envoy processes connecting to SDS are attested as workloads. SDS can be disabled with `disable_sds_api`. [`tlsv3.TlsCertificate`](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#extensions-transport-sockets-tls-v3-tlscertificate) resources containing X509-SVIDs can be fetched using the SPIFFE ID of the workload as the resource name diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index bd63a086bb..b4737d373e 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -274,20 +274,25 @@ func (a *Agent) Run(ctx context.Context) error { Metrics: metrics, }) - agentEndpoints := a.newEndpoints(metrics, mgr, workloadAttestor) - go func() { - agentEndpoints.WaitForListening(readyForHealthChecks) - a.started = true - }() - tasks := []func(context.Context) error{ metrics.ListenAndServe, mgr.Run, storeService.Run, - agentEndpoints.ListenAndServe, catalog.ReconfigureTask(a.c.Log.WithField(telemetry.SubsystemName, "reconfigurer"), cat), } + if a.c.BindAddress != nil { + agentEndpoints := a.newEndpoints(metrics, mgr, workloadAttestor) + go func() { + agentEndpoints.WaitForListening(readyForHealthChecks) + a.started = true + }() + tasks = append(tasks, agentEndpoints.ListenAndServe) + } else { + a.started = true + close(readyForHealthChecks) + } + if a.c.AdminBindAddress != nil { adminEndpoints := a.newAdminEndpoints(metrics, mgr, workloadAttestor, a.c.AuthorizedDelegates) tasks = append(tasks, adminEndpoints.ListenAndServe) @@ -518,6 +523,8 @@ func (a *Agent) newEndpoints(metrics telemetry.Metrics, mgr manager.Manager, att AllowedForeignJWTClaims: a.c.AllowedForeignJWTClaims, LogSelectors: a.c.LogSelectors, TrustDomain: a.c.TrustDomain, + DisableWorkloadAPI: a.c.DisableWorkloadAPI, + DisableSDSAPI: a.c.DisableSDSAPI, WorkloadAPIRateLimit: a.c.WorkloadAPIRateLimit, }) } @@ -540,6 +547,14 @@ func (a *Agent) newAdminEndpoints(metrics telemetry.Metrics, mgr manager.Manager // CheckHealth is used as a top-level health check for the agent. func (a *Agent) CheckHealth() health.State { + if a.c.BindAddress == nil || a.c.DisableWorkloadAPI { + return health.State{ + Started: &a.started, + Ready: a.started, + Live: true, + } + } + err := a.checkWorkloadAPI() // Both liveness and readiness checks are done by diff --git a/pkg/agent/api/health/v1/service.go b/pkg/agent/api/health/v1/service.go index a344be5999..31d32c62e1 100644 --- a/pkg/agent/api/health/v1/service.go +++ b/pkg/agent/api/health/v1/service.go @@ -23,14 +23,18 @@ func RegisterService(s grpc.ServiceRegistrar, service *Service) { // Config is the service configuration type Config struct { - // Addr is the Workload API socket address + // Addr is the public Workload API/SDS endpoint address. Addr net.Addr + + // DisableWorkloadAPI indicates that the Workload API is not registered on the endpoint. + DisableWorkloadAPI bool } // New creates a new Health service func New(config Config) *Service { return &Service{ - addr: config.Addr, + addr: config.Addr, + disableWorkloadAPI: config.DisableWorkloadAPI, } } @@ -38,7 +42,8 @@ func New(config Config) *Service { type Service struct { grpc_health_v1.UnimplementedHealthServer - addr net.Addr + addr net.Addr + disableWorkloadAPI bool } func (s *Service) Check(ctx context.Context, req *grpc_health_v1.HealthCheckRequest) (*grpc_health_v1.HealthCheckResponse, error) { @@ -49,6 +54,12 @@ func (s *Service) Check(ctx context.Context, req *grpc_health_v1.HealthCheckRequ return nil, api.MakeErr(log, codes.InvalidArgument, "per-service health is not supported", nil) } + if s.disableWorkloadAPI { + return &grpc_health_v1.HealthCheckResponse{ + Status: grpc_health_v1.HealthCheckResponse_SERVING, + }, nil + } + clientOption, err := util.GetWorkloadAPIClientOption(s.addr) if err != nil { return nil, api.MakeErr(log, codes.InvalidArgument, "could not get Workload API client options", err) diff --git a/pkg/agent/api/health/v1/service_test.go b/pkg/agent/api/health/v1/service_test.go index 93a0988ea0..19e6cae61c 100644 --- a/pkg/agent/api/health/v1/service_test.go +++ b/pkg/agent/api/health/v1/service_test.go @@ -35,6 +35,7 @@ func TestServiceCheck(t *testing.T) { for _, tt := range []struct { name string + disableWorkloadAPI bool wlapiCode codes.Code service string expectCode codes.Code @@ -53,6 +54,13 @@ func TestServiceCheck(t *testing.T) { expectCode: codes.OK, expectServingStatus: grpc_health_v1.HealthCheckResponse_SERVING, }, + { + name: "success with Workload API disabled", + disableWorkloadAPI: true, + wlapiCode: codes.Unavailable, + expectCode: codes.OK, + expectServingStatus: grpc_health_v1.HealthCheckResponse_SERVING, + }, { name: "failure with other status codes", wlapiCode: codes.Unavailable, @@ -92,7 +100,8 @@ func TestServiceCheck(t *testing.T) { } service := health.New(health.Config{ - Addr: spiretest.StartWorkloadAPI(t, wlAPI), + Addr: spiretest.StartWorkloadAPI(t, wlAPI), + DisableWorkloadAPI: tt.disableWorkloadAPI, }) server := grpctest.StartServer(t, func(s grpc.ServiceRegistrar) { diff --git a/pkg/agent/config.go b/pkg/agent/config.go index 0188344894..dd75448c49 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -27,9 +27,15 @@ const ( type WorkloadAPIRateLimitConfig = endpoints.WorkloadAPIRateLimitConfig type Config struct { - // Address to bind the workload api to + // Address to bind the public Workload API/SDS endpoint to. Nil disables the endpoint. BindAddress net.Addr + // DisableWorkloadAPI disables serving the SPIFFE Workload API. + DisableWorkloadAPI bool + + // DisableSDSAPI disables serving the Envoy SDS API. + DisableSDSAPI bool + // Directory to store runtime data DataDir string diff --git a/pkg/agent/endpoints/config.go b/pkg/agent/endpoints/config.go index a3ae40da04..d7ee0e7736 100644 --- a/pkg/agent/endpoints/config.go +++ b/pkg/agent/endpoints/config.go @@ -58,6 +58,10 @@ type Config struct { TrustDomain spiffeid.TrustDomain + DisableWorkloadAPI bool + + DisableSDSAPI bool + // WorkloadAPIRateLimit configures per-selector-set rate limiting for Workload API and SDS methods. WorkloadAPIRateLimit WorkloadAPIRateLimitConfig diff --git a/pkg/agent/endpoints/endpoints.go b/pkg/agent/endpoints/endpoints.go index 0dc9e24db6..e7e68b0629 100644 --- a/pkg/agent/endpoints/endpoints.go +++ b/pkg/agent/endpoints/endpoints.go @@ -36,6 +36,7 @@ type Endpoints struct { workloadAPIServer workload_pb.SpiffeWorkloadAPIServer sdsv3Server secret_v3.SecretDiscoveryServiceServer healthServer grpc_health_v1.HealthServer + apiNames string hooks struct { listening chan struct{} // Hook to signal when the server starts listening @@ -45,12 +46,12 @@ type Endpoints struct { func New(c Config) *Endpoints { attestor := PeerTrackerAttestor{Attestor: c.Attestor} - if c.newWorkloadAPIServer == nil { + if !c.DisableWorkloadAPI && c.newWorkloadAPIServer == nil { c.newWorkloadAPIServer = func(c workload.Config) workload_pb.SpiffeWorkloadAPIServer { return workload.New(c) } } - if c.newSDSv3Server == nil { + if !c.DisableSDSAPI && c.newSDSv3Server == nil { c.newSDSv3Server = func(c sdsv3.Config) secret_v3.SecretDiscoveryServiceServer { return sdsv3.New(c) } @@ -68,28 +69,35 @@ func New(c Config) *Endpoints { workloadRateLimiter := NewWorkloadRateLimiter(c.WorkloadAPIRateLimit, c.Log, c.Metrics) - workloadAPIServer := c.newWorkloadAPIServer(workload.Config{ - Manager: c.Manager, - Attestor: attestor, - RateLimiter: workloadRateLimiter, - AllowUnauthenticatedVerifiers: c.AllowUnauthenticatedVerifiers, - AllowedForeignJWTClaims: allowedClaims, - LogSelectors: c.LogSelectors, - TrustDomain: c.TrustDomain, - }) + var workloadAPIServer workload_pb.SpiffeWorkloadAPIServer + if !c.DisableWorkloadAPI { + workloadAPIServer = c.newWorkloadAPIServer(workload.Config{ + Manager: c.Manager, + Attestor: attestor, + RateLimiter: workloadRateLimiter, + AllowUnauthenticatedVerifiers: c.AllowUnauthenticatedVerifiers, + AllowedForeignJWTClaims: allowedClaims, + LogSelectors: c.LogSelectors, + TrustDomain: c.TrustDomain, + }) + } - sdsv3Server := c.newSDSv3Server(sdsv3.Config{ - Attestor: attestor, - Manager: c.Manager, - RateLimiter: workloadRateLimiter, - DefaultSVIDName: c.DefaultSVIDName, - DefaultBundleName: c.DefaultBundleName, - DefaultAllBundlesName: c.DefaultAllBundlesName, - DisableSPIFFECertValidation: c.DisableSPIFFECertValidation, - }) + var sdsv3Server secret_v3.SecretDiscoveryServiceServer + if !c.DisableSDSAPI { + sdsv3Server = c.newSDSv3Server(sdsv3.Config{ + Attestor: attestor, + Manager: c.Manager, + RateLimiter: workloadRateLimiter, + DefaultSVIDName: c.DefaultSVIDName, + DefaultBundleName: c.DefaultBundleName, + DefaultAllBundlesName: c.DefaultAllBundlesName, + DisableSPIFFECertValidation: c.DisableSPIFFECertValidation, + }) + } healthServer := c.newHealthServer(healthv1.Config{ - Addr: c.BindAddr, + Addr: c.BindAddr, + DisableWorkloadAPI: c.DisableWorkloadAPI, }) return &Endpoints{ @@ -99,6 +107,7 @@ func New(c Config) *Endpoints { workloadAPIServer: workloadAPIServer, sdsv3Server: sdsv3Server, healthServer: healthServer, + apiNames: apiNames(c.DisableWorkloadAPI, c.DisableSDSAPI), hooks: struct { listening chan struct{} }{ @@ -119,8 +128,12 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { grpc.ReadBufferSize(readBufferSize), ) - workload_pb.RegisterSpiffeWorkloadAPIServer(server, e.workloadAPIServer) - secret_v3.RegisterSecretDiscoveryServiceServer(server, e.sdsv3Server) + if e.workloadAPIServer != nil { + workload_pb.RegisterSpiffeWorkloadAPIServer(server, e.workloadAPIServer) + } + if e.sdsv3Server != nil { + secret_v3.RegisterSecretDiscoveryServiceServer(server, e.sdsv3Server) + } grpc_health_v1.RegisterHealthServer(server, e.healthServer) reflection.Register(server) @@ -139,7 +152,7 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { e.log.WithFields(logrus.Fields{ telemetry.Network: e.addr.Network(), telemetry.Address: e.addr, - }).Info("Starting Workload and SDS APIs") + }).Infof("Starting %s", e.apiNames) e.triggerListeningHook() errChan := make(chan error) go func() { errChan <- server.Serve(l) }() @@ -147,7 +160,7 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { select { case err = <-errChan: case <-ctx.Done(): - e.log.Info("Stopping Workload and SDS APIs") + e.log.Infof("Stopping %s", e.apiNames) server.Stop() err = <-errChan if errors.Is(err, grpc.ErrServerStopped) { @@ -157,6 +170,19 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { return err } +func apiNames(disableWorkloadAPI, disableSDSAPI bool) string { + switch { + case !disableWorkloadAPI && !disableSDSAPI: + return "Workload and SDS APIs" + case disableWorkloadAPI && !disableSDSAPI: + return "SDS API" + case !disableWorkloadAPI && disableSDSAPI: + return "Workload API" + default: + return "no APIs" + } +} + func (e *Endpoints) triggerListeningHook() { if e.hooks.listening != nil { e.hooks.listening <- struct{}{} diff --git a/pkg/agent/endpoints/endpoints_test.go b/pkg/agent/endpoints/endpoints_test.go index cb35987ce7..0b1ccec4a6 100644 --- a/pkg/agent/endpoints/endpoints_test.go +++ b/pkg/agent/endpoints/endpoints_test.go @@ -248,6 +248,165 @@ func TestEndpoints(t *testing.T) { } } +func TestEndpointsDisableAPIs(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + baseServices := []string{ + middleware.HealthServiceName, + middleware.ServerReflectionServiceName, + middleware.ServerReflectionV1AlphaServiceName, + } + + for _, tt := range []struct { + name string + disableWorkloadAPI bool + disableSDSAPI bool + expectedServices []string + expectWorkloadCode codes.Code + expectSDSCode codes.Code + expectWorkloadBuilt bool + expectSDSBuilt bool + }{ + { + name: "both APIs enabled", + expectedServices: append(baseServices, middleware.WorkloadAPIServiceName, middleware.EnvoySDSv3ServiceName), + expectWorkloadCode: codes.OK, + expectSDSCode: codes.OK, + expectWorkloadBuilt: true, + expectSDSBuilt: true, + }, + { + name: "workload API disabled", + disableWorkloadAPI: true, + expectedServices: append(baseServices, middleware.EnvoySDSv3ServiceName), + expectWorkloadCode: codes.Unimplemented, + expectSDSCode: codes.OK, + expectWorkloadBuilt: false, + expectSDSBuilt: true, + }, + { + name: "SDS API disabled", + disableSDSAPI: true, + expectedServices: append(baseServices, middleware.WorkloadAPIServiceName), + expectWorkloadCode: codes.OK, + expectSDSCode: codes.Unimplemented, + expectWorkloadBuilt: true, + expectSDSBuilt: false, + }, + { + name: "both APIs disabled", + disableWorkloadAPI: true, + disableSDSAPI: true, + expectedServices: baseServices, + expectWorkloadCode: codes.Unimplemented, + expectSDSCode: codes.Unimplemented, + expectWorkloadBuilt: false, + expectSDSBuilt: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + log, _ := test.NewNullLogger() + metrics := fakemetrics.New() + addr := getTestAddr(t) + var workloadBuilt bool + var sdsBuilt bool + + endpoints := New(Config{ + BindAddr: addr, + Log: log, + Metrics: metrics, + Attestor: FakeAttestor{}, + Manager: FakeManager{}, + DisableWorkloadAPI: tt.disableWorkloadAPI, + DisableSDSAPI: tt.disableSDSAPI, + DefaultSVIDName: "DefaultSVIDName", + DefaultBundleName: "DefaultBundleName", + DefaultAllBundlesName: "DefaultAllBundlesName", + newWorkloadAPIServer: func(c workload.Config) workload_pb.SpiffeWorkloadAPIServer { + workloadBuilt = true + attestor, ok := c.Attestor.(PeerTrackerAttestor) + require.True(t, ok, "attestor was not a PeerTrackerAttestor wrapper") + return FakeWorkloadAPIServer{Attestor: attestor} + }, + newSDSv3Server: func(c sdsv3.Config) secret_v3.SecretDiscoveryServiceServer { + sdsBuilt = true + attestor, ok := c.Attestor.(PeerTrackerAttestor) + require.True(t, ok, "attestor was not a PeerTrackerAttestor wrapper") + return FakeSDSv3Server{Attestor: attestor} + }, + newHealthServer: func(c healthv1.Config) grpc_health_v1.HealthServer { + assert.Equal(t, tt.disableWorkloadAPI, c.DisableWorkloadAPI) + return FakeHealthServer{} + }, + }) + endpoints.hooks.listening = make(chan struct{}) + + serveCtx, stopServing := context.WithCancel(ctx) + defer stopServing() + errCh := make(chan error, 1) + go func() { + errCh <- endpoints.ListenAndServe(serveCtx) + }() + defer func() { + stopServing() + assert.NoError(t, <-errCh) + }() + waitForListening(t, endpoints, errCh) + + target, err := util.GetTargetName(endpoints.addr) + require.NoError(t, err) + conn, err := util.NewGRPCClient(target) + require.NoError(t, err) + defer conn.Close() + + assert.Equal(t, tt.expectWorkloadBuilt, workloadBuilt) + assert.Equal(t, tt.expectSDSBuilt, sdsBuilt) + assert.ElementsMatch(t, tt.expectedServices, listServices(ctx, t, conn)) + + callCtx := metadata.NewOutgoingContext(ctx, metadata.Pairs("workload.spiffe.io", "true")) + wlClient := workload_pb.NewSpiffeWorkloadAPIClient(conn) + _, err = wlClient.FetchJWTSVID(callCtx, &workload_pb.JWTSVIDRequest{}) + if tt.expectWorkloadCode == codes.OK { + require.NoError(t, err) + } else { + require.Equal(t, tt.expectWorkloadCode, status.Code(err)) + } + + sdsClient := secret_v3.NewSecretDiscoveryServiceClient(conn) + _, err = sdsClient.FetchSecrets(ctx, &discovery_v3.DiscoveryRequest{}) + if tt.expectSDSCode == codes.OK { + require.NoError(t, err) + } else { + require.Equal(t, tt.expectSDSCode, status.Code(err)) + } + }) + } +} + +func listServices(ctx context.Context, t *testing.T, conn *grpc.ClientConn) []string { + client := grpc_reflection_v1.NewServerReflectionClient(conn) + clientStream, err := client.ServerReflectionInfo(ctx) + require.NoError(t, err) + + err = clientStream.Send(&grpc_reflection_v1.ServerReflectionRequest{ + MessageRequest: &grpc_reflection_v1.ServerReflectionRequest_ListServices{}, + }) + require.NoError(t, err) + + resp, err := clientStream.Recv() + require.NoError(t, err) + + listResp := resp.GetListServicesResponse() + require.NotNil(t, listResp) + + var serviceNames []string + for _, service := range listResp.Service { + serviceNames = append(serviceNames, service.Name) + } + return serviceNames +} + type FakeManager struct { manager.Manager } From 3bd8c53d00c65ad54c92df73e1ce6fd1600c89c9 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Thu, 9 Jul 2026 23:58:57 +0100 Subject: [PATCH 2/2] Address review comments Signed-off-by: Matheus Pimenta --- doc/spire_agent.md | 10 ++- pkg/agent/agent.go | 3 +- pkg/agent/agent_test.go | 90 +++++++++++++++++++ pkg/agent/endpoints/endpoints.go | 6 +- pkg/agent/endpoints/endpoints_test.go | 3 +- .../suites/evict-agent/10-start-agent | 2 +- 6 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 pkg/agent/agent_test.go diff --git a/doc/spire_agent.md b/doc/spire_agent.md index b83012f877..258f88ee5f 100644 --- a/doc/spire_agent.md +++ b/doc/spire_agent.md @@ -49,6 +49,8 @@ This may be useful for templating configuration files, for example across differ | `allowed_foreign_jwt_claims` | List of trusted claims to be returned when validating foreign JWTSVIDs | | | `authorized_delegates` | A SPIFFE ID list of the authorized delegates. See [Delegated Identity API](#delegated-identity-api) for more information | | | `data_dir` | A directory the agent can use for its runtime data | $PWD | +| `disable_sds_api` | Disable the Envoy SDS API. The public socket or named pipe remains exposed if the Workload API is enabled. | false | +| `disable_workload_api` | Disable the SPIFFE Workload API. The public socket or named pipe remains exposed if SDS is enabled. | false | | `experimental` | The experimental options that are subject to change or removal (see below) | | | `insecure_bootstrap` | If true, the agent bootstraps without verifying the server's identity | false | | `rebootstrap_mode` | Can be one of 'never', 'auto', or 'always' | never | @@ -67,8 +69,6 @@ This may be useful for templating configuration files, for example across differ | `server_address` | DNS name or IP address of the SPIRE server | | | `server_port` | Port number of the SPIRE server | | | `socket_path` | Location to bind the SPIRE Agent Workload API and SDS socket (Unix only). The socket is exposed unless both `disable_workload_api` and `disable_sds_api` are `true`. | /tmp/spire-agent/public/api.sock | -| `disable_workload_api` | Disable the SPIFFE Workload API. The public socket or named pipe remains exposed if SDS is enabled. | false | -| `disable_sds_api` | Disable the Envoy SDS API. The public socket or named pipe remains exposed if the Workload API is enabled. | false | | `sds` | Optional SDS configuration section | | | `trust_bundle_path` | Path to the SPIRE server CA bundle | | | `trust_bundle_url` | URL to download the initial SPIRE server trust bundle | | @@ -303,14 +303,14 @@ the following flags are available: | `-allowUnauthenticatedVerifiers` | Allow agent to release trust bundles to unauthenticated verifiers | | | `-config` | Path to a SPIRE config file | conf/agent/agent.conf | | `-dataDir` | A directory the agent can use for its runtime data | | +| `-disableSDSAPI` | Disable the Envoy SDS API | false | +| `-disableWorkloadAPI` | Disable the SPIFFE Workload API | false | | `-expandEnv` | Expand environment $VARIABLES in the config file | | | `-joinToken` | An optional token which has been generated by the SPIRE server | | | `-joinTokenFile` | Path to a file containing an optional join token which has been generated by the SPIRE server | | | `-logFile` | File to write logs to | | | `-logFormat` | Format of logs, <text|json> | | | `-logLevel` | DEBUG, INFO, WARN or ERROR | | -| `-disableWorkloadAPI` | Disable the SPIFFE Workload API | false | -| `-disableSDSAPI` | Disable the Envoy SDS API | false | | `-serverAddress` | IP address or DNS name of the SPIRE server | | | `-serverPort` | Port number of the SPIRE server | | | `-socketPath` | Location to bind the Workload API and SDS socket | | @@ -393,6 +393,8 @@ Attaches to the workload API and watches for X509-SVID updates, printing details Checks SPIRE agent's health. +This command connects to the public Workload API/SDS endpoint. If both `disable_workload_api` and `disable_sds_api` are `true`, use the `health_checks` HTTP listener for liveness and readiness probes instead. + | Command | Action | Default | |:--------------|:--------------------------------------|:---------------------------------| | `-shallow` | Perform a less stringent health check | | diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index b4737d373e..c58e437b9f 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -289,6 +289,7 @@ func (a *Agent) Run(ctx context.Context) error { }() tasks = append(tasks, agentEndpoints.ListenAndServe) } else { + a.c.Log.WithField("apis", "Workload and SDS APIs").Info("Skipping agent APIs because public endpoint is disabled") a.started = true close(readyForHealthChecks) } @@ -547,7 +548,7 @@ func (a *Agent) newAdminEndpoints(metrics telemetry.Metrics, mgr manager.Manager // CheckHealth is used as a top-level health check for the agent. func (a *Agent) CheckHealth() health.State { - if a.c.BindAddress == nil || a.c.DisableWorkloadAPI { + if a.c.BindAddress == nil { return health.State{ Started: &a.started, Ready: a.started, diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go new file mode 100644 index 0000000000..62bb1ca251 --- /dev/null +++ b/pkg/agent/agent_test.go @@ -0,0 +1,90 @@ +package agent + +import ( + "net" + "testing" + + workload_pb "github.com/spiffe/go-spiffe/v2/proto/spiffe/workload" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/spiffe/spire/test/spiretest" +) + +func TestCheckHealth(t *testing.T) { + for _, tt := range []struct { + name string + disableWorkloadAPI bool + started bool + setupBindAddress func(t *testing.T) net.Addr + expectReady bool + expectLive bool + expectErr string + }{ + { + name: "public endpoint disabled", + started: true, + expectReady: true, + expectLive: true, + }, + { + name: "workload API disabled with serving endpoint", + disableWorkloadAPI: true, + started: true, + setupBindAddress: func(t *testing.T) net.Addr { + return spiretest.StartGRPCServer(t, func(s *grpc.Server) {}) + }, + expectReady: true, + expectLive: true, + }, + { + name: "workload API disabled with unavailable endpoint", + disableWorkloadAPI: true, + started: true, + setupBindAddress: func(t *testing.T) net.Addr { + return spiretest.StartWorkloadAPI(t, unavailableWorkloadAPI{}) + }, + expectReady: false, + expectLive: false, + expectErr: "workload api is unavailable", + }, + } { + t.Run(tt.name, func(t *testing.T) { + c := &Config{ + DisableWorkloadAPI: tt.disableWorkloadAPI, + } + if tt.setupBindAddress != nil { + c.BindAddress = tt.setupBindAddress(t) + } + + a := New(c) + a.started = tt.started + + state := a.CheckHealth() + require.NotNil(t, state.Started) + require.Equal(t, tt.started, *state.Started) + require.Equal(t, tt.expectReady, state.Ready) + require.Equal(t, tt.expectLive, state.Live) + + if tt.expectErr == "" { + return + } + require.Equal(t, agentHealthDetails{ + WorkloadAPIErr: tt.expectErr, + }, state.ReadyDetails) + require.Equal(t, agentHealthDetails{ + WorkloadAPIErr: tt.expectErr, + }, state.LiveDetails) + }) + } +} + +type unavailableWorkloadAPI struct { + workload_pb.UnimplementedSpiffeWorkloadAPIServer +} + +func (unavailableWorkloadAPI) FetchX509Bundles(_ *workload_pb.X509BundlesRequest, _ workload_pb.SpiffeWorkloadAPI_FetchX509BundlesServer) error { + return status.Error(codes.Unavailable, "") +} diff --git a/pkg/agent/endpoints/endpoints.go b/pkg/agent/endpoints/endpoints.go index e7e68b0629..bd963978db 100644 --- a/pkg/agent/endpoints/endpoints.go +++ b/pkg/agent/endpoints/endpoints.go @@ -22,6 +22,7 @@ import ( const ( readBufferSize = 4096 + apiNamesField = "apis" ) type Server interface { @@ -152,7 +153,8 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { e.log.WithFields(logrus.Fields{ telemetry.Network: e.addr.Network(), telemetry.Address: e.addr, - }).Infof("Starting %s", e.apiNames) + apiNamesField: e.apiNames, + }).Info("Starting agent APIs") e.triggerListeningHook() errChan := make(chan error) go func() { errChan <- server.Serve(l) }() @@ -160,7 +162,7 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { select { case err = <-errChan: case <-ctx.Done(): - e.log.Infof("Stopping %s", e.apiNames) + e.log.WithField(apiNamesField, e.apiNames).Info("Stopping agent APIs") server.Stop() err = <-errChan if errors.Is(err, grpc.ErrServerStopped) { diff --git a/pkg/agent/endpoints/endpoints_test.go b/pkg/agent/endpoints/endpoints_test.go index 0b1ccec4a6..4a0216de66 100644 --- a/pkg/agent/endpoints/endpoints_test.go +++ b/pkg/agent/endpoints/endpoints_test.go @@ -236,9 +236,10 @@ func TestEndpoints(t *testing.T) { spiretest.AssertLogs(t, hook.AllEntries(), append([]spiretest.LogEntry{ { Level: logrus.InfoLevel, - Message: "Starting Workload and SDS APIs", + Message: "Starting agent APIs", Data: logrus.Fields{ "address": endpoints.addr.String(), + "apis": "Workload and SDS APIs", "network": addr.Network(), }, }, diff --git a/test/integration/suites/evict-agent/10-start-agent b/test/integration/suites/evict-agent/10-start-agent index 1597a12e14..f280450da5 100755 --- a/test/integration/suites/evict-agent/10-start-agent +++ b/test/integration/suites/evict-agent/10-start-agent @@ -12,7 +12,7 @@ CHECKINTERVAL=1 for ((i=1;i<=MAXCHECKS;i++)); do log-info "checking that the agent is back up ($i of $MAXCHECKS max)..." docker compose logs spire-agent - if docker compose logs spire-agent | grep "Starting Workload and SDS APIs"; then + if docker compose logs spire-agent | grep "Starting agent APIs"; then exit 0 fi sleep "${CHECKINTERVAL}"