Skip to content

Commit 0b106c2

Browse files
committed
Register bundleplugin listner only when plugin is initialized
1 parent 526b67e commit 0b106c2

File tree

2 files changed

+19
-31
lines changed

2 files changed

+19
-31
lines changed

filters/openpolicyagent/openpolicyagent.go

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,6 @@ func (registry *OpenPolicyAgentRegistry) new(store storage.Store, configBytes []
478478
// policies, report status, etc.
479479
func (opa *OpenPolicyAgentInstance) Start(ctx context.Context, timeout time.Duration) error {
480480
discoveryPlugin := discovery.Lookup(opa.manager)
481-
bundlePlugin := bundle.Lookup(opa.manager)
482481

483482
done := make(chan struct{})
484483
defer close(done)
@@ -491,15 +490,19 @@ func (opa *OpenPolicyAgentInstance) Start(ctx context.Context, timeout time.Dura
491490
}
492491
})
493492

494-
bundlePlugin.Register("startuplistener", func(status bundle.Status) {
495-
if len(status.Errors) > 0 {
496-
failed <- fmt.Errorf("bundle activation failed: %w", errors.Join(status.Errors...))
497-
}
498-
})
499-
defer bundlePlugin.Unregister("startuplistener")
500-
501493
opa.manager.RegisterPluginStatusListener("startuplistener", func(status map[string]*plugins.Status) {
502-
for _, pluginstatus := range status {
494+
for pluginname, pluginstatus := range status {
495+
if pluginname == "bundle" { //To make sure bundle plugin is present to register the listener
496+
bundlePlugin := bundle.Lookup(opa.manager)
497+
498+
bundlePlugin.Register("startuplistener", func(status bundle.Status) {
499+
if len(status.Errors) > 0 {
500+
failed <- fmt.Errorf("bundle activation failed: %w", errors.Join(status.Errors...))
501+
}
502+
})
503+
defer bundlePlugin.Unregister("startuplistener")
504+
}
505+
503506
if pluginstatus != nil && pluginstatus.State != plugins.StateOK {
504507
return
505508
}
@@ -521,17 +524,21 @@ func (opa *OpenPolicyAgentInstance) Start(ctx context.Context, timeout time.Dura
521524

522525
return err
523526
case <-ctx.Done():
527+
timeoutErr := ctx.Err()
524528
for pluginName, status := range opa.manager.PluginStatus() {
525529
if status != nil && status.State != plugins.StateOK {
526530
opa.Logger().WithFields(map[string]interface{}{
527531
"plugin_name": pluginName,
528532
"plugin_state": status.State,
529533
"error_message": status.Message,
530-
}).Error("Open policy agent plugin did not start in time")
534+
}).Error("Open policy agent plugin: %v did not start in time", pluginName)
531535
}
532536
}
533537
opa.Close(ctx)
534-
return fmt.Errorf("one or more open policy agent plugins failed to start in %v with error: %w", timeout, err)
538+
if timeoutErr != nil {
539+
return fmt.Errorf("one or more open policy agent plugins failed to start in %v with error: %w", timeout, timeoutErr)
540+
}
541+
return fmt.Errorf("one or more open policy agent plugins failed to start in %v", timeout)
535542
}
536543
}
537544

@@ -542,25 +549,6 @@ func (opa *OpenPolicyAgentInstance) Close(ctx context.Context) {
542549
})
543550
}
544551

545-
func waitFunc(ctx context.Context, fun func() bool, interval time.Duration) error {
546-
if fun() {
547-
return nil
548-
}
549-
ticker := time.NewTicker(interval)
550-
defer ticker.Stop()
551-
552-
for {
553-
select {
554-
case <-ctx.Done():
555-
return fmt.Errorf("timed out while starting: %w", ctx.Err())
556-
case <-ticker.C:
557-
if fun() {
558-
return nil
559-
}
560-
}
561-
}
562-
}
563-
564552
func configLabelsInfo(opaConfig config.Config) func(*plugins.Manager) {
565553
info := ast.NewObject()
566554
labels := ast.NewObject()

filters/openpolicyagent/openpolicyagent_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ func TestOpaActivationTimeOutWithDiscoveryPointingWrongBundle(t *testing.T) {
339339

340340
instance, err := registry.NewOpenPolicyAgentInstance("test", *cfg, "testfilter")
341341
assert.Nil(t, instance)
342-
assert.Contains(t, err.Error(), "one or more open policy agent plugins failed to start in 1s with error: timed out while starting: context deadline exceeded")
342+
assert.Contains(t, err.Error(), "one or more open policy agent plugins failed to start in 1s with error: context deadline exceeded")
343343
assert.Equal(t, 0, len(registry.instances))
344344
}
345345

0 commit comments

Comments
 (0)