feat(dynamic-secrets): retry lease revocation when failing

This commit is contained in:
Daniel Hougaard
2025-11-25 10:11:20 -08:00
parent 3847d64b0d
commit d71eac815d
7 changed files with 434 additions and 126 deletions
@@ -1,10 +1,14 @@
import { DisableRotationErrors } from "@app/ee/services/secret-rotation/secret-rotation-queue";
import { applyJitter } from "@app/lib/delay";
import { NotFoundError } from "@app/lib/errors";
import { logger } from "@app/lib/logger";
import { QueueJobs, QueueName, TQueueServiceFactory } from "@app/queue";
import { TIdentityDALFactory } from "@app/services/identity/identity-dal";
import { TKmsServiceFactory } from "@app/services/kms/kms-service";
import { KmsDataKey } from "@app/services/kms/kms-types";
import { TSecretFolderDALFactory } from "@app/services/secret-folder/secret-folder-dal";
import { TSmtpService } from "@app/services/smtp/smtp-service";
import { TUserDALFactory } from "@app/services/user/user-dal";
import { TDynamicSecretDALFactory } from "../dynamic-secret/dynamic-secret-dal";
import { DynamicSecretStatus } from "../dynamic-secret/dynamic-secret-types";
@@ -15,6 +19,9 @@ import { TDynamicSecretLeaseConfig } from "./dynamic-secret-lease-types";
type TDynamicSecretLeaseQueueServiceFactoryDep = {
queueService: TQueueServiceFactory;
dynamicSecretLeaseDAL: Pick<TDynamicSecretLeaseDALFactory, "findById" | "deleteById" | "find" | "updateById">;
smtpService: Pick<TSmtpService, "sendMail">;
userDAL: Pick<TUserDALFactory, "findById">;
identityDAL: TIdentityDALFactory;
dynamicSecretDAL: Pick<TDynamicSecretDALFactory, "findById" | "deleteById" | "updateById">;
dynamicSecretProviders: Record<DynamicSecretProviders, TDynamicProviderFns>;
kmsService: Pick<TKmsServiceFactory, "createCipherPairWithDataKey">;
@@ -25,6 +32,7 @@ export type TDynamicSecretLeaseQueueServiceFactory = {
pruneDynamicSecret: (dynamicSecretCfgId: string) => Promise<void>;
setLeaseRevocation: (leaseId: string, expiryAt: Date) => Promise<void>;
unsetLeaseRevocation: (leaseId: string) => Promise<void>;
queueFailedRevocation: (leaseId: string) => Promise<void>;
init: () => Promise<void>;
};
@@ -68,6 +76,20 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
await queueService.stopJobByIdPg(QueueName.DynamicSecretRevocation, leaseId);
};
const queueFailedRevocation = async (leaseId: string) => {
await queueService.queuePg<QueueName.DynamicSecretRevocation>(
QueueJobs.DynamicSecretRevocation,
{ leaseId },
{
singletonKey: `${leaseId}-retry`, // avoid conflicts with scheduled revocation
retryDelay: Math.floor(applyJitter(3_600_000 * 4) / 1000), // retry every 4 hours with 20% +- jitter
retryLimit: 20, // we dont want it to ever hit the limit, we want the expireInHours to take effect.
expireInHours: 23, // if we set it to 24 hours, pgboss will complain that the expireIn is too high
deadLetter: QueueName.DynamicSecretRevocationFailedRetry // if all fails, we will send a notification to the user
}
);
};
const $dynamicSecretQueueJob = async (
jobName: string,
jobId: string,
@@ -79,7 +101,9 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
logger.info("Dynamic secret lease revocation started: ", leaseId, jobId);
const dynamicSecretLease = await dynamicSecretLeaseDAL.findById(leaseId);
if (!dynamicSecretLease) throw new DisableRotationErrors({ message: "Dynamic secret lease not found" });
if (!dynamicSecretLease) {
throw new DisableRotationErrors({ message: "Dynamic secret lease not found" });
}
const folder = await folderDAL.findById(dynamicSecretLease.dynamicSecret.folderId);
if (!folder)
@@ -150,7 +174,7 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
}
logger.info("Finished dynamic secret job", jobId);
} catch (error) {
logger.error(error);
logger.error(error, "Failed to delete dynamic secret");
if (jobName === QueueJobs.DynamicSecretPruning) {
const { dynamicSecretCfgId } = data as { dynamicSecretCfgId: string };
@@ -166,6 +190,11 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
status: DynamicSecretStatus.FailedDeletion,
statusDetails: (error as Error)?.message?.slice(0, 255)
});
// if revocation fails, we should stop the job and queue a new job to retry the revocation at a later time.
await queueService.stopJobById(QueueName.DynamicSecretRevocation, jobId);
await queueService.stopJobByIdPg(QueueName.DynamicSecretRevocation, jobId);
await queueFailedRevocation(leaseId);
}
if (error instanceof DisableRotationErrors) {
if (jobId) {
@@ -173,7 +202,35 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
await queueService.stopJobByIdPg(QueueName.DynamicSecretRevocation, jobId);
}
}
// propogate to next part
}
};
// TODO(daniel): add alerting. this is scaffolding for now, pending dashboard overview page for alerts.
const $dynamicSecretRevocationFailedRetryJob = async (jobData: { leaseId: string }, jobId: string) => {
try {
const { leaseId } = jobData;
logger.info({ leaseId, jobId }, "Dynamic secret revocation failed. Notifying root user about failed revocation.");
// const lease = await dynamicSecretLeaseDAL.findById(leaseId);
// if (!lease) {
// throw new DisableRotationErrors({ message: "Dynamic secret lease not found" });
// }
// const folder = await folderDAL.findById(lease.dynamicSecret.folderId);
// if (!folder) throw new NotFoundError({ message: `Failed to find folder with ${lease.dynamicSecret.folderId}` });
//
//
// this is where we would send a notification to the user who created the identity, that started the revocation process.
// currently we have no way of knowing which user created the identity, so we cannot send them a notification.
// we shouldn't send an email for EVERY failed revocation. we should have a delay in between, so we don't send spam emails.
// we should have a delay for 2 minutes so we only send out (at most) 1 email every 2 minutes for revocations.
// as an example if 100 failed revocations happen at the same time, we only send out 1 email.
} catch (error) {
if (error instanceof DisableRotationErrors) {
if (jobId) {
await queueService.stopJobById(QueueName.DynamicSecretRevocationFailedRetry, jobId);
await queueService.stopJobByIdPg(QueueName.DynamicSecretRevocationFailedRetry, jobId);
}
}
throw error;
}
};
@@ -204,12 +261,25 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
pollingIntervalSeconds: 1
}
);
// this job is triggered when the dead letter queue is triggered from retrying failed lease revocations.
await queueService.startPg<QueueName.DynamicSecretRevocationFailedRetry>(
QueueJobs.DynamicSecretRevocationFailedRetry,
async ([job]) => {
await $dynamicSecretRevocationFailedRetryJob(job.data, job.id);
},
{
workerCount: 5,
pollingIntervalSeconds: 1
}
);
};
return {
pruneDynamicSecret,
setLeaseRevocation,
unsetLeaseRevocation,
queueFailedRevocation,
init
};
};
@@ -358,11 +358,13 @@ export const dynamicSecretLeaseServiceFactory = ({
if ((revokeResponse as { error?: Error })?.error) {
const { error } = revokeResponse as { error?: Error };
logger.error(error?.message, "Failed to revoke lease");
const deletedDynamicSecretLease = await dynamicSecretLeaseDAL.updateById(dynamicSecretLease.id, {
const updatedDynamicSecretLease = await dynamicSecretLeaseDAL.updateById(dynamicSecretLease.id, {
status: DynamicSecretLeaseStatus.FailedDeletion,
statusDetails: error?.message?.slice(0, 255)
});
return deletedDynamicSecretLease;
// queue a job to retry the revocation at a later time
await dynamicSecretQueueService.queueFailedRevocation(dynamicSecretLease.id);
return updatedDynamicSecretLease;
}
await dynamicSecretQueueService.unsetLeaseRevocation(dynamicSecretLease.id);
+10
View File
@@ -2,3 +2,13 @@ export const delay = (ms: number) =>
new Promise<void>((resolve) => {
setTimeout(resolve, ms);
});
export const applyJitter = (delayMs: number) => {
const jitterFactor = 0.2;
// generates random value in [-0.2, +0.2] range
const randomFactor = (Math.random() * 2 - 1) * jitterFactor;
const jitterAmount = randomFactor * delayMs;
return delayMs + jitterAmount;
};
+8
View File
@@ -61,6 +61,7 @@ export enum QueueName {
SecretPushEventScan = "secret-push-event-scan",
UpgradeProjectToGhost = "upgrade-project-to-ghost",
DynamicSecretRevocation = "dynamic-secret-revocation",
DynamicSecretRevocationFailedRetry = "dynamic-secret-revocation-failed-retry",
CaCrlRotation = "ca-crl-rotation",
CaLifecycle = "ca-lifecycle", // parent queue to ca-order-certificate-for-subscriber
SecretReplication = "secret-replication",
@@ -101,6 +102,7 @@ export enum QueueJobs {
UpgradeProjectToGhost = "upgrade-project-to-ghost-job",
DynamicSecretRevocation = "dynamic-secret-revocation",
DynamicSecretPruning = "dynamic-secret-pruning",
DynamicSecretRevocationFailedRetry = "dynamic-secret-revocation-failed-retry",
CaCrlRotation = "ca-crl-rotation-job",
SecretReplication = "secret-replication",
SecretSync = "secret-sync", // parent queue to push integration sync, webhook, and secret replication
@@ -219,6 +221,12 @@ export type TQueueJobTypes = {
name: QueueJobs.TelemetryInstanceStats;
payload: undefined;
};
[QueueName.DynamicSecretRevocationFailedRetry]: {
name: QueueJobs.DynamicSecretRevocationFailedRetry;
payload: {
leaseId: string;
};
};
[QueueName.DynamicSecretRevocation]:
| {
name: QueueJobs.DynamicSecretRevocation;
+4 -1
View File
@@ -1874,7 +1874,10 @@ export const registerRoutes = async (
dynamicSecretProviders,
dynamicSecretDAL,
folderDAL,
kmsService
kmsService,
smtpService,
userDAL,
identityDAL
});
const dynamicSecretService = dynamicSecretServiceFactory({
projectDAL,