Skip to content

Commit

Permalink
server/server: exclude-plugin param in /health route
Browse files Browse the repository at this point in the history
- 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 <gshively@godaddy.com>
  • Loading branch information
gshively11 authored and tsandall committed Aug 12, 2021
1 parent 6369788 commit 727140a
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 1 deletion.
9 changes: 9 additions & 0 deletions docs/content/rest-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
26 changes: 25 additions & 1 deletion server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
101 changes: 101 additions & 0 deletions server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions server/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 727140a

Please sign in to comment.