Skip to content

Commit

Permalink
bundle/status: Include bundle type in status information
Browse files Browse the repository at this point in the history
OPA has support for Delta Bundles. The status object already
contains valuable information such as last activation timestamp but
does not specify if the bundle was a canonical snapshot or delta.

This change updates the bundle.Status object to include the
bundle type string: either "snapshot" or "delta". This can be useful
for status endpoints to differentiate between the bundle types.

Issue: 4477

Signed-off-by: Bryan Fulton <bryan@styra.com>
  • Loading branch information
bryan-styra authored and ashutosh-narkar committed Apr 27, 2022
1 parent 654b245 commit 02c1c1e
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 10 deletions.
2 changes: 2 additions & 0 deletions plugins/bundle/plugin.go
Expand Up @@ -375,6 +375,7 @@ func (p *Plugin) loadAndActivateBundlesFromDisk(ctx context.Context) {
numActivatedBundles := 0
for name, b := range persistedBundles {
p.status[name].Metrics = metrics.New()
p.status[name].Type = b.Type()

err := p.activate(ctx, name, b)
if err != nil {
Expand Down Expand Up @@ -474,6 +475,7 @@ func (p *Plugin) process(ctx context.Context, name string, u download.Update) {
p.status[name].LastSuccessfulRequest = p.status[name].LastRequest

if u.Bundle != nil {
p.status[name].Type = u.Bundle.Type()
p.status[name].LastSuccessfulDownload = p.status[name].LastSuccessfulRequest

p.status[name].Metrics.Timer(metrics.RegoLoadBundles).Start()
Expand Down
12 changes: 12 additions & 0 deletions plugins/bundle/plugin_test.go
Expand Up @@ -71,6 +71,12 @@ func TestPluginOneShot(t *testing.T) {

ensurePluginState(t, plugin, plugins.StateOK)

if status, ok := plugin.status[bundleName]; !ok {
t.Fatalf("Expected to find status for %s, found nil", bundleName)
} else if status.Type != bundle.SnapshotBundleType {
t.Fatalf("expected snapshot bundle but got %v", status.Type)
}

txn := storage.NewTransactionOrDie(ctx, manager.Store)
defer manager.Store.Abort(ctx, txn)

Expand Down Expand Up @@ -262,6 +268,12 @@ func TestPluginOneShotDeltaBundle(t *testing.T) {

ensurePluginState(t, plugin, plugins.StateOK)

if status, ok := plugin.status[bundleName]; !ok {
t.Fatalf("Expected to find status for %s, found nil", bundleName)
} else if status.Type != bundle.DeltaBundleType {
t.Fatalf("expected delta bundle but got %v", status.Type)
}

txn := storage.NewTransactionOrDie(ctx, manager.Store)
defer manager.Store.Abort(ctx, txn)

Expand Down
1 change: 1 addition & 0 deletions plugins/bundle/status.go
Expand Up @@ -26,6 +26,7 @@ type Status struct {
Name string `json:"name"`
ActiveRevision string `json:"active_revision,omitempty"`
LastSuccessfulActivation time.Time `json:"last_successful_activation,omitempty"`
Type string `json:"type,omitempty"`
LastSuccessfulDownload time.Time `json:"last_successful_download,omitempty"`
LastSuccessfulRequest time.Time `json:"last_successful_request,omitempty"`
LastRequest time.Time `json:"last_request,omitempty"`
Expand Down
1 change: 1 addition & 0 deletions plugins/discovery/discovery.go
Expand Up @@ -188,6 +188,7 @@ func (c *Discovery) processUpdate(ctx context.Context, u download.Update) {
c.status.LastSuccessfulRequest = c.status.LastRequest

if u.Bundle != nil {
c.status.Type = u.Bundle.Type()
c.status.LastSuccessfulDownload = c.status.LastSuccessfulRequest

if err := c.reconfigure(ctx, u); err != nil {
Expand Down
33 changes: 23 additions & 10 deletions plugins/discovery/discovery_test.go
Expand Up @@ -25,11 +25,12 @@ import (
"github.com/open-policy-agent/opa/logging/test"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
bundleApi "github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/download"
"github.com/open-policy-agent/opa/metrics"
"github.com/open-policy-agent/opa/plugins"
"github.com/open-policy-agent/opa/plugins/bundle"
bundlePlugin "github.com/open-policy-agent/opa/plugins/bundle"
"github.com/open-policy-agent/opa/plugins/logs"
"github.com/open-policy-agent/opa/plugins/status"
"github.com/open-policy-agent/opa/server"
Expand Down Expand Up @@ -90,13 +91,13 @@ func TestEvaluateBundle(t *testing.T) {
t.Fatal("Expected a bundle configuration")
}

var parsedConfig bundle.Config
var parsedConfig bundlePlugin.Config

if err := util.Unmarshal(config.Bundle, &parsedConfig); err != nil {
t.Fatal("Unexpected error:", err)
}

expectedBundleConfig := bundle.Config{
expectedBundleConfig := bundlePlugin.Config{
Name: "test/bundle1",
Service: "example",
}
Expand Down Expand Up @@ -416,6 +417,12 @@ func TestReconfigure(t *testing.T) {

disco.oneShot(ctx, download.Update{Bundle: initialBundle})

if disco.status == nil {
t.Fatal("Expected to find status, found nil")
} else if disco.status.Type != bundle.SnapshotBundleType {
t.Fatalf("expected snapshot bundle but got %v", disco.status.Type)
}

// Verify labels are unchanged
exp := map[string]string{"x": "y", "id": "test-id", "version": version.Version}
if !reflect.DeepEqual(manager.Labels(), exp) {
Expand Down Expand Up @@ -453,6 +460,12 @@ func TestReconfigure(t *testing.T) {

disco.oneShot(ctx, download.Update{Bundle: updatedBundle})

if disco.status == nil {
t.Fatal("Expected to find status, found nil")
} else if disco.status.Type != bundle.SnapshotBundleType {
t.Fatalf("expected snapshot bundle but got %v", disco.status.Type)
}

if !reflect.DeepEqual(testPlugin.counts, map[string]int{"start": 1, "reconfig": 1}) {
t.Errorf("Expected one plugin start and one reconfig but got %v", testPlugin)
}
Expand Down Expand Up @@ -619,7 +632,7 @@ func TestReconfigureWithUpdates(t *testing.T) {
t.Fatalf("Expected two services but got %v\n", len(disco.manager.Services()))
}

bPlugin := bundle.Lookup(disco.manager)
bPlugin := bundlePlugin.Lookup(disco.manager)
config := bPlugin.Config()
expected := "acmecorp"
if config.Bundles["authz"].Service != expected {
Expand Down Expand Up @@ -648,7 +661,7 @@ func TestReconfigureWithUpdates(t *testing.T) {
t.Fatalf("Unexpected error %v", err)
}

bPlugin = bundle.Lookup(disco.manager)
bPlugin = bundlePlugin.Lookup(disco.manager)
config = bPlugin.Config()
expectedSvc := "localhost"
if config.Bundles["authz"].Service != expectedSvc {
Expand Down Expand Up @@ -1163,11 +1176,11 @@ bundle:
t.Fatalf("Unexpected error: %s", err)
}

p := manager.Plugin(bundle.Name)
p := manager.Plugin(bundlePlugin.Name)
if p == nil {
t.Fatal("Unable to find bundle plugin on manager")
}
bp := p.(*bundle.Plugin)
bp := p.(*bundlePlugin.Plugin)

// make sure the older style `bundle` config takes precedence
if bp.Config().Name != "bundle-classic" {
Expand Down Expand Up @@ -1200,11 +1213,11 @@ bundles:
t.Fatalf("Unexpected error: %s", err)
}

p := manager.Plugin(bundle.Name)
p := manager.Plugin(bundlePlugin.Name)
if p == nil {
t.Fatal("Unable to find bundle plugin on manager")
}
bp := p.(*bundle.Plugin)
bp := p.(*bundlePlugin.Plugin)

if len(bp.Config().Bundles) != 1 {
t.Fatal("Expected a single bundle configured")
Expand Down Expand Up @@ -1742,7 +1755,7 @@ func (t *testFixture) loop(ctx context.Context) {
close(done)

case done := <-t.bundleTrigger:
if p, ok := t.manager.Plugin(bundle.Name).(plugins.Triggerable); ok {
if p, ok := t.manager.Plugin(bundlePlugin.Name).(plugins.Triggerable); ok {
_ = p.Trigger(ctx)
}
close(done)
Expand Down

0 comments on commit 02c1c1e

Please sign in to comment.