Allow semaphore initialization with limit 0 to support "approval gate" pattern #16730
Closed
mustafasezer
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Use case
We want to build an "approval gate" for Workflows: a Workflow should be created holding a synchronization.semaphore whose configMapKeyRef points to a per-workflow ConfigMap key initialized to 0. The workflow should stay blocked (never acquire the lock) until an operator manually edits the ConfigMap to raise the limit to 1, at which point the workflow is allowed to proceed.
This is a natural way to implement manual-approval / hold-until-signaled semantics using the existing semaphore primitive instead of introducing a separate suspend/resume mechanism, but the current implementation prevents it because any limit of 0 is treated as an initialization failure rather than a valid "closed" semaphore state.
Current workaround
Once a semaphore has been successfully initialized with a limit >= 1, the backing ConfigMap value can subsequently be updated to 0 at runtime without error — only the initial creation path rejects 0. So today we're forced to initialize the ConfigMap with a limit of 1 (or higher) and then immediately patch it down to 0 before the workflow's first reconcile, purely to work around this initialization check. This is a hacky, race-prone workaround (the workflow could acquire the lock in the window before the value is patched down) rather than a real solution, and it would be much cleaner to allow 0 as a valid initial limit.
Proposed behavior
Allow limit == 0 to be treated as a valid state (a semaphore that currently permits zero concurrent holders), and only fail initialization for actual errors reading/resolving the limit (e.g. missing ConfigMap/key, or a genuine parse error), rather than for a successfully resolved value of 0.
Additional context
Relevant code: workflow/sync/semaphore.go#L44
All reactions