Implement the readiness endpoint for health checking

conf/agent/agent_full.conf

@@ -1,5 +1,5 @@
 # This is the SPIRE Agent configuration file including all possible configuration
-# options. 
+# options.
 
 # agent: Contains core configuration parameters.
 agent {
@@ -24,16 +24,16 @@ agent {
 
     # server_address: DNS name or IP address of the SPIRE server.
     server_address = "127.0.0.1"
-    
+
     # server_port: Port number of the SPIRE server.
     server_port = "8081"
-    
+
     # socket_path: Location to bind the workload API socket. Default: /tmp/agent.sock.
     socket_path = "/tmp/agent.sock"
-    
+
     # trust_bundle_path: Path to the SPIRE server CA bundle.
     trust_bundle_path = "./conf/agent/dummy_root_ca.crt"
-    
+
     # trust_bundle_url: URL to download the initial SPIRE server trust bundle.
     # trust_bundle_url = ""
 
@@ -56,9 +56,9 @@ agent {
 # Each nested object has the following format:
 #
 #     PluginType "plugin_name" {
-#    
+#
 #         # plugin_cmd: Path to the plugin implementation binary (optional, not
-#         # needed for built-ins)    
+#         # needed for built-ins)
 #         plugin_cmd = <string>
 #
 #         # plugin_checksum: An optional sha256 of the plugin binary (optional,
@@ -149,7 +149,7 @@ plugins {
             # cluster: Name of the cluster. It must correspond to a cluster
             # configured in the server plugin.
             # cluster = ""
-            
+
             # token_path: Path to the service account token on disk.
             # Default: /run/secrets/kubernetes.io/serviceaccount/token.
             # token_path = "/run/secrets/kubernetes.io/serviceaccount/token"
@@ -180,7 +180,7 @@ plugins {
             # certificate_path: The path to the certificate bundle on disk. The
             # file must contain one or more PEM blocks, starting with the identity
             # certificate followed by any intermediate certificates necessary for
-            # chain-of-trust validation.	
+            # chain-of-trust validation.
             # certificate_path = ""
 
             # intermediates_path: Optional. The path to a chain of intermediate
@@ -204,7 +204,7 @@ plugins {
             # docker_version = ""
         }
     }
-    
+
     # WorkloadAttestor "k8s": A workload attestor which allows selectors based
     # on Kubernetes constructs such ns (namespace) and sa (service account).
     WorkloadAttestor "k8s" {
@@ -244,7 +244,7 @@ plugins {
             # node_name_env = "MY_NODE_NAME"
 
             # node_name: The name of the node. Overrides the value obtained by
-            # the environment variable specified by node_name_env.            
+            # the environment variable specified by node_name_env.
             # node_name = ""
         }
     }
@@ -276,7 +276,7 @@ plugins {
 #         port = 9988
 #     }
 
-#     DogStatsd = [ 
+#     DogStatsd = [
 #         # List of DogStatsd addresses.
 #         { address = "localhost:8125" },
 #         { address = "collector.example.org:1337" },
@@ -301,7 +301,7 @@ plugins {
 # }
 
 # health_checks: If health checking is desired use this section to configure
-# and expose an additional server endpoint for such purpose.
+# and expose an additional agent endpoint for such purpose.
 # health_checks {
 #     # listener_enabled: Enables health checks endpoint.
 #     listener_enabled = true
@@ -312,9 +312,12 @@ plugins {
 #     # bind_port: HTTP Port number of the health checks endpoint. Default: 80.
 #     # bind_port = "80"
 
-#     # live_path: HTTP resource path for checking server liveness. Default: /live.
+#     # live_path: HTTP resource path for checking agent liveness. Default: /live.
 #     # live_path = "/live"
 
-#     # ready_path: HTTP resource path for checking server readiness. Default: /ready.
+#     # ready_path: HTTP resource path for checking agent readiness. Default: /ready.
 #     # ready_path = "/ready"
+
+#     # checking_readiness_interval: Interval for checking agent readiness. Default: 1m.
+#     # checking_readiness_interval = "1m"
 # }

conf/server/server_full.conf

@@ -694,4 +694,7 @@ plugins {
 
 #     # ready_path: HTTP resource path for checking server readiness. Default: /ready.
 #     # ready_path = "/ready"
+
+#     # checking_readiness_interval: Interval for checking server readiness. Default: 1m.
+#     # checking_readiness_interval = "1m"
 # }

doc/spire_agent.md

@@ -112,6 +112,7 @@ health_checks {
         bind_port = "80"
         live_path = "/live"
         ready_path = "/ready"
+        checking_readiness_interval = "1m"
 }
 ```
 

doc/spire_server.md

@@ -188,6 +188,7 @@ health_checks {
         bind_port = "80"
         live_path = "/live"
         ready_path = "/ready"
+        checking_readiness_interval = "1m"
 }
 ```
 

pkg/agent/agent.go

@@ -2,16 +2,17 @@ package agent
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	_ "net/http/pprof" //nolint: gosec // import registers routes on DefaultServeMux
 	"os"
 	"path"
 	"runtime"
 	"sync"
-	"time"
 
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
+	api_workload "github.com/spiffe/spire/api/workload"
 	admin_api "github.com/spiffe/spire/pkg/agent/api"
 	node_attestor "github.com/spiffe/spire/pkg/agent/attestor/node"
 	workload_attestor "github.com/spiffe/spire/pkg/agent/attestor/workload"
@@ -30,6 +31,8 @@ import (
 	"github.com/spiffe/spire/proto/spire/api/server/bundle/v1"
 	_ "golang.org/x/net/trace" // registers handlers on the DefaultServeMux
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 type Agent struct {
@@ -97,7 +100,7 @@ func (a *Agent) Run(ctx context.Context) error {
 
 	endpoints := a.newEndpoints(cat, metrics, manager)
 
-	if err := healthChecks.AddCheck("agent", a, time.Minute); err != nil {
+	if err := healthChecks.AddCheck("agent", a); err != nil {
 		return fmt.Errorf("failed adding healthcheck: %v", err)
 	}
 
@@ -261,5 +264,23 @@ func (a *Agent) agentSVIDPath() string {
 
 // Status is used as a top-level health check for the Agent.
 func (a *Agent) Status() (interface{}, error) {
-	return nil, nil
+	client := api_workload.NewX509Client(&api_workload.X509ClientConfig{
+		Addr:        a.c.BindAddress,
+		FailOnError: true,
+	})
+	defer client.Stop()
+
+	errCh := make(chan error, 1)
+	go func() {
+		errCh <- client.Start()
+	}()
+
+	err := <-errCh
+	if status.Code(err) == codes.Unavailable {
+		return nil, errors.New("workload api is unavailable") //nolint: golint // error is (ab)used for CLI output
+	}
+
+	return health.Details{
+		Message: "successfully created a workload api client to fetch x509 svid",
+	}, nil
 }

pkg/common/health/config.go

@@ -15,9 +15,27 @@ type Config struct {
 	ReadyPath string `hcl:"ready_path"`
 	LivePath  string `hcl:"live_path"`
 
+	// Interval for checking readiness
+	CheckingReadinessInterval string `hcl:"checking_readiness_interval"`
+
 	UnusedKeys []string `hcl:",unusedKeys"`
 }
 
+// getAddress returns an address suitable for use as http.Server.Addr.
+func (c *Config) getAddress() string {
+	host := "localhost"
+	if c.BindAddress != "" {
+		host = c.BindAddress
+	}
+
+	port := "80"
+	if c.BindPort != "" {
+		port = c.BindPort
+	}
+
+	return fmt.Sprintf("%s:%s", host, port)
+}
+
 // getReadyPath returns the configured value or a default
 func (c *Config) getReadyPath() string {
 	if c.ReadyPath == "" {
@@ -36,17 +54,16 @@ func (c *Config) getLivePath() string {
 	return c.LivePath
 }
 
-// getAddress returns an address suitable for use as http.Server.Addr.
-func (c *Config) getAddress() string {
-	host := "localhost"
-	if c.BindAddress != "" {
-		host = c.BindAddress
+// getCheckingReadinessInterval returns the configured value or a default
+func (c *Config) getCheckingReadinessInterval() string {
+	if c.CheckingReadinessInterval == "" {
+		return "1m"
 	}
 
-	port := "80"
-	if c.BindPort != "" {
-		port = c.BindPort
-	}
+	return c.CheckingReadinessInterval
+}
 
-	return fmt.Sprintf("%s:%s", host, port)
+// Details are additional data to be used when the system is ready
+type Details struct {
+	Message string `json:"message,omitempty"`
 }

pkg/common/health/health.go

@@ -2,6 +2,7 @@ package health
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"sync"
 	"time"
@@ -46,16 +47,22 @@ func NewChecker(config Config, log logrus.FieldLogger) *Checker {
 		}
 	}
 
-	hc.StatusListener = &statusListener{}
-	hc.Logger = &logadapter{FieldLogger: log.WithField(telemetry.SubsystemName, "health")}
+	l := log.WithField(telemetry.SubsystemName, "health")
+	hc.StatusListener = &statusListener{log: l}
+	hc.Logger = &logadapter{FieldLogger: l}
 
 	return &Checker{config: config, server: server, hc: hc, log: log}
 }
 
-func (c *Checker) AddCheck(name string, checker health.ICheckable, interval time.Duration) error {
+func (c *Checker) AddCheck(name string, checker health.ICheckable) error {
 	c.mutex.Lock()
 	defer c.mutex.Unlock()
 
+	interval, err := time.ParseDuration(c.config.getCheckingReadinessInterval())
+	if err != nil {
+		return fmt.Errorf("could not parse interval for checking readiness %q: %v", c.config.getCheckingReadinessInterval(), err)
+	}
+
 	return c.hc.AddCheck(&health.Config{
 		Name:     name,
 		Checker:  checker,

pkg/server/server.go

@@ -2,17 +2,18 @@ package server
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	_ "net/http/pprof" //nolint: gosec // import registers routes on DefaultServeMux
 	"net/url"
 	"os"
 	"runtime"
 	"sync"
-	"time"
 
 	"github.com/andres-erbsen/clock"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
+	server_util "github.com/spiffe/spire/cmd/spire-server/util"
 	"github.com/spiffe/spire/pkg/common/health"
 	"github.com/spiffe/spire/pkg/common/hostservices/metricsservice"
 	common_services "github.com/spiffe/spire/pkg/common/plugin/hostservices"
@@ -30,6 +31,7 @@ import (
 	"github.com/spiffe/spire/pkg/server/plugin/hostservices"
 	"github.com/spiffe/spire/pkg/server/registration"
 	"github.com/spiffe/spire/pkg/server/svid"
+	"github.com/spiffe/spire/proto/spire/api/server/bundle/v1"
 	"google.golang.org/grpc"
 )
 
@@ -159,7 +161,7 @@ func (s *Server) run(ctx context.Context) (err error) {
 
 	registrationManager := s.newRegistrationManager(cat, metrics)
 
-	if err := healthChecks.AddCheck("server", s, time.Minute); err != nil {
+	if err := healthChecks.AddCheck("server", s); err != nil {
 		return fmt.Errorf("failed adding healthcheck: %v", err)
 	}
 
@@ -385,5 +387,21 @@ func (s *Server) validateTrustDomain(ctx context.Context, ds datastore.DataStore
 
 // Status is used as a top-level health check for the Server.
 func (s *Server) Status() (interface{}, error) {
-	return nil, nil
+	client, err := server_util.NewServerClient(s.config.BindUDSAddress.Name)
+	if err != nil {
+		return nil, errors.New("cannot create registration client")
+	}
+	bundleClient := client.NewBundleClient()
+
+	// Currently using the ability to fetch a bundle as the health check. This
+	// **could** be problematic if the Upstream CA signing process is lengthy.
+	// As currently coded however, the registration API isn't served until after
+	// the server CA has been signed by upstream.
+	if _, err := bundleClient.GetBundle(context.Background(), &bundle.GetBundleRequest{}); err != nil {
+		return nil, errors.New("unable to fetch bundle")
+	}
+
+	return health.Details{
+		Message: "successfully fetched bundle",
+	}, nil
 }

test/integration/suites/k8s-reconcile/conf/agent/spire-agent.yaml

@@ -78,6 +78,15 @@ data:
       }
     }
 
+    health_checks {
+      listener_enabled = true
+      bind_address = "0.0.0.0"
+      bind_port = "80"
+      live_path = "/live"
+      ready_path = "/ready"
+      checking_readiness_interval = "5s"
+    }
+
 ---
 
 apiVersion: apps/v1
@@ -129,13 +138,15 @@ spec:
             - name: spire-token
               mountPath: /var/run/secrets/tokens
           livenessProbe:
-            exec:
-              command: ["/opt/spire/bin/spire-agent", "healthcheck", "-socketPath", "/run/spire/sockets/agent.sock"]
+            httpGet:
+              path: /live
+              port: 80
             initialDelaySeconds: 10
             periodSeconds: 10
           readinessProbe:
-            exec:
-              command: ["/opt/spire/bin/spire-agent", "healthcheck", "-socketPath", "/run/spire/sockets/agent.sock", "--shallow"]
+            httpGet:
+              path: /ready
+              port: 80
             initialDelaySeconds: 10
             periodSeconds: 10
       volumes:
@@ -151,8 +162,8 @@ spec:
             type: DirectoryOrCreate
         - name: spire-token
           projected:
-           sources:
-           - serviceAccountToken:
-              path: spire-agent
-              expirationSeconds: 7200
-              audience: spire-server
+            sources:
+            - serviceAccountToken:
+                path: spire-agent
+                expirationSeconds: 7200
+                audience: spire-server