add option to override period triggers for discovery and bundle plugin

- control timing of f.ex. bundle download directly
- fail fast for non-recoverable errors

Signed-off-by: Torsten Wunderlich <[email protected]>

Author: torwunder

Diff for: config/config.go

@@ -294,14 +294,17 @@ type Config struct {
 	LuaModules *listFlag `yaml:"lua-modules"`
 	LuaSources *listFlag `yaml:"lua-sources"`
 
-	EnableOpenPolicyAgent                bool          `yaml:"enable-open-policy-agent"`
-	OpenPolicyAgentConfigTemplate        string        `yaml:"open-policy-agent-config-template"`
-	OpenPolicyAgentEnvoyMetadata         string        `yaml:"open-policy-agent-envoy-metadata"`
-	OpenPolicyAgentCleanerInterval       time.Duration `yaml:"open-policy-agent-cleaner-interval"`
-	OpenPolicyAgentStartupTimeout        time.Duration `yaml:"open-policy-agent-startup-timeout"`
-	OpenPolicyAgentRequestBodyBufferSize int64         `yaml:"open-policy-agent-request-body-buffer-size"`
-	OpenPolicyAgentMaxRequestBodySize    int64         `yaml:"open-policy-agent-max-request-body-size"`
-	OpenPolicyAgentMaxMemoryBodyParsing  int64         `yaml:"open-policy-agent-max-memory-body-parsing"`
+	EnableOpenPolicyAgent                       bool          `yaml:"enable-open-policy-agent"`
+	EnableOpenPolicyAgentOverridePeriodTriggers bool          `yaml:"enable-open-policy-agent-override-period-triggers"`
+	OpenPolicyAgentConfigTemplate               string        `yaml:"open-policy-agent-config-template"`
+	OpenPolicyAgentEnvoyMetadata                string        `yaml:"open-policy-agent-envoy-metadata"`
+	OpenPolicyAgentCleanerInterval              time.Duration `yaml:"open-policy-agent-cleaner-interval"`
+	OpenPolicyAgentPluginTriggerInterval        time.Duration `yaml:"open-policy-agent-plugin-trigger-interval"`
+	OpenPolicyAgentMaxPluginTriggerJitter       time.Duration `yaml:"open-policy-agent-max-plugin-trigger-jitter"`
+	OpenPolicyAgentStartupTimeout               time.Duration `yaml:"open-policy-agent-startup-timeout"`
+	OpenPolicyAgentRequestBodyBufferSize        int64         `yaml:"open-policy-agent-request-body-buffer-size"`
+	OpenPolicyAgentMaxRequestBodySize           int64         `yaml:"open-policy-agent-max-request-body-size"`
+	OpenPolicyAgentMaxMemoryBodyParsing         int64         `yaml:"open-policy-agent-max-memory-body-parsing"`
 
 	PassiveHealthCheck mapFlags `yaml:"passive-health-check"`
 }
