From 1811bbf211bbb7c9c7c4b82b5d2f75efe30d6447 Mon Sep 17 00:00:00 2001 From: James Maes Date: Wed, 14 Oct 2026 23:54:33 -0500 Subject: [PATCH] fix(health): an unconfigured Slack app is UNKNOWN, never DOWN With no SLACK_* secrets at all the app readiness check is NOT_CONFIGURED and the health indicator reports UNKNOWN (HTTP 200) with configured=false, so an optional, unset Slack integration can't fail a readiness probe (it took kof22's probe down). Not-yet-checked is UNKNOWN too. Partial or invalid configuration stays DOWN. Tests for all three. Co-Authored-By: Claude Opus 5.5 --- docs/DESIGN.md | 5 ++ .../qbits/slack/readiness/SlackHealth.java | 24 +++++++- .../qbits/slack/readiness/SlackReadiness.java | 40 +++++++++++++ .../qbits/slack/SlackReadinessTest.java | 57 ++++++++++++++++++- 4 files changed, 122 insertions(+), 4 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 8bfa621..ecddb19 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -430,6 +430,11 @@ user with `users.lookupByEmail`, but only when the IdP marked the session email results, and a user sees their own in Preferences. - **Checks run:** every `readinessIntervalMinutes` (15), on the **Check readiness** action, and from `SlackApi.readiness*`. Messages never include token values or exception text. +- **Not configured is not a failure.** With no Slack app secrets at all, the `app` check is NOT_CONFIGURED and + health is UNKNOWN (HTTP 200) with `configured: false`. Before the first readiness run, health is also UNKNOWN. + Only a configured-but-broken app (some secrets missing, or an invalid token key) is DOWN. Slack is optional, so an + unset Slack integration must never fail a readiness probe; "not configured" shows in the readiness and + central-health views only. - **Health:** when `qqq-middleware-health` is present, the qbit registers `HealthCheckMetaData` `slack`. - It reports the stored deployment-level readiness (`app` and `outbox`), without calling Slack on each probe. diff --git a/src/main/java/com/kingsrook/qbits/slack/readiness/SlackHealth.java b/src/main/java/com/kingsrook/qbits/slack/readiness/SlackHealth.java index c36ecf1..d0da35c 100644 --- a/src/main/java/com/kingsrook/qbits/slack/readiness/SlackHealth.java +++ b/src/main/java/com/kingsrook/qbits/slack/readiness/SlackHealth.java @@ -64,9 +64,29 @@ public class SlackHealth implements HealthIndicator public HealthCheckResult check(QInstance instance) { var readiness = SlackReadiness.stored(instance); - boolean appFailed = readiness.checks().stream().anyMatch(c -> "app".equals(c.name()) && "ERROR".equals(c.status())); - HealthStatus status = appFailed ? HealthStatus.DOWN : readiness.ready() ? HealthStatus.UP : HealthStatus.DEGRADED; + String appStatus = readiness.checks().stream().filter(c -> "app".equals(c.name())).map(c -> c.status()).findFirst().orElse("UNKNOWN"); + + ////////////////////////////////////////////////////////////////////////////// + // Slack is optional. Not configured (no secrets at all) or not yet checked // + // is UNKNOWN, which never fails a readiness probe (HTTP 200). Only a // + // configured-but-broken app (partial or invalid secrets) is DOWN. // + ////////////////////////////////////////////////////////////////////////////// + if(SlackReadiness.NOT_CONFIGURED.equals(appStatus)) + { + return (new HealthCheckResult().withStatus(HealthStatus.UNKNOWN) + .withDetail("configured", false) + .withDetail("message", "Slack is not configured") + .withDetail("checks", readiness.checks())); + } + if("UNKNOWN".equals(appStatus)) + { + return (new HealthCheckResult().withStatus(HealthStatus.UNKNOWN) + .withDetail("message", "Slack readiness has not run yet") + .withDetail("checks", readiness.checks())); + } + HealthStatus status = "ERROR".equals(appStatus) ? HealthStatus.DOWN : readiness.ready() ? HealthStatus.UP : HealthStatus.DEGRADED; return (new HealthCheckResult().withStatus(status) + .withDetail("configured", true) .withDetail("checks", readiness.checks())); } } diff --git a/src/main/java/com/kingsrook/qbits/slack/readiness/SlackReadiness.java b/src/main/java/com/kingsrook/qbits/slack/readiness/SlackReadiness.java index 8185b2f..a3ca907 100644 --- a/src/main/java/com/kingsrook/qbits/slack/readiness/SlackReadiness.java +++ b/src/main/java/com/kingsrook/qbits/slack/readiness/SlackReadiness.java @@ -47,6 +47,8 @@ import com.kingsrook.qqq.backend.core.model.metadata.QInstance; *******************************************************************************/ public final class SlackReadiness { + public static final String NOT_CONFIGURED = "NOT_CONFIGURED"; + private static final Map STORED = Collections.synchronizedMap(new WeakHashMap<>()); private static final Map> WORKSPACES = java.util.Collections.synchronizedMap(new java.util.WeakHashMap<>()); @@ -87,6 +89,17 @@ public final class SlackReadiness app = false; } List checks = new ArrayList<>(); + if(!app && !anySecretPresent(config)) + { + ////////////////////////////////////////////////////////////////////////// + // no SLACK_* secret at all: Slack is simply not set up in this // + // deployment. That is a state, not a failure; health reports UNKNOWN. // + ////////////////////////////////////////////////////////////////////////// + checks.add(new SlackApiReadinessCheck("app", NOT_CONFIGURED, "Slack is not configured: no Slack app secrets are set")); + SlackApiReadiness result = new SlackApiReadiness(false, checks); + STORED.put(instance, result); + return (result); + } checks.add(check("app", app, "Application configuration")); boolean outbox = true; if(config != null && QContext.getQInstance() == instance) @@ -112,6 +125,33 @@ public final class SlackReadiness return (result); } + /******************************************************************************* + ** True when at least one of the app's secret names resolves to a value. + ** Presence only; values are never retained. + *******************************************************************************/ + static boolean anySecretPresent(SlackQBitConfig config) + { + if(config == null) + { + return (false); + } + for(String name : java.util.Arrays.asList(config.getClientIdEnvName(), config.getClientSecretEnvName(), config.getSigningSecretEnvName(), config.getTokenKeyEnvName())) + { + try + { + SlackSecrets.get(config, name); + return (true); + } + catch(RuntimeException e) + { + // absent or not allowed; keep looking + } + } + return (false); + } + + + /******************************************************************************* ** Last scheduled deployment result, without network or database work. *******************************************************************************/ diff --git a/src/test/java/com/kingsrook/qbits/slack/SlackReadinessTest.java b/src/test/java/com/kingsrook/qbits/slack/SlackReadinessTest.java index d80a6e9..3d8c29d 100644 --- a/src/test/java/com/kingsrook/qbits/slack/SlackReadinessTest.java +++ b/src/test/java/com/kingsrook/qbits/slack/SlackReadinessTest.java @@ -33,6 +33,7 @@ import com.kingsrook.qbits.slack.support.SlackStore; import com.kingsrook.qqq.backend.core.model.actions.processes.RunBackendStepInput; import com.kingsrook.qqq.backend.core.model.actions.processes.RunBackendStepOutput; import com.kingsrook.qqq.backend.core.model.data.QRecord; +import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -62,6 +63,58 @@ class SlackReadinessTest } } + /******************************************************************************* + ** No Slack secrets at all is "not configured": UNKNOWN, never DOWN, so an + ** optional, unset Slack integration can't fail a readiness probe. + *******************************************************************************/ + @Test + void notConfiguredIsUnknownNotDown() throws Exception + { + try(SlackTestEnv env = environment()) + { + env.config().withSecretResolver(n -> null); + assertThat(SlackReadiness.deployment(env.instance()).checks()) + .anyMatch(c -> c.name().equals("app") && c.status().equals(SlackReadiness.NOT_CONFIGURED)); + var result = new SlackHealth().check(env.instance()); + assertThat(result.getStatus().name()).isEqualTo("UNKNOWN"); + assertThat(result.getDetails()).containsEntry("configured", false); + } + } + + + + /******************************************************************************* + ** Before readiness has run, health is UNKNOWN rather than degraded or down. + *******************************************************************************/ + @Test + void notYetCheckedIsUnknown() throws Exception + { + try(SlackTestEnv env = environment()) + { + QInstance fresh = new QInstance(); + assertThat(new SlackHealth().check(fresh).getStatus().name()).isEqualTo("UNKNOWN"); + } + } + + + + /******************************************************************************* + ** Partially configured (some secrets set, others missing) is a real + ** misconfiguration and stays DOWN. + *******************************************************************************/ + @Test + void partiallyConfiguredIsDown() throws Exception + { + try(SlackTestEnv env = environment()) + { + env.config().withSecretResolver(n -> "SLACK_CLIENT_ID".equals(n) ? "123.456" : null); + SlackReadiness.deployment(env.instance()); + assertThat(new SlackHealth().check(env.instance()).getStatus().name()).isEqualTo("DOWN"); + } + } + + + /******************************************************************************* ** A valid Base64 string still needs exactly 32 decoded bytes. *******************************************************************************/ @@ -166,7 +219,7 @@ class SlackReadinessTest } /******************************************************************************* - ** Uninitialized readiness maps to degraded health without network access. + ** Uninitialized readiness maps to UNKNOWN health without network access. *******************************************************************************/ @Test void notReadyHealth() throws Exception @@ -174,7 +227,7 @@ class SlackReadinessTest try(SlackTestEnv env = environment()) { assertThat(SlackReadiness.stored(env.instance()).ready()).isFalse(); - assertThat(new SlackHealth().check(env.instance()).getStatus().name()).isEqualTo("DEGRADED"); + assertThat(new SlackHealth().check(env.instance()).getStatus().name()).isEqualTo("UNKNOWN"); assertThat(env.slack().requests()).isEmpty(); } } -- 2.54.0