Addressed PR suggestions

This commit is contained in:
Carlos Monastyrski
2025-07-23 10:37:23 -03:00
parent 05408bc151
commit 60657f0bc6
11 changed files with 37 additions and 34 deletions

View File

@@ -14,6 +14,7 @@ export async function up(knex: Knex): Promise<void> {
t.uuid("envId").notNullable();
t.foreign("envId").references("id").inTable(TableName.Environment).onDelete("CASCADE");
t.timestamps(true, true, true);
t.unique(["policyId", "envId"]);
});
}
if (!(await knex.schema.hasTable(TableName.SecretApprovalPolicyEnvironment))) {
@@ -24,6 +25,7 @@ export async function up(knex: Knex): Promise<void> {
t.uuid("envId").notNullable();
t.foreign("envId").references("id").inTable(TableName.Environment).onDelete("CASCADE");
t.timestamps(true, true, true);
t.unique(["policyId", "envId"]);
});
}

View File

@@ -92,7 +92,8 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi
actorOrgId: req.permission.orgId,
...req.body,
projectSlug: req.body.projectSlug,
name: req.body.name ?? `${req.body.environment || req.body.environments?.join("-")}-${nanoid(3)}`,
name:
req.body.name ?? `${req.body.environment || req.body.environments?.join("-").substring(0, 250)}-${nanoid(3)}`,
enforcementLevel: req.body.enforcementLevel
});
return { approval };

View File

@@ -144,7 +144,7 @@ export interface TAccessApprovalPolicyDALFactory
}
| undefined
>;
findPoliciesByEnvIdAndSecretPath: (
findPolicyByEnvIdAndSecretPath: (
{ envIds, secretPath }: { envIds: string[]; secretPath: string },
tx?: Knex
) => Promise<{
@@ -399,17 +399,17 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient): TAccessApprovalPo
void qb.where(`${TableName.AccessApprovalPolicy}.id`, "=", customFilter.policyId);
}
})
.where((qb) => {
if (customFilter?.envId) {
void qb.where(`${TableName.AccessApprovalPolicyEnvironment}.envId`, "=", customFilter.envId);
}
})
.join(
TableName.AccessApprovalPolicyEnvironment,
`${TableName.AccessApprovalPolicy}.id`,
`${TableName.AccessApprovalPolicyEnvironment}.policyId`
)
.join(TableName.Environment, `${TableName.AccessApprovalPolicyEnvironment}.envId`, `${TableName.Environment}.id`)
.where((qb) => {
if (customFilter?.envId) {
void qb.where(`${TableName.AccessApprovalPolicyEnvironment}.envId`, "=", customFilter.envId);
}
})
.leftJoin(
TableName.AccessApprovalPolicyApprover,
`${TableName.AccessApprovalPolicy}.id`,
@@ -612,7 +612,7 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient): TAccessApprovalPo
}
};
const findPoliciesByEnvIdAndSecretPath: TAccessApprovalPolicyDALFactory["findPoliciesByEnvIdAndSecretPath"] = async (
const findPolicyByEnvIdAndSecretPath: TAccessApprovalPolicyDALFactory["findPolicyByEnvIdAndSecretPath"] = async (
{ envIds, secretPath },
tx
) => {
@@ -677,7 +677,7 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient): TAccessApprovalPo
});
return formattedDocs?.[0];
} catch (error) {
throw new DatabaseError({ error, name: "FindPoliciesByEnvIdAndSecretPath" });
throw new DatabaseError({ error, name: "findPolicyByEnvIdAndSecretPath" });
}
};
@@ -687,6 +687,6 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient): TAccessApprovalPo
findById,
softDeleteById,
findLastValidPolicy,
findPoliciesByEnvIdAndSecretPath
findPolicyByEnvIdAndSecretPath
};
};

View File

