From 64488644890929ef22f2470cf36cc52dee07ee3d Mon Sep 17 00:00:00 2001 From: Kyle Pitzen Date: Mon, 31 Oct 2022 14:25:24 -0400 Subject: [PATCH] feat(sdk/nodejs): delegates alias computation to the engine --- ...as-computation-to-engine-for-node-sdk.yaml | 4 + sdk/nodejs/resource.ts | 53 ------------ sdk/nodejs/runtime/resource.ts | 34 +++++++- sdk/nodejs/tests/resource.spec.ts | 86 +------------------ .../072.large_alias_lineage_chains/index.js | 29 +++++++ sdk/nodejs/tests/runtime/langhost/run.spec.ts | 15 ++-- 6 files changed, 71 insertions(+), 150 deletions(-) create mode 100644 changelog/pending/20221031--sdk-nodejs--delegates-alias-computation-to-engine-for-node-sdk.yaml diff --git a/changelog/pending/20221031--sdk-nodejs--delegates-alias-computation-to-engine-for-node-sdk.yaml b/changelog/pending/20221031--sdk-nodejs--delegates-alias-computation-to-engine-for-node-sdk.yaml new file mode 100644 index 000000000000..05d37f8cfae9 --- /dev/null +++ b/changelog/pending/20221031--sdk-nodejs--delegates-alias-computation-to-engine-for-node-sdk.yaml @@ -0,0 +1,4 @@ +changes: +- type: feat + scope: sdk/nodejs + description: Delegates alias computation to engine for Node SDK diff --git a/sdk/nodejs/resource.ts b/sdk/nodejs/resource.ts index 2534f2318e8e..3bb3ce68e8a3 100644 --- a/sdk/nodejs/resource.ts +++ b/sdk/nodejs/resource.ts @@ -17,7 +17,6 @@ import { Input, Inputs, interpolate, Output, output } from "./output"; import { getResource, readResource, registerResource, registerResourceOutputs } from "./runtime/resource"; import { getProject, getStack } from "./runtime/settings"; import { getStackResource } from "./runtime/state"; -import * as stacklib from "./runtime/stack"; import { unknownValue } from "./runtime/rpc"; import * as utils from "./utils"; import * as log from "./log"; @@ -52,32 +51,6 @@ export function createUrn(name: Input, type: Input, parent?: Res return interpolate`${parentPrefix}${type}::${name}`; } -// inheritedChildAlias computes the alias that should be applied to a child based on an alias applied to it's parent. -// This may involve changing the name of the resource in cases where the resource has a named derived from the name of -// the parent, and the parent name changed. -function inheritedChildAlias(childName: string, parentName: string, parentAlias: Input, childType: string): Output { - // If the child name has the parent name as a prefix, then we make the assumption that it was - // constructed from the convention of using `{name}-details` as the name of the child resource. To - // ensure this is aliased correctly, we must then also replace the parent aliases name in the prefix of - // the child resource name. - // - // For example: - // * name: "newapp-function" - // * opts.parent.__name: "newapp" - // * parentAlias: "urn:pulumi:stackname::projectname::awsx:ec2:Vpc::app" - // * parentAliasName: "app" - // * aliasName: "app-function" - // * childAlias: "urn:pulumi:stackname::projectname::aws:s3/bucket:Bucket::app-function" - let aliasName = output(childName); - if (childName.startsWith(parentName)) { - aliasName = output(parentAlias).apply(parentAliasUrn => { - const parentAliasName = parentAliasUrn.substring(parentAliasUrn.lastIndexOf("::") + 2); - return parentAliasName + childName.substring(parentName.length); - }); - } - return createUrn(aliasName, childType, parentAlias); -} - // Extract the type and name parts of a URN function urnTypeAndName(urn: URN) { const parts = urn.split("::"); @@ -88,29 +61,6 @@ function urnTypeAndName(urn: URN) { }; } -// Make a copy of the aliases array, and add to it any implicit aliases inherited from its parent. -// If there are N child aliases, and M parent aliases, there will be (M+1)*(N+1)-1 total aliases, -// or, as calculated in the logic below, N+(M*(1+N)). -export function allAliases(childAliases: Input[], childName: string, childType: string, parent: Resource, parentName: string): Output[] { - const aliases: Output[] = []; - for (const childAlias of childAliases) { - aliases.push(collapseAliasToUrn(childAlias, childName, childType, parent)); - } - for (const parentAlias of (parent.__aliases || [])) { - // For each parent alias, add an alias that uses that base child name and the parent alias - aliases.push(inheritedChildAlias(childName, parentName, parentAlias, childType)); - // Also add an alias for each child alias and the parent alias - for (const childAlias of childAliases) { - const inheritedAlias = collapseAliasToUrn(childAlias, childName, childType, parent).apply(childAliasURN => { - const {name: aliasedChildName, type: aliasedChildType} = urnTypeAndName(childAliasURN); - return inheritedChildAlias(aliasedChildName, parentName, parentAlias, aliasedChildType); - }); - aliases.push(inheritedAlias); - } - } - return aliases; -} - /** * Resource represents a class whose CRUD operations are implemented by a provider plugin. */ @@ -333,9 +283,6 @@ export abstract class Resource { opts.protect = parent.__protect; } - // Update aliases to include the full set of aliases implied by the child and parent aliases. - opts.aliases = allAliases(opts.aliases || [], name, t, parent, parent.__name!); - this.__providers = parent.__providers; } diff --git a/sdk/nodejs/runtime/resource.ts b/sdk/nodejs/runtime/resource.ts index b94d2700bb91..aefdfef0d4f7 100644 --- a/sdk/nodejs/runtime/resource.ts +++ b/sdk/nodejs/runtime/resource.ts @@ -20,6 +20,7 @@ import * as utils from "../utils"; import { getAllResources, Input, Inputs, Output, output } from "../output"; import { ResolvedResource } from "../queryable"; import { + Alias, ComponentResource, ComponentResourceOptions, createUrn, @@ -59,6 +60,7 @@ import { const gstruct = require("google-protobuf/google/protobuf/struct_pb.js"); const resproto = require("../proto/resource_pb.js"); +const aliasproto = require("../proto/alias_pb.js"); interface ResourceResolverOperation { // A resolver for a resource's URN. @@ -83,7 +85,7 @@ interface ResourceResolverOperation { // all be URNs of custom resources, not component resources. propertyToDirectDependencyURNs: Map>; // A list of aliases applied to this resource. - aliases: URN[]; + aliases: (Alias | URN)[]; // An ID to import, if any. import: ID | undefined; } @@ -272,6 +274,28 @@ export function readResource(res: Resource, parent: Resource | undefined, t: str }), label); } +function mapAliasesForRequest(aliases: (string | Alias)[] | undefined) { + if (aliases === undefined) { + return []; + } + + return aliases.map(a => { + const newAlias = new aliasproto.Alias(); + if (typeof a === "string") { + newAlias.setUrn(a); + } else { + const newAliasSpec = new aliasproto.Alias.Spec(); + newAliasSpec.setName(a.name); + newAliasSpec.setType(a.type); + newAliasSpec.setStack(a.stack); + newAliasSpec.setProject(a.project); + newAliasSpec.setParenturn(a.parent); + newAlias.setSpec(newAliasSpec); + } + return newAlias; + }); +} + /** * registerResource registers a new resource object with a given type t and name. It returns the auto-generated * URN and the ID that will resolve after the deployment has completed. All properties will be initialized to property @@ -294,6 +318,8 @@ export function registerResource(res: Resource, parent: Resource | undefined, t: log.debug(`RegisterResource RPC prepared: t=${t}, name=${name}` + (excessiveDebugOutput ? `, obj=${JSON.stringify(resop.serializedProps)}` : ``)); + const reqAliases = mapAliasesForRequest(resop.aliases); + const req = new resproto.RegisterResourceRequest(); req.setType(t); req.setName(name); @@ -310,7 +336,7 @@ export function registerResource(res: Resource, parent: Resource | undefined, t: req.setAcceptsecrets(true); req.setAcceptresources(!utils.disableResourceReferences); req.setAdditionalsecretoutputsList((opts).additionalSecretOutputs || []); - req.setAliasurnsList(resop.aliases); + req.setAliasesList(reqAliases); req.setImportid(resop.import || ""); req.setSupportspartialvalues(true); req.setRemote(remote); @@ -579,8 +605,8 @@ async function prepareResource(label: string, res: Resource, parent: Resource | // in the Resource constructor prior to calling `registerResource` - both adding new inherited aliases and // simplifying aliases down to URNs. const aliases = []; - const uniqueAliases = new Set(); - for (const alias of (res.__aliases || [])) { + const uniqueAliases = new Set(); + for (const alias of (opts.aliases || [])) { const aliasVal = await output(alias).promise(); if (!uniqueAliases.has(aliasVal)) { uniqueAliases.add(aliasVal); diff --git a/sdk/nodejs/tests/resource.spec.ts b/sdk/nodejs/tests/resource.spec.ts index 29f87b3f5483..a3bf6dd4c174 100644 --- a/sdk/nodejs/tests/resource.spec.ts +++ b/sdk/nodejs/tests/resource.spec.ts @@ -15,10 +15,9 @@ /* eslint-disable */ import * as assert from "assert"; -import { all } from "../output"; import * as runtime from "../runtime"; import { asyncTest } from "./util"; -import { allAliases, createUrn, ComponentResource, CustomResourceOptions, DependencyProviderResource } from "../resource"; +import { createUrn, ComponentResource, CustomResourceOptions, DependencyProviderResource } from "../resource"; class MyResource extends ComponentResource { constructor(name: string, opts?: CustomResourceOptions) { @@ -69,89 +68,6 @@ class TestResource extends ComponentResource { } } -describe("allAliases", () => { - before(() => { - runtime._setProject("project"); - runtime._setStack("stack"); - }); - - after(() => { - runtime._setProject(undefined); - runtime._setStack(undefined); - }) - - const testCases = [ - { - name: "no aliases", - parentAliases: [], - childAliases: [], - results: [], - }, - { - name: "one child alias (type), no parent aliases", - parentAliases: [], - childAliases: [{ type: "test:resource:child2" }], - results: ["urn:pulumi:stack::project::test:resource:type$test:resource:child2::myres-child"], - }, - { - name: "one child alias (name), no parent aliases", - parentAliases: [], - childAliases: [{ name: "child2" }], - results: ["urn:pulumi:stack::project::test:resource:type$test:resource:child::child2"], - }, - { - name: "one child alias (name), one parent alias (type)", - parentAliases: [{type: "test:resource:type3"}], - childAliases: [{name: "myres-child2"}], - results: [ - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres-child2", - "urn:pulumi:stack::project::test:resource:type3$test:resource:child::myres-child", - "urn:pulumi:stack::project::test:resource:type3$test:resource:child::myres-child2", - ], - }, - { - name: "one child alias (name), one parent alias (name)", - parentAliases: [{name: "myres2"}], - childAliases: [{name: "myres-child2"}], - results: [ - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres-child2", - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres2-child", - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres2-child2", - ], - }, - { - name: "two child aliases, three parent aliases", - parentAliases: [{ name: "myres2" }, { type: "test:resource:type3" }, { name: "myres3" }], - childAliases: [{ name: "myres-child2" }, { type: "test:resource:child2" }], - results: [ - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres-child2", - "urn:pulumi:stack::project::test:resource:type$test:resource:child2::myres-child", - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres2-child", - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres2-child2", - "urn:pulumi:stack::project::test:resource:type$test:resource:child2::myres2-child", - "urn:pulumi:stack::project::test:resource:type3$test:resource:child::myres-child", - "urn:pulumi:stack::project::test:resource:type3$test:resource:child::myres-child2", - "urn:pulumi:stack::project::test:resource:type3$test:resource:child2::myres-child", - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres3-child", - "urn:pulumi:stack::project::test:resource:type$test:resource:child::myres3-child2", - "urn:pulumi:stack::project::test:resource:type$test:resource:child2::myres3-child", - ], - }, - ]; - - for (const testCase of testCases) { - it(testCase.name, asyncTest(async () => { - const res = new TestResource("myres", { aliases: testCase.parentAliases }); - const aliases = allAliases(testCase.childAliases, "myres-child", "test:resource:child", res, "myres"); - assert.strictEqual(aliases.length, testCase.results.length); - const aliasURNs = await all(aliases).promise(); - for (let i = 0; i < aliasURNs.length; i++) { - assert.strictEqual(aliasURNs[i], testCase.results[i]); - } - })); - } -}); - describe("DependencyProviderResource", () => { describe("getPackage", () => { it("returns the expected package", () => { diff --git a/sdk/nodejs/tests/runtime/langhost/cases/072.large_alias_lineage_chains/index.js b/sdk/nodejs/tests/runtime/langhost/cases/072.large_alias_lineage_chains/index.js index 23cdb785836a..ac595b163a58 100644 --- a/sdk/nodejs/tests/runtime/langhost/cases/072.large_alias_lineage_chains/index.js +++ b/sdk/nodejs/tests/runtime/langhost/cases/072.large_alias_lineage_chains/index.js @@ -18,10 +18,39 @@ class MyResource extends pulumi.CustomResource { } } +class MyOtherResource extends pulumi.CustomResource { + constructor(name, aliases, parent) { + super( + "test:index:MyOtherResource", + name, + {}, + { + aliases: aliases, + parent + } + ); + } +} + const resource1Aliases = Array.from(new Array(1000).keys()).map(key => `my-alias-name-${key}`); const resource1 = new MyResource("testResource1", resource1Aliases); resource1.__aliases.map(alias => alias.apply(aliasName => assert(resource1Aliases.includes(aliasName)))); +assert.equal(resource1.__aliases.length, 1000); const resource2Aliases = Array.from(new Array(1000).keys()).map(key => `my-other-alias-name-${key}`); const resource2 = new MyResource("testResource2", resource2Aliases, resource1) resource2.__aliases.map(alias => alias.apply(aliasName => assert(resource2Aliases.includes(aliasName)))); +assert.equal(resource2.__aliases.length, 1000); + +const resource3Aliases = Array.from(new Array(1000).keys()).map(key => { + return { + name: `my-alias-${key}`, + stack: "my-stack", + project: "my-project", + type: "test:index:MyOtherResource", + } +}); +const resource3 = new MyOtherResource("testResource2", resource3Aliases, resource2) +assert.equal(resource3.__aliases.length, 1000); +// We want to ensure that the parent's type is included in computed aliases from the engine +resource3.__aliases[0].apply(aliasName => assert(aliasName.includes("test:index:MyResource"))); diff --git a/sdk/nodejs/tests/runtime/langhost/run.spec.ts b/sdk/nodejs/tests/runtime/langhost/run.spec.ts index 72013bfade05..1a40380a5532 100644 --- a/sdk/nodejs/tests/runtime/langhost/run.spec.ts +++ b/sdk/nodejs/tests/runtime/langhost/run.spec.ts @@ -1249,14 +1249,13 @@ describe("rpc", () => { return { urn: makeUrn(t, name), id: undefined, props: undefined }; }, }, - /** Skipping this test case as it requires limiting the alias multiplication which occurs */ - // "large_alias_lineage_chains": { - // program: path.join(base, "072.large_alias_lineage_chains"), - // expectResourceCount: 1, - // registerResource: (ctx: any, dryrun: boolean, t: string, name: string, res: any, ...args: any) => { - // return { urn: makeUrn(t, name), id: undefined, props: undefined }; - // }, - // } + "large_alias_lineage_chains": { + program: path.join(base, "072.large_alias_lineage_chains"), + expectResourceCount: 3, + registerResource: (ctx: any, dryrun: boolean, t: string, name: string, res: any, ...args: any) => { + return { urn: makeUrn(t, name), id: undefined, props: undefined }; + }, + }, }; for (const casename of Object.keys(cases)) {