From 727140a8e66b3a252c37a3bf145f75b83653b239 Mon Sep 17 00:00:00 2001 From: Grant Shively Date: Fri, 6 Aug 2021 12:40:10 -0700 Subject: [PATCH] server/server: exclude-plugin param in /health route - Added a new querystring param to the /health route: `exclude-plugin` - Can be specified multiple times - Value is the name of a plugin that should be excluded from the OK status check performed when the `plugins` qs param is specified Fixes #3713 Signed-off-by: Grant Shively --- docs/content/rest-api.md | 9 ++++ server/server.go | 26 +++++++++- server/server_test.go | 101 +++++++++++++++++++++++++++++++++++++++ server/types/types.go | 5 ++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/docs/content/rest-api.md b/docs/content/rest-api.md index df48ace041..5449fad479 100644 --- a/docs/content/rest-api.md +++ b/docs/content/rest-api.md @@ -1787,6 +1787,10 @@ that the server is operational. Optionally it can account for bundle activation `bundles` - Boolean parameter to account for bundle activation status in response. This includes any discovery bundles or bundles defined in the loaded discovery configuration. `plugins` - Boolean parameter to account for plugin status in response. +`exclude-plugin` - String parameter to exclude a plugin from status checks. Can be added multiple + times. Does nothing if `plugins` is not true. This parameter is useful for special use cases + where a plugin depends on the server being fully initialized before it can fully intialize + itself. #### Status Codes - **200** - OPA service is healthy. If the `bundles` option is specified then all configured bundles have @@ -1814,6 +1818,11 @@ GET /health?bundles HTTP/1.1 GET /health?plugins HTTP/1.1 ``` +#### Example Request (plugin status with exclude) +```http +GET /health?plugins&exclude-plugin=decision-logs&exclude-plugin=status HTTP/1.1 +``` + #### Healthy Response ```http HTTP/1.1 200 OK diff --git a/server/server.go b/server/server.go index d5e3c1be2f..b9fec52b34 100644 --- a/server/server.go +++ b/server/server.go @@ -1016,6 +1016,11 @@ func (s *Server) unversionedGetHealth(w http.ResponseWriter, r *http.Request) { includeBundleStatus := getBoolParam(r.URL, types.ParamBundleActivationV1, true) || getBoolParam(r.URL, types.ParamBundlesActivationV1, true) includePluginStatus := getBoolParam(r.URL, types.ParamPluginsV1, true) + excludePlugin := getStringSliceParam(r.URL, types.ParamExcludePluginV1) + excludePluginMap := map[string]struct{}{} + for _, name := range excludePlugin { + excludePluginMap[name] = struct{}{} + } // Ensure the server can evaluate a simple query if !s.canEval(ctx) { @@ -1037,7 +1042,10 @@ func (s *Server) unversionedGetHealth(w http.ResponseWriter, r *http.Request) { if includePluginStatus { // Ensure that all plugins (if requested to be included in the result) have an OK status. hasErr := false - for _, status := range pluginStatuses { + for name, status := range pluginStatuses { + if _, exclude := excludePluginMap[name]; exclude { + continue + } if status != nil && status.State != plugins.StateOK { hasErr = true break @@ -2410,6 +2418,22 @@ func getBoolParam(url *url.URL, name string, ifEmpty bool) bool { return false } +func getStringSliceParam(url *url.URL, name string) []string { + + p, ok := url.Query()[name] + if !ok { + return nil + } + + // Query params w/o values are represented as slice (of len 1) with an + // empty string. + if len(p) == 1 && p[0] == "" { + return nil + } + + return p +} + func getExplain(p []string, zero types.ExplainModeV1) types.ExplainModeV1 { for _, x := range p { switch x { diff --git a/server/server_test.go b/server/server_test.go index d2eb2df100..d576e0684e 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -348,6 +348,107 @@ func TestUnversionedGetHealthCheckDiscoveryWithPlugins(t *testing.T) { } } +func TestUnversionedGetHealthCheckDiscoveryWithPluginsAndExclude(t *testing.T) { + + // Use the same server through the cases, the status updates apply incrementally to it. + f := newFixture(t) + + cases := []struct { + note string + statusUpdates map[string]*plugins.Status + exp int + }{ + { + note: "no plugins configured", + statusUpdates: nil, + exp: 200, + }, + { + note: "one plugin configured - not ready", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateNotReady}, + }, + exp: 500, + }, + { + note: "one plugin configured - ready", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateOK}, + }, + exp: 200, + }, + { + note: "one plugin configured - error state", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateErr}, + }, + exp: 500, + }, + { + note: "one plugin configured - recovered from error", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateOK}, + }, + exp: 200, + }, + { + note: "add excluded plugin - not ready", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateOK}, + "p2": {State: plugins.StateNotReady}, + }, + exp: 200, + }, + { + note: "add another excluded plugin - not ready", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateOK}, + "p2": {State: plugins.StateNotReady}, + "p3": {State: plugins.StateNotReady}, + }, + exp: 200, + }, + { + note: "excluded plugin - error", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateOK}, + "p2": {State: plugins.StateErr}, + "p3": {State: plugins.StateErr}, + }, + exp: 200, + }, + { + note: "first plugin - error", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateErr}, + "p2": {State: plugins.StateErr}, + "p3": {State: plugins.StateErr}, + }, + exp: 500, + }, + { + note: "all plugins ready", + statusUpdates: map[string]*plugins.Status{ + "p1": {State: plugins.StateOK}, + "p2": {State: plugins.StateOK}, + "p3": {State: plugins.StateOK}, + }, + exp: 200, + }, + } + + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + for name, status := range tc.statusUpdates { + f.server.manager.UpdatePluginStatus(name, status) + } + + req := newReqUnversioned(http.MethodGet, "/health?plugins&exclude-plugin=p2&exclude-plugin=p3", "") + validateDiagnosticRequest(t, f, req, tc.exp, `{}`) + }) + } +} + func TestUnversionedGetHealthCheckBundleAndPlugins(t *testing.T) { cases := []struct { diff --git a/server/types/types.go b/server/types/types.go index 2ca2cc693a..5347ab4b5f 100644 --- a/server/types/types.go +++ b/server/types/types.go @@ -431,6 +431,11 @@ const ( // of the health API. ParamPluginsV1 = "plugins" + // ParamExcludePluginV1 defines the name of the HTTP URL parameter that + // indicates the client wants to exclude plugin status in the results + // of the health API for the specified plugin(s) + ParamExcludePluginV1 = "exclude-plugin" + // ParamStrictBuiltinErrors names the HTTP URL parameter that indicates the client // wants built-in function errors to be treated as fatal. ParamStrictBuiltinErrors = "strict-builtin-errors"