Enable VSecM Sentinel Init Command to Wait Until VSecM Safe is Healthy #577
Conversation
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
| @@ -37,7 +37,7 @@ func rootKeyTriplet(content string) (string, string, string) { | |||
| } | |||
|
|
|||
| func keys() (string, string, string) { | |||
| p := env.KeyGenRootKeyPath() | |||
There was a problem hiding this comment.
I renamed the ones for Safe because Safe was looking like an adjective in say SafeSpiffeId() implying spiffie id is safe/secure/etc; which is not the case; the spiffe id is an id destined to safe it is a SpiffeIdForSafe()
Once renaming Safe env getters, the rest were inconsistent; so I renamed these too, for consistency and alignment.
| @@ -53,8 +55,38 @@ import ( | |||
| func RunInitCommands(ctx context.Context) { | |||
| cid := ctx.Value("correlationId").(*string) | |||
|
|
|||
| src, acquired := spiffe.AcquireSourceForSentinel(ctx) | |||
There was a problem hiding this comment.
This is the actual fix.
| for { | ||
| select { | ||
| case <-timeoutCtx.Done(): | ||
| log.ErrorLn(cid, "Failed to acquire source at RunInitCommands (1)") |
There was a problem hiding this comment.
I thought of using Fatalln() here and kill the app; but even if it cannot run init commands, it might still function, so I made a sacrifice of stability over reliability.
This case should ideally never happen anyways.
| "github.com/vmware-tanzu/secrets-manager/core/env" | ||
| log "github.com/vmware-tanzu/secrets-manager/core/log/rpc" | ||
| "github.com/vmware-tanzu/secrets-manager/core/validation" | ||
| ) | ||
|
|
||
| func acquireSource(ctx context.Context) (*workloadapi.X509Source, bool) { |
There was a problem hiding this comment.
made this method public.
| func InitCommandRunnerWaitTimeoutForSentinel() time.Duration { | ||
| p := os.Getenv("VSECM_SENTINEL_INIT_COMMAND_RUNNER_WAIT_TIMEOUT") | ||
| if p == "" { | ||
| p = "300000" |
There was a problem hiding this comment.
300 secs is the default timeout for Sentinel to way.
It typically takes 30 seconds for everything to reconcile, so 300 secs is more than graceful of a wait time.
Enable VSecM Sentinel Init Command to Wait Until VSecM Safe is Healthy
Description
Until now, VSecM Sentinel init command stanza was executing as soon as VSecM Sentinel booted up. That resulted a race condition because it was highly likely that VSecM Safe was not ready to receive messages yet. We solved out this as introducing the
sleep:pragma as a temporary hack:For example
sleep:30000would instruct VSecM Sentinel to wait 30 more seconds before running the init scripts; which saves the day but is suboptimal.This PR makes VSecM Sentinel try acquiring an identity from the workload API (hence ensure that VSecM Safe is in good shape) before proceeding with the rest of the commands.
This still is “not” enough, but gives a very optimistic guarantee that by the time Sentinel fetches and ID, Safe will already be ready. — That is a step in the right direction.
As a follow-up PR, I’ll enable a direct health check that will call a
/healthendpoint of VSecM Safe to 100% ensure that VSecM Safe is ready before proceeding — but I wanted to get this in first.Changes
List the major changes you have made in bullet points:
RunInitCommands()that waits for the workload API to be ready, and gives up after a timeout (default 300secs)Test Policy Compliance
Unit tests need to be added, I haven’t added them.
Code Quality
to understand.
Documentation
Checklist
Before you submit this PR, please make sure:
especially the test policy.
under the project’s license.
By submitting this pull request, you confirm that my contribution is made under
the terms of the project’s license and that you have the authority to grant
these rights.
Thank you for your contribution to VMware Secrets Manager
🐢⚡️!