@@ -524,9 +527,12 @@ func NewConfig() *Config {
 	flag.Var(cfg.CredentialPaths, "credentials-paths", "directories or files to watch for credentials to use by bearerinjector filter")
 	flag.DurationVar(&cfg.CredentialsUpdateInterval, "credentials-update-interval", 10*time.Minute, "sets the interval to update secrets")
 	flag.BoolVar(&cfg.EnableOpenPolicyAgent, "enable-open-policy-agent", false, "enables Open Policy Agent filters")
+	flag.BoolVar(&cfg.EnableOpenPolicyAgentOverridePeriodTriggers, "enable-open-policy-agent-override-period-triggers", false, "when enabled skipper will set all plugin triggers to manual and trigger all plugins directly, which includes f.ex. to download of new bundles")
 	flag.StringVar(&cfg.OpenPolicyAgentConfigTemplate, "open-policy-agent-config-template", "", "file containing a template for an Open Policy Agent configuration file that is interpolated for each OPA filter instance")
 	flag.StringVar(&cfg.OpenPolicyAgentEnvoyMetadata, "open-policy-agent-envoy-metadata", "", "JSON file containing meta-data passed as input for compatibility with Envoy policies in the format")
 	flag.DurationVar(&cfg.OpenPolicyAgentCleanerInterval, "open-policy-agent-cleaner-interval", openpolicyagent.DefaultCleanIdlePeriod, "Duration in seconds to wait before cleaning up unused opa instances")
+	flag.DurationVar(&cfg.OpenPolicyAgentPluginTriggerInterval, "open-policy-agent-plugin-trigger-interval", openpolicyagent.DefaultPluginTriggerInterval, "Interval between triggering the opa plugins to f.ex. download new bundles. Only applies when overriding period triggers is enabled")
+	flag.DurationVar(&cfg.OpenPolicyAgentMaxPluginTriggerJitter, "open-policy-agent-max-plugin-trigger-jitter", openpolicyagent.DefaultMaxPluginTriggerJitter, "Maximum jitter to add to the plugin trigger interval. Only applies when overriding period triggers is enabled")
 	flag.DurationVar(&cfg.OpenPolicyAgentStartupTimeout, "open-policy-agent-startup-timeout", openpolicyagent.DefaultOpaStartupTimeout, "Maximum duration in seconds to wait for the open policy agent to start up")
 	flag.Int64Var(&cfg.OpenPolicyAgentMaxRequestBodySize, "open-policy-agent-max-request-body-size", openpolicyagent.DefaultMaxRequestBodySize, "Maximum number of bytes from a http request body that are passed as input to the policy")
 	flag.Int64Var(&cfg.OpenPolicyAgentRequestBodyBufferSize, "open-policy-agent-request-body-buffer-size", openpolicyagent.DefaultRequestBodyBufferSize, "Read buffer size for the request body")
@@ -978,14 +984,17 @@ func (c *Config) ToOptions() skipper.Options {
 		LuaModules: c.LuaModules.values,
 		LuaSources: c.LuaSources.values,
 
-		EnableOpenPolicyAgent:                c.EnableOpenPolicyAgent,
-		OpenPolicyAgentConfigTemplate:        c.OpenPolicyAgentConfigTemplate,
-		OpenPolicyAgentEnvoyMetadata:         c.OpenPolicyAgentEnvoyMetadata,
-		OpenPolicyAgentCleanerInterval:       c.OpenPolicyAgentCleanerInterval,
-		OpenPolicyAgentStartupTimeout:        c.OpenPolicyAgentStartupTimeout,
-		OpenPolicyAgentMaxRequestBodySize:    c.OpenPolicyAgentMaxRequestBodySize,
-		OpenPolicyAgentRequestBodyBufferSize: c.OpenPolicyAgentRequestBodyBufferSize,
-		OpenPolicyAgentMaxMemoryBodyParsing:  c.OpenPolicyAgentMaxMemoryBodyParsing,
+		EnableOpenPolicyAgent:                       c.EnableOpenPolicyAgent,
+		EnableOpenPolicyAgentOverridePeriodTriggers: c.EnableOpenPolicyAgentOverridePeriodTriggers,
+		OpenPolicyAgentConfigTemplate:               c.OpenPolicyAgentConfigTemplate,
+		OpenPolicyAgentEnvoyMetadata:                c.OpenPolicyAgentEnvoyMetadata,
+		OpenPolicyAgentCleanerInterval:              c.OpenPolicyAgentCleanerInterval,
+		OpenPolicyAgentPluginTriggerInterval:        c.OpenPolicyAgentPluginTriggerInterval,
+		OpenPolicyAgentMaxPluginTriggerJitter:       c.OpenPolicyAgentMaxPluginTriggerJitter,
+		OpenPolicyAgentStartupTimeout:               c.OpenPolicyAgentStartupTimeout,
+		OpenPolicyAgentMaxRequestBodySize:           c.OpenPolicyAgentMaxRequestBodySize,
+		OpenPolicyAgentRequestBodyBufferSize:        c.OpenPolicyAgentRequestBodyBufferSize,
+		OpenPolicyAgentMaxMemoryBodyParsing:         c.OpenPolicyAgentMaxMemoryBodyParsing,
 
 		PassiveHealthCheck: c.PassiveHealthCheck.values,
 	}

Diff for: config/config_test.go

@@ -165,7 +165,9 @@ func defaultConfig(with func(*Config)) *Config {
 		LuaModules:                              commaListFlag(),
 		LuaSources:                              commaListFlag(),
 		OpenPolicyAgentCleanerInterval:          openpolicyagent.DefaultCleanIdlePeriod,
-		OpenPolicyAgentStartupTimeout:           30 * time.Second,
+		OpenPolicyAgentStartupTimeout:           openpolicyagent.DefaultOpaStartupTimeout,
+		OpenPolicyAgentPluginTriggerInterval:    openpolicyagent.DefaultPluginTriggerInterval,
+		OpenPolicyAgentMaxPluginTriggerJitter:   openpolicyagent.DefaultMaxPluginTriggerJitter,
 		OpenPolicyAgentMaxRequestBodySize:       openpolicyagent.DefaultMaxRequestBodySize,
 		OpenPolicyAgentMaxMemoryBodyParsing:     openpolicyagent.DefaultMaxMemoryBodyParsing,
 		OpenPolicyAgentRequestBodyBufferSize:    openpolicyagent.DefaultRequestBodyBufferSize,

Diff for: filters/openpolicyagent/internal/confighook.go

@@ -0,0 +1,80 @@
+package internal
+
+import (
+	"context"
+	"encoding/json"
+	"github.com/open-policy-agent/opa/v1/config"
+	"github.com/open-policy-agent/opa/v1/plugins"
+	"github.com/open-policy-agent/opa/v1/plugins/bundle"
+	"github.com/open-policy-agent/opa/v1/plugins/discovery"
+)
+
+// ManualOverride is override the plugin trigger to manual trigger mode, allowing the openpolicyagent filter
+// to take control of the trigger mechanism.
+// * OnConfig will handle a general config change and will override the trigger mode for both discovery
+//	 and bundle plugins.
+// * OnConfigDiscovery will handle a config change via discovery mechanism and will only override
+//	 the trigger mode for the bundle plugin as the discovery plugin is involved in the trigger for
+//	 this config change.
+// See https://github.com/open-policy-agent/opa/pull/6053 for details on the hooks.
+
+type ManualOverride struct {
+}
+
+func (m *ManualOverride) OnConfig(ctx context.Context, config *config.Config) (*config.Config, error) {
+	config, err := discoveryPluginOverride(config)
+	if err != nil {
+		return config, err
+	}
+	return bundlePluginConfigOverride(config)
+}
+
+func (m *ManualOverride) OnConfigDiscovery(ctx context.Context, config *config.Config) (*config.Config, error) {
+	return bundlePluginConfigOverride(config)
+}
+
+func discoveryPluginOverride(config *config.Config) (*config.Config, error) {
+	var (
+		discoveryConfig discovery.Config
+		triggerManual   = plugins.TriggerManual
+		message         []byte
+	)
+
+	if config.Discovery != nil {
+		if err := json.Unmarshal(config.Discovery, &discoveryConfig); err == nil {
+			discoveryConfig.Trigger = &triggerManual
+			if message, err = json.Marshal(discoveryConfig); err == nil {
+				config.Discovery = message
+			} else {
+				return config, err
+			}
+		} else {
+			return config, err
+		}
+	}
+	return config, nil
+}
+
+func bundlePluginConfigOverride(config *config.Config) (*config.Config, error) {
+	var (
+		bundlesConfig map[string]*bundle.Source
+		manualTrigger = plugins.TriggerManual
+		message       []byte
+	)
+
+	if config.Bundles != nil {
+		if err := json.Unmarshal(config.Bundles, &bundlesConfig); err == nil {
+			for _, bndlCfg := range bundlesConfig {
+				bndlCfg.Trigger = &manualTrigger
+			}
+			if message, err = json.Marshal(bundlesConfig); err == nil {
+				config.Bundles = message
+			} else {
+				return config, err
+			}
+		} else {
+			return config, err
+		}
+	}
+	return config, nil
+}

Diff for: filters/openpolicyagent/internal/confighook_test.go

@@ -0,0 +1,138 @@
+package internal
+
+import (
+	"context"
+	"encoding/json"
+	"github.com/open-policy-agent/opa/v1/config"
+	"github.com/open-policy-agent/opa/v1/plugins"
+	"github.com/open-policy-agent/opa/v1/plugins/bundle"
+	"github.com/open-policy-agent/opa/v1/plugins/discovery"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestManualOverride_OnConfig_WithDiscoveryConfig(t *testing.T) {
+	config := config.Config{
+		Discovery: []byte(`{
+			"resource": "discovery",
+			"service": "test",
+			"persist": false
+		}`),
+	}
+
+	m := &ManualOverride{}
+	m.OnConfig(context.Background(), &config)
+
+	var discoveryConfig discovery.Config
+
+	err := json.Unmarshal(config.Discovery, &discoveryConfig)
+	assert.NoError(t, err)
+
+	// verify trigger is set to manual
+	assert.Equal(t, *discoveryConfig.Trigger, plugins.TriggerManual)
+
+	// verify existing config is untouched
+	assert.Equal(t, *discoveryConfig.Resource, "discovery")
+	assert.Equal(t, discoveryConfig.Service, "test")
+	assert.Equal(t, discoveryConfig.Persist, false)
+}
+
+func TestManualOverride_OnConfig_WithBundlesConfig(t *testing.T) {
+	config := config.Config{
+		Bundles: []byte(`{"test":{
+			"resource": "test-bundle",
+			"service": "test",
+			"persist": false
+		}}`),
+	}
+
+	m := &ManualOverride{}
+	m.OnConfig(context.Background(), &config)
+
+	var bundlesConfig map[string]*bundle.Source
+
+	err := json.Unmarshal(config.Bundles, &bundlesConfig)
+	assert.NoError(t, err)
+
+	// verify trigger is set to manual
+	assert.Equal(t, *bundlesConfig["test"].Trigger, plugins.TriggerManual)
+}
+
+func TestManualOverride_OnConfig_WithEmptyConfig(t *testing.T) {
+	config := &config.Config{}
+
+	m := &ManualOverride{}
+	processedConfig, err := m.OnConfig(context.Background(), config)
+
+	assert.NoError(t, err)
+	assert.Equal(t, config, processedConfig)
+}
+
+func TestManualOverride_OnConfig_InvalidDiscoveryConfig(t *testing.T) {
+	config := config.Config{
+		Discovery: []byte(`invalid`),
+	}
+
+	m := &ManualOverride{}
+	_, err := m.OnConfig(context.Background(), &config)
+
+	assert.ErrorContains(t, err, "invalid character")
+}
+
+func TestManualOverride_OnConfig_InvalidBundlesConfig(t *testing.T) {
+	config := config.Config{
+		Bundles: []byte(`invalid`),
+	}
+
+	m := &ManualOverride{}
+	_, err := m.OnConfig(context.Background(), &config)
+
+	assert.ErrorContains(t, err, "invalid character")
+}
+
+func TestManualOverride_OnConfigDiscovery(t *testing.T) {
+	config := config.Config{
+		Bundles: []byte(`{"test":{
+			"resource": "test-bundle",
+			"service": "test",
+			"persist": false
+		}}`),
+	}
+
+	m := &ManualOverride{}
+	m.OnConfigDiscovery(context.Background(), &config)
+
+	var bundlesConfig map[string]*bundle.Source
+
+	err := json.Unmarshal(config.Bundles, &bundlesConfig)
+	assert.NoError(t, err)
+
+	// verify trigger is set to manual
+	assert.Equal(t, *bundlesConfig["test"].Trigger, plugins.TriggerManual)
+
+	// verify existing config is untouched
+	assert.Equal(t, bundlesConfig["test"].Resource, "test-bundle")
+	assert.Equal(t, bundlesConfig["test"].Service, "test")
+	assert.Equal(t, bundlesConfig["test"].Persist, false)
+}
+
+func TestManualOverride_OnConfigDiscovery_WithoutBundlesConfig(t *testing.T) {
+	config := &config.Config{}
+
+	m := &ManualOverride{}
+	processedConfig, err := m.OnConfigDiscovery(context.Background(), config)
+
+	assert.NoError(t, err)
+	assert.Equal(t, config, processedConfig)
+}
+
+func TestManualOverride_OnConfigDiscovery_InvalidBundlesConfig(t *testing.T) {
+	config := config.Config{
+		Bundles: []byte(`invalid`),
+	}
+
+	m := &ManualOverride{}
+	_, err := m.OnConfigDiscovery(context.Background(), &config)
+
+	assert.ErrorContains(t, err, "invalid character")
+}