@@ -3,14 +3,14 @@ import { Knex } from "knex";
import { TDbClient } from "@app/db";
import { TableName } from "@app/db/schemas";
import { DatabaseError } from "@app/lib/errors";
import { ormify, selectAllTableCols } from "@app/lib/knex";
import { buildFindFilter, ormify, selectAllTableCols } from "@app/lib/knex";
export type TAccessApprovalPolicyEnvironmentDALFactory = ReturnType<typeof accessApprovalPolicyEnvironmentDALFactory>;
export const accessApprovalPolicyEnvironmentDALFactory = (db: TDbClient) => {
const accessApprovalPolicyEnvironmentOrm = ormify(db, TableName.AccessApprovalPolicyEnvironment);
const findAvailablePoliciesIds = async (envId: string, tx?: Knex) => {
const findAvailablePoliciesByEnvId = async (envId: string, tx?: Knex) => {
try {
const docs = await (tx || db.replicaNode())(TableName.AccessApprovalPolicyEnvironment)
.join(
@@ -18,14 +18,15 @@ export const accessApprovalPolicyEnvironmentDALFactory = (db: TDbClient) => {
`${TableName.AccessApprovalPolicyEnvironment}.policyId`,
`${TableName.AccessApprovalPolicy}.id`
)
.where({ [`${TableName.AccessApprovalPolicyEnvironment}.envId` as "envId"]: envId })
// eslint-disable-next-line @typescript-eslint/no-misused-promises
.where(buildFindFilter({ envId }, TableName.AccessApprovalPolicyEnvironment))
.whereNull(`${TableName.AccessApprovalPolicy}.deletedAt`)
.select(selectAllTableCols(TableName.AccessApprovalPolicyEnvironment));
return docs;
} catch (error) {
throw new DatabaseError({ error, name: "findAvailablePoliciesIds" });
throw new DatabaseError({ error, name: "findAvailablePoliciesByEnvId" });
}
};
return { ...accessApprovalPolicyEnvironmentOrm, findAvailablePoliciesIds };
return { ...accessApprovalPolicyEnvironmentOrm, findAvailablePoliciesByEnvId };
};

View File

@@ -77,9 +77,9 @@ export const accessApprovalPolicyServiceFactory = ({
if (!envId && !envIds) {
throw new BadRequestError({ message: "Must provide either envId or envIds" });
}
const policy = await accessApprovalPolicyDAL.findPoliciesByEnvIdAndSecretPath({
const policy = await accessApprovalPolicyDAL.findPolicyByEnvIdAndSecretPath({
secretPath,
envIds: envId ? [envId] : envIds || []
envIds: envId ? [envId] : (envIds as string[])
});
return policyId ? policy && policy.id !== policyId : Boolean(policy);
};

View File

@@ -265,7 +265,7 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => {
return softDeletedPolicy;
};
const findPoliciesByEnvIdAndSecretPath = async (
const findPolicyByEnvIdAndSecretPath = async (
{ envIds, secretPath }: { envIds: string[]; secretPath: string },
tx?: Knex
) => {
@@ -330,9 +330,9 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => {
});
return formattedDocs?.[0];
} catch (error) {
throw new DatabaseError({ error, name: "FindPoliciesByEnvIdAndSecretPath" });
throw new DatabaseError({ error, name: "findPolicyByEnvIdAndSecretPath" });
}
};
return { ...secretApprovalPolicyOrm, findById, find, softDeleteById, findPoliciesByEnvIdAndSecretPath };
return { ...secretApprovalPolicyOrm, findById, find, softDeleteById, findPolicyByEnvIdAndSecretPath };
};

View File

@@ -3,14 +3,14 @@ import { Knex } from "knex";
import { TDbClient } from "@app/db";
import { TableName } from "@app/db/schemas";
import { DatabaseError } from "@app/lib/errors";
import { ormify, selectAllTableCols } from "@app/lib/knex";
import { buildFindFilter, ormify, selectAllTableCols } from "@app/lib/knex";
export type TSecretApprovalPolicyEnvironmentDALFactory = ReturnType<typeof secretApprovalPolicyEnvironmentDALFactory>;
export const secretApprovalPolicyEnvironmentDALFactory = (db: TDbClient) => {
const secretApprovalPolicyEnvironmentOrm = ormify(db, TableName.SecretApprovalPolicyEnvironment);
const findAvailablePoliciesIds = async (envId: string, tx?: Knex) => {
const findAvailablePoliciesByEnvId = async (envId: string, tx?: Knex) => {
try {
const docs = await (tx || db.replicaNode())(TableName.SecretApprovalPolicyEnvironment)
.join(
@@ -18,14 +18,15 @@ export const secretApprovalPolicyEnvironmentDALFactory = (db: TDbClient) => {
`${TableName.SecretApprovalPolicyEnvironment}.policyId`,
`${TableName.SecretApprovalPolicy}.id`
)
.where({ [`${TableName.SecretApprovalPolicyEnvironment}.envId` as "envId"]: envId })
// eslint-disable-next-line @typescript-eslint/no-misused-promises
.where(buildFindFilter({ envId }, TableName.SecretApprovalPolicyEnvironment))
.whereNull(`${TableName.SecretApprovalPolicy}.deletedAt`)
.select(selectAllTableCols(TableName.SecretApprovalPolicyEnvironment));
return docs;
} catch (error) {
throw new DatabaseError({ error, name: "findAvailablePoliciesIds" });
throw new DatabaseError({ error, name: "findAvailablePoliciesByEnvId" });
}
};
return { ...secretApprovalPolicyEnvironmentOrm, findAvailablePoliciesIds };
return { ...secretApprovalPolicyEnvironmentOrm, findAvailablePoliciesByEnvId };
};

View File

@@ -72,7 +72,7 @@ export const secretApprovalPolicyServiceFactory = ({
if (!envIds && !envId) {
throw new BadRequestError({ message: "At least one environment should be provided" });
}
const policy = await secretApprovalPolicyDAL.findPoliciesByEnvIdAndSecretPath({
const policy = await secretApprovalPolicyDAL.findPolicyByEnvIdAndSecretPath({
envIds: envId ? [envId] : envIds || [],
secretPath
});

View File

@@ -21,8 +21,8 @@ type TProjectEnvServiceFactoryDep = {
permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
licenseService: Pick<TLicenseServiceFactory, "getPlan">;
keyStore: Pick<TKeyStoreFactory, "acquireLock" | "setItemWithExpiry" | "getItem" | "waitTillReady">;
accessApprovalPolicyEnvironmentDAL: Pick<TAccessApprovalPolicyEnvironmentDALFactory, "findAvailablePoliciesIds">;
secretApprovalPolicyEnvironmentDAL: Pick<TSecretApprovalPolicyEnvironmentDALFactory, "findAvailablePoliciesIds">;
accessApprovalPolicyEnvironmentDAL: Pick<TAccessApprovalPolicyEnvironmentDALFactory, "findAvailablePoliciesByEnvId">;
secretApprovalPolicyEnvironmentDAL: Pick<TSecretApprovalPolicyEnvironmentDALFactory, "findAvailablePoliciesByEnvId">;
};
export type TProjectEnvServiceFactory = ReturnType<typeof projectEnvServiceFactory>;
@@ -222,15 +222,15 @@ export const projectEnvServiceFactory = ({
}
const env = await projectEnvDAL.transaction(async (tx) => {
const secretApprovalRequest = await secretApprovalPolicyEnvironmentDAL.findAvailablePoliciesIds(id, tx);
if (secretApprovalRequest.length > 0) {
const secretApprovalPolicies = await secretApprovalPolicyEnvironmentDAL.findAvailablePoliciesByEnvId(id, tx);
if (secretApprovalPolicies.length > 0) {
throw new BadRequestError({
message: "Environment is in use by a secret approval policy",
name: "DeleteEnvironment"
});
}
const accessApprovalPolicy = await accessApprovalPolicyEnvironmentDAL.findAvailablePoliciesIds(id, tx);
if (accessApprovalPolicy.length > 0) {
const accessApprovalPolicies = await accessApprovalPolicyEnvironmentDAL.findAvailablePoliciesByEnvId(id, tx);
if (accessApprovalPolicies.length > 0) {
throw new BadRequestError({
message: "Environment is in use by an access approval policy",
name: "DeleteEnvironment"

View File

@@ -8,7 +8,6 @@ export type TAccessApprovalPolicy = {
name: string;
approvals: number;
secretPath: string;
envId: string;
workspace: string;
environments: WorkspaceEnv[];
projectId: string;

View File

@@ -5,7 +5,6 @@ export type TSecretApprovalPolicy = {
id: string;
workspace: string;
name: string;
envId: string;
environments: WorkspaceEnv[];
secretPath?: string;
approvals: number;