From b0c0a148926932d996ede4949c140d2dca053826 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Fri, 9 May 2025 22:04:32 -0700 Subject: [PATCH 1/4] fix: reference variable test working --- .../identifier/[identifier]/route.ts | 50 +++---- e2e/tests/api/resource-variables.spec.ts | 17 +-- packages/api/src/router/resources.ts | 124 +++++++----------- packages/rule-engine/src/index.ts | 1 + .../variables/resolve-reference-variable.ts | 29 ++++ 5 files changed, 105 insertions(+), 116 deletions(-) create mode 100644 packages/rule-engine/src/manager/variables/resolve-reference-variable.ts diff --git a/apps/webservice/src/app/api/v1/workspaces/[workspaceId]/resources/identifier/[identifier]/route.ts b/apps/webservice/src/app/api/v1/workspaces/[workspaceId]/resources/identifier/[identifier]/route.ts index 6e1a16d65..6a3f260de 100644 --- a/apps/webservice/src/app/api/v1/workspaces/[workspaceId]/resources/identifier/[identifier]/route.ts +++ b/apps/webservice/src/app/api/v1/workspaces/[workspaceId]/resources/identifier/[identifier]/route.ts @@ -1,11 +1,11 @@ import { NextResponse } from "next/server"; -import { get } from "lodash"; import { and, eq, isNull } from "@ctrlplane/db"; import { db } from "@ctrlplane/db/client"; import { getResourceParents } from "@ctrlplane/db/queries"; import * as schema from "@ctrlplane/db/schema"; import { Channel, getQueue } from "@ctrlplane/events"; +import { getReferenceVariableValue } from "@ctrlplane/rule-engine"; import { variablesAES256 } from "@ctrlplane/secrets"; import { Permission } from "@ctrlplane/validators/auth"; @@ -74,34 +74,26 @@ export const GET = request() ); } - const { relationships, getTargetsWithMetadata } = await getResourceParents( - db, - resource.id, - ); - const relatipnshipTargets = await getTargetsWithMetadata(); - - const variables = Object.fromEntries( - resource.variables.map((v) => { - if (v.valueType === "direct") { - const strval = String(v.value); - const value = v.sensitive - ? variablesAES256().decrypt(strval) - : v.value; - return [v.key, value]; - } - - if (v.valueType === "reference") { - if (v.path == null) return [v.key, v.defaultValue]; - if (v.reference == null) return [v.key, v.defaultValue]; - const target = relationships[v.reference]?.target.id; - const targetResource = relatipnshipTargets[target ?? ""]; - if (targetResource == null) return [v.key, v.defaultValue]; - return [v.key, get(targetResource, v.path, v.defaultValue)]; - } - - throw new Error(`Unknown variable value type: ${v.valueType}`); - }), - ); + const { relationships } = await getResourceParents(db, resource.id); + + const resourceVariablesPromises = resource.variables.map(async (v) => { + if (v.valueType === "direct") { + const strval = String(v.value); + const value = v.sensitive ? variablesAES256().decrypt(strval) : v.value; + return [v.key, value]; + } + + if (v.valueType === "reference") { + const resolvedValue = await getReferenceVariableValue( + v as schema.ReferenceResourceVariable, + ); + return [v.key, resolvedValue]; + } + + throw new Error(`Unknown variable value type: ${v.valueType}`); + }); + const resourceVariables = await Promise.all(resourceVariablesPromises); + const variables = Object.fromEntries(resourceVariables); const metadata = Object.fromEntries( resource.metadata.map((t) => [t.key, t.value]), diff --git a/e2e/tests/api/resource-variables.spec.ts b/e2e/tests/api/resource-variables.spec.ts index 80f57e8ba..10a908d64 100644 --- a/e2e/tests/api/resource-variables.spec.ts +++ b/e2e/tests/api/resource-variables.spec.ts @@ -176,7 +176,9 @@ test.describe("Resource Variables API", () => { api, workspace, }) => { - const systemPrefix = importedEntities.system.slug.split("-")[0]!; + const systemPrefix = importedEntities.system.slug + .split("-")[0]! + .toLowerCase(); // Create target resource const targetResource = await api.POST("/v1/resources", { @@ -185,13 +187,12 @@ test.describe("Resource Variables API", () => { name: `${systemPrefix}-target`, kind: "Target", identifier: `${systemPrefix}-target`, - version: "test-version/v1", + version: `${systemPrefix}-version/v1`, config: { "e2e-test": true } as any, metadata: { "e2e-test": "true", - [`${systemPrefix}`]: "true", + [systemPrefix]: "true", }, - variables: [{ key: "target-var", value: "target-value" }], }, }); expect(targetResource.response.status).toBe(200); @@ -204,7 +205,7 @@ test.describe("Resource Variables API", () => { name: `${systemPrefix}-source`, kind: "Source", identifier: `${systemPrefix}-source`, - version: "test-version/v1", + version: `${systemPrefix}-version/v1`, config: { "e2e-test": true } as any, metadata: { "e2e-test": "true", @@ -214,7 +215,7 @@ test.describe("Resource Variables API", () => { { key: "ref-var", reference: systemPrefix, - path: ["metadata", "e2e-test"], + path: ["e2e-test"], }, ], }, @@ -230,9 +231,9 @@ test.describe("Resource Variables API", () => { reference: systemPrefix, dependencyType: "depends_on", sourceKind: "Source", - sourceVersion: "test-version/v1", + sourceVersion: `${systemPrefix}-version/v1`, targetKind: "Target", - targetVersion: "test-version/v1", + targetVersion: `${systemPrefix}-version/v1`, metadataKeysMatch: ["e2e-test", systemPrefix], }, }); diff --git a/packages/api/src/router/resources.ts b/packages/api/src/router/resources.ts index f3e038db7..38d7d7c7c 100644 --- a/packages/api/src/router/resources.ts +++ b/packages/api/src/router/resources.ts @@ -1,6 +1,6 @@ import type { SQL, Tx } from "@ctrlplane/db"; import type { ResourceCondition } from "@ctrlplane/validators/resources"; -import _, { get } from "lodash"; +import _ from "lodash"; import { isPresent } from "ts-is-present"; import { z } from "zod"; @@ -32,6 +32,7 @@ import { isPassingChannelSelectorPolicy, isPassingNoPendingJobsPolicy, } from "@ctrlplane/job-dispatch"; +import { getReferenceVariableValue } from "@ctrlplane/rule-engine"; import { Permission } from "@ctrlplane/validators/auth"; import { resourceCondition } from "@ctrlplane/validators/resources"; @@ -383,87 +384,52 @@ export const resourceRouter = createTRPCRouter({ .on({ type: "resource", id: input }), }) .input(z.string().uuid()) - .query(({ ctx, input }) => - ctx.db.query.resource - .findFirst({ - where: and(eq(schema.resource.id, input), isNotDeleted), - with: { metadata: true, variables: true, provider: true }, + .query(async ({ ctx, input }) => { + const resource = await ctx.db.query.resource.findFirst({ + where: and(eq(schema.resource.id, input), isNotDeleted), + with: { metadata: true, variables: true, provider: true }, + }); + if (resource == null) return null; + + const { relationships } = await getResourceParents(ctx.db, resource.id); + + const parsedVariables = resource.variables + .map((v) => { + const parsed = schema.resourceVariableSchema.safeParse(v); + if (!parsed.success) return null; + return parsed.data; }) - .then(async (t) => { - if (t == null) return null; - - const { relationships, getTargetsWithMetadata } = - await getResourceParents(ctx.db, t.id); - const relationshipTargets = await getTargetsWithMetadata(); - - const parsedVariables = t.variables - .map((v) => { - try { - return schema.resourceVariableSchema.parse(v); - } catch (error) { - console.error( - `Failed to parse variable ${v.key} for resource ${t.id}:`, - error, - ); - return null; - } - }) - .filter(isPresent); - - const directVariables = parsedVariables.filter( - (v): v is schema.DirectResourceVariable => v.valueType === "direct", - ); + .filter(isPresent); - const referenceVariables = parsedVariables - .filter( - (v): v is schema.ReferenceResourceVariable => - v.valueType === "reference", - ) - .map((v) => { - const relationshipInfo = relationships[v.reference]; - if (!relationshipInfo) - return { ...v, resolvedValue: v.defaultValue }; - - const targetId = relationshipInfo.target.id; - const targetResource = relationshipTargets[targetId]; - - if (!targetResource) - return { ...v, resolvedValue: v.defaultValue }; - - if (v.path.length === 0) - return { - ...v, - resolvedValue: get(targetResource, [], v.defaultValue), - }; - - const metadataKey = v.path.join("/"); - const metadataValue = - metadataKey in targetResource.metadata - ? targetResource.metadata[metadataKey] - : undefined; - - if (metadataValue !== undefined) - return { ...v, resolvedValue: metadataValue }; - - return { - ...v, - resolvedValue: get(targetResource, v.path, v.defaultValue), - }; - }); - - const metadata = Object.fromEntries( - t.metadata.map((m) => [m.key, m.value]), - ); - return { - ...t, - relationships, - directVariables, - referenceVariables, - metadata, - rules: await getResourceRelationshipRules(ctx.db, t.id), - }; + const directVariables = parsedVariables.filter( + (v): v is schema.DirectResourceVariable => v.valueType === "direct", + ); + + const referenceVariables = parsedVariables.filter( + (v): v is schema.ReferenceResourceVariable => + v.valueType === "reference", + ); + + const resolvedReferenceVariables = await Promise.all( + referenceVariables.map(async (v) => { + const resolvedValue = await getReferenceVariableValue(v); + return { ...v, resolvedValue }; }), - ), + ); + + const metadata = Object.fromEntries( + resource.metadata.map((m) => [m.key, m.value]), + ); + + return { + ...resource, + relationships, + directVariables, + referenceVariables: resolvedReferenceVariables, + metadata, + rules: await getResourceRelationshipRules(ctx.db, resource.id), + }; + }), latestDeployedVersions: createTRPCRouter({ byResourceAndEnvironmentId: protectedProcedure diff --git a/packages/rule-engine/src/index.ts b/packages/rule-engine/src/index.ts index 68f412ec7..a3410eac4 100644 --- a/packages/rule-engine/src/index.ts +++ b/packages/rule-engine/src/index.ts @@ -5,3 +5,4 @@ export * from "./utils/merge-policies.js"; export * from "./types.js"; export * from "./manager/version-manager.js"; export * from "./manager/variable-manager.js"; +export * from "./manager/variables/resolve-reference-variable.js"; diff --git a/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts b/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts new file mode 100644 index 000000000..8d06c3cd4 --- /dev/null +++ b/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts @@ -0,0 +1,29 @@ +import type * as schema from "@ctrlplane/db/schema"; +import _ from "lodash"; + +import { db } from "@ctrlplane/db/client"; +import { getResourceParents } from "@ctrlplane/db/queries"; + +export const getReferenceVariableValue = async ( + variable: schema.ReferenceResourceVariable, +) => { + const { relationships, getTargetsWithMetadata } = await getResourceParents( + db, + variable.resourceId, + ); + const relationshipTargets = await getTargetsWithMetadata(); + + const targetId = relationships[variable.reference]?.target.id ?? ""; + const targetResource = relationshipTargets[targetId]; + if (targetResource == null) return variable.defaultValue; + + const isResourceAttribute = variable.path.length === 0; + if (isResourceAttribute) { + const resolvedValue = _.get(targetResource, [], variable.defaultValue); + return resolvedValue; + } + + const metadataKey = variable.path.join("/"); + const metadataValue = targetResource.metadata[metadataKey]; + return metadataValue ?? variable.defaultValue; +}; From f50ceca89ff02f33435b76c6bbaa7f340250151b Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Fri, 9 May 2025 22:09:24 -0700 Subject: [PATCH 2/4] more fix --- .../api/v1/resources/[resourceId]/route.ts | 32 ++++++++++++------- .../identifier/[identifier]/route.ts | 6 ++-- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/apps/webservice/src/app/api/v1/resources/[resourceId]/route.ts b/apps/webservice/src/app/api/v1/resources/[resourceId]/route.ts index fbf582c83..4bda7e6fa 100644 --- a/apps/webservice/src/app/api/v1/resources/[resourceId]/route.ts +++ b/apps/webservice/src/app/api/v1/resources/[resourceId]/route.ts @@ -7,6 +7,7 @@ import { db } from "@ctrlplane/db/client"; import * as schema from "@ctrlplane/db/schema"; import { Channel, getQueue } from "@ctrlplane/events"; import { logger } from "@ctrlplane/logger"; +import { getReferenceVariableValue } from "@ctrlplane/rule-engine"; import { variablesAES256 } from "@ctrlplane/secrets"; import { Permission } from "@ctrlplane/validators/auth"; @@ -57,18 +58,27 @@ export const GET = request() ); const { metadata, ...resource } = data; + const variablesPromises = data.variables.map(async (v) => { + if (v.valueType === "direct") { + const strval = String(v.value); + const value = v.sensitive + ? variablesAES256().decrypt(strval) + : v.value; + return [v.key, value] as const; + } + + if (v.valueType === "reference") { + const resolvedValue = await getReferenceVariableValue( + v as schema.ReferenceResourceVariable, + ); + return [v.key, resolvedValue] as const; + } + + return [v.key, v.defaultValue] as const; + }); + const variables = Object.fromEntries( - data.variables.map((v) => { - if (v.valueType === "direct") { - const strval = String(v.value); - const value = v.sensitive - ? variablesAES256().decrypt(strval) - : v.value; - return [v.key, value]; - } - - return [v.key, v.defaultValue]; - }), + await Promise.all(variablesPromises), ); return NextResponse.json({ diff --git a/apps/webservice/src/app/api/v1/workspaces/[workspaceId]/resources/identifier/[identifier]/route.ts b/apps/webservice/src/app/api/v1/workspaces/[workspaceId]/resources/identifier/[identifier]/route.ts index 6a3f260de..9e82ee5cc 100644 --- a/apps/webservice/src/app/api/v1/workspaces/[workspaceId]/resources/identifier/[identifier]/route.ts +++ b/apps/webservice/src/app/api/v1/workspaces/[workspaceId]/resources/identifier/[identifier]/route.ts @@ -80,17 +80,17 @@ export const GET = request() if (v.valueType === "direct") { const strval = String(v.value); const value = v.sensitive ? variablesAES256().decrypt(strval) : v.value; - return [v.key, value]; + return [v.key, value] as const; } if (v.valueType === "reference") { const resolvedValue = await getReferenceVariableValue( v as schema.ReferenceResourceVariable, ); - return [v.key, resolvedValue]; + return [v.key, resolvedValue] as const; } - throw new Error(`Unknown variable value type: ${v.valueType}`); + return [v.key, v.defaultValue] as const; }); const resourceVariables = await Promise.all(resourceVariablesPromises); const variables = Object.fromEntries(resourceVariables); From 465f399d4d76a9034ce30fbedd70787c3d030f30 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Fri, 9 May 2025 22:30:37 -0700 Subject: [PATCH 3/4] fix --- e2e/tests/api/resource-variables.spec.ts | 2 +- .../manager/variables/resolve-reference-variable.ts | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/e2e/tests/api/resource-variables.spec.ts b/e2e/tests/api/resource-variables.spec.ts index 10a908d64..8832ccbc5 100644 --- a/e2e/tests/api/resource-variables.spec.ts +++ b/e2e/tests/api/resource-variables.spec.ts @@ -215,7 +215,7 @@ test.describe("Resource Variables API", () => { { key: "ref-var", reference: systemPrefix, - path: ["e2e-test"], + path: ["metadata", "e2e-test"], }, ], }, diff --git a/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts b/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts index 8d06c3cd4..533dc9de2 100644 --- a/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts +++ b/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts @@ -17,13 +17,5 @@ export const getReferenceVariableValue = async ( const targetResource = relationshipTargets[targetId]; if (targetResource == null) return variable.defaultValue; - const isResourceAttribute = variable.path.length === 0; - if (isResourceAttribute) { - const resolvedValue = _.get(targetResource, [], variable.defaultValue); - return resolvedValue; - } - - const metadataKey = variable.path.join("/"); - const metadataValue = targetResource.metadata[metadataKey]; - return metadataValue ?? variable.defaultValue; + return _.get(targetResource, variable.path, variable.defaultValue); }; From 01b641fac9c42c02bb511382b456182b58bf12da Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Fri, 9 May 2025 22:33:19 -0700 Subject: [PATCH 4/4] rabbit comment --- .../variables/resolve-reference-variable.ts | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts b/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts index 533dc9de2..6b123cf1c 100644 --- a/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts +++ b/packages/rule-engine/src/manager/variables/resolve-reference-variable.ts @@ -3,19 +3,25 @@ import _ from "lodash"; import { db } from "@ctrlplane/db/client"; import { getResourceParents } from "@ctrlplane/db/queries"; +import { logger } from "@ctrlplane/logger"; export const getReferenceVariableValue = async ( variable: schema.ReferenceResourceVariable, ) => { - const { relationships, getTargetsWithMetadata } = await getResourceParents( - db, - variable.resourceId, - ); - const relationshipTargets = await getTargetsWithMetadata(); + try { + const { relationships, getTargetsWithMetadata } = await getResourceParents( + db, + variable.resourceId, + ); + const relationshipTargets = await getTargetsWithMetadata(); - const targetId = relationships[variable.reference]?.target.id ?? ""; - const targetResource = relationshipTargets[targetId]; - if (targetResource == null) return variable.defaultValue; + const targetId = relationships[variable.reference]?.target.id ?? ""; + const targetResource = relationshipTargets[targetId]; + if (targetResource == null) return variable.defaultValue; - return _.get(targetResource, variable.path, variable.defaultValue); + return _.get(targetResource, variable.path, variable.defaultValue); + } catch (error) { + logger.error("Error resolving reference variable", { error, variable }); + return variable.defaultValue; + } };