diff --git a/opa-services/src/main/java/io/github/open_policy_agent/opa/config/Config.java b/opa-services/src/main/java/io/github/open_policy_agent/opa/config/Config.java index fe943e0f..ed96d4ec 100644 --- a/opa-services/src/main/java/io/github/open_policy_agent/opa/config/Config.java +++ b/opa-services/src/main/java/io/github/open_policy_agent/opa/config/Config.java @@ -455,6 +455,12 @@ public static class StatusConfig { @JsonProperty("resource") private String resource = "/status"; + @JsonProperty("min_delay_seconds") + private Integer minDelaySeconds; + + @JsonProperty("max_delay_seconds") + private Integer maxDelaySeconds; + public Boolean getConsole() { return console; } @@ -482,6 +488,24 @@ public StatusConfig setResource(String resource) { return this; } + public Integer getMinDelaySeconds() { + return minDelaySeconds; + } + + public StatusConfig setMinDelaySeconds(Integer minDelaySeconds) { + this.minDelaySeconds = minDelaySeconds; + return this; + } + + public Integer getMaxDelaySeconds() { + return maxDelaySeconds; + } + + public StatusConfig setMaxDelaySeconds(Integer maxDelaySeconds) { + this.maxDelaySeconds = maxDelaySeconds; + return this; + } + @Override public String toString() { return "StatusConfig{" @@ -493,6 +517,10 @@ public String toString() { + ", resource='" + resource + '\'' + + ", minDelaySeconds=" + + minDelaySeconds + + ", maxDelaySeconds=" + + maxDelaySeconds + '}'; } } diff --git a/opa-services/src/main/java/io/github/open_policy_agent/opa/plugins/StatusPlugin.java b/opa-services/src/main/java/io/github/open_policy_agent/opa/plugins/StatusPlugin.java index 86066ff3..a2d16447 100644 --- a/opa-services/src/main/java/io/github/open_policy_agent/opa/plugins/StatusPlugin.java +++ b/opa-services/src/main/java/io/github/open_policy_agent/opa/plugins/StatusPlugin.java @@ -5,8 +5,9 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import io.github.open_policy_agent.opa.bundle.Bundle; import io.github.open_policy_agent.opa.config.Config; @@ -48,6 +49,24 @@ public Set validate(PluginManager manager) { } } + // Validate delay settings (mirrors BundleDownloader.validatePolling) + Integer min = statusConfig.getMinDelaySeconds(); + Integer max = statusConfig.getMaxDelaySeconds(); + if (min != null && min < 0) { + errors.add("Status min_delay_seconds must be >= 0"); + } + if (max != null && max < 0) { + errors.add("Status max_delay_seconds must be >= 0"); + } + if (min != null && max != null && min >= 0 && max >= 0 && min > max) { + errors.add( + "Status min_delay_seconds (" + + min + + ") cannot be greater than max_delay_seconds (" + + max + + ")"); + } + return errors; } @@ -55,7 +74,7 @@ public Set validate(PluginManager manager) { public Plugin initialize(PluginManager manager) { StatusPlugin plugin = new StatusPlugin(); plugin.manager = manager; - plugin.scheduler = Executors.newScheduledThreadPool(1); + plugin.scheduler = BundleDownloader.newPollScheduler("opa-status-scheduler"); Config.StatusConfig statusConfig = manager.getConfig().getStatus(); if (statusConfig != null) { @@ -63,7 +82,9 @@ public Plugin initialize(PluginManager manager) { new Status(manager, manager.getLogger()) .setConsole(statusConfig.getConsole()) .setService(statusConfig.getService()) - .setResource(statusConfig.getResource()); + .setResource(statusConfig.getResource()) + .setMinDelaySeconds(statusConfig.getMinDelaySeconds()) + .setMaxDelaySeconds(statusConfig.getMaxDelaySeconds()); } return plugin; @@ -76,12 +97,70 @@ public void start() { return; } - // Report status every 30 seconds (matches OPA default) - scheduler.scheduleAtFixedRate(() -> status.reportStatus(), 0, 30, TimeUnit.SECONDS); + // Get report interval bounds (default: 30 seconds, matching OPA's previous fixed interval; + // OPA Go's status plugin has no standalone min/max delay of its own today - reports are + // triggered by the bundle/discovery plugin's polling - so there's no upstream number to + // mirror here beyond the interval this SDK already used). + // - only min set → max defaults to 2 * min + // - only max set → min defaults to min(30, max) so a max below 30 is not silently ignored + // - neither set → min=30, max=60 + Integer configuredMin = status.getMinDelaySeconds(); + Integer configuredMax = status.getMaxDelaySeconds(); + int minDelaySeconds; + int maxDelaySeconds; + if (configuredMin != null && configuredMax != null) { + minDelaySeconds = configuredMin; + maxDelaySeconds = configuredMax; + } else if (configuredMin != null) { + minDelaySeconds = configuredMin; + maxDelaySeconds = configuredMin * 2; + } else if (configuredMax != null) { + maxDelaySeconds = configuredMax; + minDelaySeconds = Math.min(30, configuredMax); + } else { + minDelaySeconds = 30; + maxDelaySeconds = 60; + } + + // Report immediately on startup (matches previous behavior), then continue with a jittered + // chained schedule for subsequent reports - mirrors BundleDownloader.startPolling(), which + // downloads immediately before starting its own chained poll. + scheduler.schedule(() -> status.reportStatus(), 0, TimeUnit.SECONDS); + scheduleNextReport(minDelaySeconds, maxDelaySeconds); manager.updatePluginStatus("status", PluginManager.Status.OK); } + // Re-schedules the next status report with a uniformly random delay in [minDelay, maxDelay], + // mirroring BundleDownloader.scheduleNextPoll and DecisionLogPlugin.scheduleNextFlush. + // ScheduledExecutorService has no built-in jitter, so the task chains itself. + // RejectedExecutionException after a shutdown breaks the chain cleanly. + private void scheduleNextReport(int minDelay, int maxDelay) { + long delay = + minDelay >= maxDelay + ? minDelay + : ThreadLocalRandom.current().nextLong(minDelay, (long) maxDelay + 1); + try { + scheduler.schedule( + () -> { + try { + status.reportStatus(); + } catch (Exception e) { + // reportStatus() handles its own logging; swallow so the chain keeps reporting. + // Only Exception is caught here — Errors (OOM, etc.) propagate and let the + // executor's uncaught-exception handler tear down the pool, which is the right + // outcome for unrecoverable conditions. + } finally { + scheduleNextReport(minDelay, maxDelay); + } + }, + delay, + TimeUnit.SECONDS); + } catch (RejectedExecutionException stopped) { + // Scheduler was shut down; let the chain end. + } + } + @Override public void stop() { if (scheduler != null) { @@ -109,6 +188,8 @@ public static class Status { private Boolean console; private String service; private String resource; + private Integer minDelaySeconds; + private Integer maxDelaySeconds; private Status(PluginManager manager, Logger logger) { this.manager = manager; @@ -142,6 +223,24 @@ public Status setResource(String resource) { return this; } + public Integer getMinDelaySeconds() { + return minDelaySeconds; + } + + public Status setMinDelaySeconds(Integer minDelaySeconds) { + this.minDelaySeconds = minDelaySeconds; + return this; + } + + public Integer getMaxDelaySeconds() { + return maxDelaySeconds; + } + + public Status setMaxDelaySeconds(Integer maxDelaySeconds) { + this.maxDelaySeconds = maxDelaySeconds; + return this; + } + /** Collect and report current status. */ void reportStatus() { try { diff --git a/opa-services/src/test/java/io/github/open_policy_agent/opa/config/ConfigTest.java b/opa-services/src/test/java/io/github/open_policy_agent/opa/config/ConfigTest.java index d64be422..c287527a 100644 --- a/opa-services/src/test/java/io/github/open_policy_agent/opa/config/ConfigTest.java +++ b/opa-services/src/test/java/io/github/open_policy_agent/opa/config/ConfigTest.java @@ -293,6 +293,8 @@ void config_statusDefaults() { assertFalse(status.getConsole()); assertEquals("/status", status.getResource()); + assertNull(status.getMinDelaySeconds()); + assertNull(status.getMaxDelaySeconds()); } @Test diff --git a/opa-services/src/test/java/io/github/open_policy_agent/opa/plugins/StatusPluginTest.java b/opa-services/src/test/java/io/github/open_policy_agent/opa/plugins/StatusPluginTest.java index 5f17b338..6688d45e 100644 --- a/opa-services/src/test/java/io/github/open_policy_agent/opa/plugins/StatusPluginTest.java +++ b/opa-services/src/test/java/io/github/open_policy_agent/opa/plugins/StatusPluginTest.java @@ -114,6 +114,99 @@ void validate_statusConsoleOnly_returnsNoErrors() { assertTrue(errors.isEmpty()); } + @Test + void validate_delaySecondsInvalid_returnsError() { + Config.StatusConfig status = + new Config.StatusConfig() + .setService("test-service") + .setMinDelaySeconds(60) + .setMaxDelaySeconds(30); // min > max is invalid + config.setStatus(status); + + manager = + new PluginManager.Builder() + .withId("test-opa") + .withStore(store) + .withConfig(config) + .withLogger(mockLogger) + .build(); + + StatusPlugin plugin = new StatusPlugin(); + Set errors = plugin.validate(manager); + + assertFalse(errors.isEmpty()); + assertTrue( + errors.stream() + .anyMatch(e -> e.contains("min_delay_seconds") && e.contains("max_delay_seconds"))); + } + + @Test + void validate_delaySecondsEqual_returnsNoErrors() { + Config.StatusConfig status = + new Config.StatusConfig() + .setService("test-service") + .setMinDelaySeconds(30) + .setMaxDelaySeconds(30); + config.setStatus(status); + + manager = + new PluginManager.Builder() + .withId("test-opa") + .withStore(store) + .withConfig(config) + .withLogger(mockLogger) + .build(); + + StatusPlugin plugin = new StatusPlugin(); + Set errors = plugin.validate(manager); + + assertTrue(errors.isEmpty()); + } + + @Test + void validate_negativeDelaySeconds_returnsError() { + Config.StatusConfig status = + new Config.StatusConfig() + .setService("test-service") + .setMinDelaySeconds(-1) + .setMaxDelaySeconds(-5); + config.setStatus(status); + + manager = + new PluginManager.Builder() + .withId("test-opa") + .withStore(store) + .withConfig(config) + .withLogger(mockLogger) + .build(); + + StatusPlugin plugin = new StatusPlugin(); + Set errors = plugin.validate(manager); + + assertTrue(errors.stream().anyMatch(e -> e.contains("min_delay_seconds must be >= 0"))); + assertTrue(errors.stream().anyMatch(e -> e.contains("max_delay_seconds must be >= 0"))); + } + + @Test + void validate_onlyMaxDelayNegative_returnsError() { + Config.StatusConfig status = + new Config.StatusConfig().setService("test-service").setMaxDelaySeconds(-1); + config.setStatus(status); + + manager = + new PluginManager.Builder() + .withId("test-opa") + .withStore(store) + .withConfig(config) + .withLogger(mockLogger) + .build(); + + StatusPlugin plugin = new StatusPlugin(); + Set errors = plugin.validate(manager); + + assertTrue(errors.stream().anyMatch(e -> e.contains("max_delay_seconds must be >= 0"))); + } + @Test void initialize_noStatusConfigured_returnsPlugin() { manager = @@ -173,6 +266,88 @@ void start_setsStatusOk() { assertEquals(PluginManager.Status.OK, manager.getPluginStatus("status")); } + @Test + void start_periodicReport_usesJitteredChainedSchedule() throws Exception { + // min == max makes the jittered delay deterministic (always 1s), keeping the test fast and + // non-flaky while still exercising the chained re-scheduling in scheduleNextReport. + Config.StatusConfig status = + new Config.StatusConfig().setConsole(true).setMinDelaySeconds(1).setMaxDelaySeconds(1); + config.setStatus(status); + + manager = + new PluginManager.Builder() + .withId("test-opa") + .withStore(store) + .withConfig(config) + .withLogger(mockLogger) + .build(); + + StatusPlugin plugin = new StatusPlugin(); + plugin = (StatusPlugin) plugin.initialize(manager); + plugin.start(); + + // Immediate report at t=0, plus chained reports at ~t=1s and ~t=2s. + Thread.sleep(2500); + + // Three reports ~1s apart proves scheduleNextReport re-chains itself instead of firing once + // (which the old immediate-only schedule() call would not do). + verify(mockLogger, atLeast(3)).info(eq("Status: %s"), anyString()); + } + + @Test + void start_onlyMinDelayConfigured_defaultsMaxToTwiceMin() throws Exception { + Config.StatusConfig status = + new Config.StatusConfig() + .setConsole(true) + .setMinDelaySeconds(1); // max omitted → start() uses 2 * min + config.setStatus(status); + + manager = + new PluginManager.Builder() + .withId("test-opa") + .withStore(store) + .withConfig(config) + .withLogger(mockLogger) + .build(); + + StatusPlugin plugin = new StatusPlugin(); + plugin = (StatusPlugin) plugin.initialize(manager); + plugin.start(); + + // With max unset, the chained report falls back to 2x min (2s here). Together with the + // immediate report at t=0, that guarantees at least 2 reports by t=2.5s. If the fallback + // instead used an unrelated large default, only the immediate report would land in time. + Thread.sleep(2500); + + verify(mockLogger, atLeast(2)).info(eq("Status: %s"), anyString()); + } + + @Test + void start_onlyMaxDelayBelowDefaultMin_defaultsMinToMax() throws Exception { + // max-only with max < 30: previously min defaulted to 30, so scheduleNextReport used + // min>=max → always 30s and silently ignored the configured max. Now min = min(30, max). + Config.StatusConfig status = + new Config.StatusConfig().setConsole(true).setMaxDelaySeconds(1); + config.setStatus(status); + + manager = + new PluginManager.Builder() + .withId("test-opa") + .withStore(store) + .withConfig(config) + .withLogger(mockLogger) + .build(); + + StatusPlugin plugin = new StatusPlugin(); + plugin = (StatusPlugin) plugin.initialize(manager); + plugin.start(); + + Thread.sleep(2500); + + // Immediate + chained at ~1s and ~2s proves the max was honored (not replaced by 30s). + verify(mockLogger, atLeast(3)).info(eq("Status: %s"), anyString()); + } + @Test void configDefaults_consoleIsFalse() { Config.StatusConfig status = new Config.StatusConfig(); @@ -181,17 +356,31 @@ void configDefaults_consoleIsFalse() { assertEquals("/status", status.getResource()); } + @Test + void configDefaults_delaySecondsAreCorrect() { + Config.StatusConfig status = new Config.StatusConfig(); + + // Unset in config (null) so Jackson-omitted keys keep the start()-time defaults + // (min=30, max=2*min) reachable — same pattern as ReportingConfig. + assertNull(status.getMinDelaySeconds()); + assertNull(status.getMaxDelaySeconds()); + } + @Test void configBuilder_setsAllFields() { Config.StatusConfig status = new Config.StatusConfig() .setService("test-service") .setConsole(true) - .setResource("/custom/status"); + .setResource("/custom/status") + .setMinDelaySeconds(60) + .setMaxDelaySeconds(120); assertEquals("test-service", status.getService()); assertTrue(status.getConsole()); assertEquals("/custom/status", status.getResource()); + assertEquals(60, status.getMinDelaySeconds()); + assertEquals(120, status.getMaxDelaySeconds()); } @Test