Skip to content

Commit

Permalink
server: Fix panic in /health?bundle=true
Browse files Browse the repository at this point in the history
The issue was that with bundles loaded from the file system we would
not initialize the mutex used for checking bundle status.

This fixes the initialization and prevents the error. Health status
works as expected now.

Fixes: open-policy-agent#1703
Signed-off-by: Patrick East <east.patrick@gmail.com>
  • Loading branch information
patrick-east committed Aug 29, 2019
1 parent c47f9da commit 3248c2d
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 15 deletions.
3 changes: 1 addition & 2 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ type Server struct {
runtime *ast.Term
httpListeners []httpListener
bundleStatuses map[string]*bundlePlugin.Status
bundleStatusMtx *sync.RWMutex
bundleStatusMtx sync.RWMutex
metrics Metrics
}

Expand Down Expand Up @@ -176,7 +176,6 @@ func (s *Server) Init(ctx context.Context) (*Server, error) {

bp := bundlePlugin.Lookup(s.manager)
if bp != nil {
s.bundleStatusMtx = new(sync.RWMutex)

// initialize statuses to empty defaults for server /health check
s.bundleStatuses = map[string]*bundlePlugin.Status{}
Expand Down
38 changes: 25 additions & 13 deletions server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"reflect"
"sort"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -81,7 +80,6 @@ func TestUnversionedGetHealthCheckBundleActivationSingle(t *testing.T) {
// Initialize the server as if a bundle plugin was
// configured on the manager.
f.server.manager.Register(pluginBundle.Name, &pluginBundle.Plugin{})
f.server.bundleStatusMtx = new(sync.RWMutex)
f.server.bundleStatuses = map[string]*pluginBundle.Status{
bundleName: &pluginBundle.Status{Name: bundleName},
}
Expand All @@ -106,6 +104,31 @@ func TestUnversionedGetHealthCheckBundleActivationSingle(t *testing.T) {
}
}

func TestUnversionedGetHealthCheckBundleActivationSingleLegacy(t *testing.T) {

// Initialize the server as if there is no bundle plugin

f := newFixture(t)

ctx := context.Background()

err := storage.Txn(ctx, f.server.store, storage.WriteParams, func(txn storage.Transaction) error {
return bundle.LegacyWriteManifestToStore(ctx, f.server.store, txn, bundle.Manifest{
Revision: "a",
})
})

if err != nil {
t.Fatalf("Unexpected error: %s", err)
}

// The heath check should now respond as healthy
req := newReqUnversioned(http.MethodGet, "/health?bundle=true", "")
if err := f.executeRequest(req, 200, `{}`); err != nil {
t.Fatal(err)
}
}

func TestUnversionedGetHealthCheckBundleActivationMulti(t *testing.T) {

f := newFixture(t)
Expand All @@ -118,7 +141,6 @@ func TestUnversionedGetHealthCheckBundleActivationMulti(t *testing.T) {
"b3": {Service: "s3", Resource: "bundle.tar.gz"},
}}, f.server.manager)
f.server.manager.Register(pluginBundle.Name, bp)
f.server.bundleStatusMtx = new(sync.RWMutex)
f.server.bundleStatuses = map[string]*pluginBundle.Status{
"b1": {Name: "b1"},
"b2": {Name: "b2"},
Expand Down Expand Up @@ -187,10 +209,6 @@ func TestInitWithBundlePlugin(t *testing.T) {
t.Error("server.hasBundle should be true")
}

if server.bundleStatusMtx == nil {
t.Error("server.bundleStatusMtx should be initialized")
}

isActivated := server.bundlesActivated()
if isActivated {
t.Error("bundle should not be initialized to activated status")
Expand Down Expand Up @@ -225,10 +243,6 @@ func TestInitWithBundlePluginMultiBundle(t *testing.T) {
t.Error("server.hasBundle should be true")
}

if server.bundleStatusMtx == nil {
t.Error("server.bundleStatusMtx should be initialized")
}

isActivated := server.bundlesActivated()
if isActivated {
t.Error("bundle should not be initialized to activated")
Expand Down Expand Up @@ -1662,7 +1676,6 @@ func TestDataProvenanceSingleBundle(t *testing.T) {
// Initialize as if a bundle plugin is running
bp := pluginBundle.New(&pluginBundle.Config{Name: "b1"}, f.server.manager)
f.server.manager.Register(pluginBundle.Name, bp)
f.server.bundleStatusMtx = new(sync.RWMutex)
f.server.bundleStatuses = map[string]*pluginBundle.Status{
"b1": {Name: "b1"},
}
Expand Down Expand Up @@ -1774,7 +1787,6 @@ func TestDataProvenanceMultiBundle(t *testing.T) {
}}, f.server.manager)
f.server.manager.Register(pluginBundle.Name, bp)

f.server.bundleStatusMtx = new(sync.RWMutex)
f.server.bundleStatuses = map[string]*pluginBundle.Status{
"b1": {Name: "b1"},
"b2": {Name: "b2"},
Expand Down

0 comments on commit 3248c2d

Please sign in to comment.