search

pulumi / pulumi-aws

An Amazon Web Services (AWS) Pulumi resource package, providing multi-language access to AWS
Apache License 2.0
464 stars 155 forks source link

Unit testing resources with dangling type refs fails in node #4585

Open flostadler opened 1 month ago

flostadler commented 1 month ago

Describe what happened

When unit testing with jest in node I'm getting failures for resources with dangling type refs:

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From cluster.test.ts.

      at node_modules/@pulumi/iam/index.ts:154:45
      at Object.get [as Role] (node_modules/@pulumi/utilities.ts:65:24)
      at Object.construct (node_modules/@pulumi/iam/index.ts:262:28)
      at deserializeProperty (node_modules/@pulumi/runtime/rpc.ts:655:51)
      at Object.deserializeProperties (node_modules/@pulumi/runtime/rpc.ts:195:24)
      at MockMonitor.<anonymous> (node_modules/@pulumi/runtime/mocks.ts:186:25)
      at node_modules/@pulumi/pulumi/runtime/mocks.js:21:71
      at Object.<anonymous>.__awaiter (node_modules/@pulumi/pulumi/runtime/mocks.js:17:12)
      at MockMonitor.registerResource (node_modules/@pulumi/pulumi/runtime/mocks.js:107:16)
      at node_modules/@pulumi/runtime/resource.ts:557:41
      at Object.<anonymous> (node_modules/@pulumi/runtime/resource.ts:556:29)
      at node_modules/@pulumi/pulumi/runtime/resource.js:21:71
      at Object.<anonymous>.__awaiter (node_modules/@pulumi/pulumi/runtime/resource.js:17:12)
      at node_modules/@pulumi/runtime/resource.ts:549:52
      at node_modules/@pulumi/runtime/resource.ts:1181:24
      at node_modules/@pulumi/pulumi/runtime/resource.js:21:71
      at Object.<anonymous>.__awaiter (node_modules/@pulumi/pulumi/runtime/resource.js:17:12)
      at node_modules/@pulumi/runtime/resource.ts:1176:43
/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/@pulumi/pulumi/runtime/resource.js:303
    const preallocError = new Error();
                          ^

<ref *1> Error: failed to register new resource test-policy [aws:iam/rolePolicyAttachment:RolePolicyAttachment]: exports.Role is not a constructor
    at Object.registerResource (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/@pulumi/runtime/resource.ts:455:27)
    at new Resource (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/@pulumi/resource.ts:518:13)
    at new CustomResource (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/@pulumi/resource.ts:976:9)
    at new RolePolicyAttachment (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/@pulumi/iam/rolePolicyAttachment.ts:127:9)
    at Object.<anonymous> (/Users/flo/development/pulumi-eks/nodejs/eks/cluster.test.ts:48:5)
    at Promise.then.completed (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/utils.js:231:10)
    at _callCircusTest (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/run.js:316:40)
    at _runTest (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/run.js:121:9)
    at run (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-runner/build/runTest.js:367:16)
    at runTest (/Users/flo/development/pulumi-eks/nodejs/eks/node_modules/jest-runner/build/runTest.js:444:34) {
  promise: Promise { <rejected> [Circular *1] }
}

The interesting bit is:

Error: failed to register new resource test-policy [aws:iam/rolePolicyAttachment:RolePolicyAttachment]: exports.Role is not a constructor

The provider currently has multiple resources with dangling refs, see: https://github.com/pulumi/pulumi-aws/issues/2565. One example is the role property of the aws.iam.RolePolicyAttachment resource.

Sample program

import * as pulumi from "@pulumi/pulumi";

describe("createCore", function () {
    beforeAll(() => {
        pulumi.runtime.setMocks(
            {
                newResource: function (args: pulumi.runtime.MockResourceArgs): {
                    id: string;
                    state: any;
                } {
                    return {
                        id: args.inputs.name + "_id",
                        state: args.inputs,
                    };
                },
                call: function (args: pulumi.runtime.MockCallArgs) {
                    return args.inputs;
                },
            },
            "project",
            "stack",
            false, // Sets the flag `dryRun`, which indicates if pulumi is running in preview mode.
        );
    });

    let aws: typeof import("@pulumi/aws");
    beforeEach(async function () {
        aws = await import("@pulumi/aws");
    });

    test("union type", async () => {
        new aws.iam.RolePolicyAttachment("test-policy",
            {
                policyArn: "test",
                role: "test", //works
            },
        );

        const role = new aws.iam.Role("test-role", {
            assumeRolePolicy: aws.iam.assumeRolePolicyForPrincipal({
                Service: "ec2.amazonaws.com",
            }),
        });

        new aws.iam.RolePolicyAttachment("test-policy",
            {
                policyArn: "test",
                role: role, //fails
            },
        );
    });
});

Log output

No response

Affected Resource(s)

No response

Output of pulumi about

pulumi v3.134.1

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

t0yv0 commented 1 month ago

Indeed, this is very unfortunate. Are some of the resources more important for your use case than other resources? Trying to see what our options are here, fixing the entire list necessitates breaking changes that need to wait until the v7, but some of the smaller ones can be possibly tidied up before v7 taking a minor break. Full list:

schema_test.go:38: Dangling reference: aws:alb/ipAddressType:IpAddressType (isRes=false)
    schema_test.go:38: Dangling reference: aws:alb/loadBalancerType:LoadBalancerType (isRes=false)
    schema_test.go:38: Dangling reference: aws:apigateway/deployment:Deployment (isRes=true)
    schema_test.go:38: Dangling reference: aws:apigateway/restApi:RestApi (isRes=true)
    schema_test.go:38: Dangling reference: aws:autoscaling/metrics:Metric (isRes=false)
    schema_test.go:38: Dangling reference: aws:autoscaling/notificationType:NotificationType (isRes=false)
    schema_test.go:38: Dangling reference: aws:cloudwatch/logGroup:LogGroup (isRes=true)
    schema_test.go:38: Dangling reference: aws:ec2/launchConfiguration:LaunchConfiguration (isRes=true)
    schema_test.go:38: Dangling reference: aws:ec2/placementGroup:PlacementGroup (isRes=true)
    schema_test.go:38: Dangling reference: aws:ecr/lifecyclePolicyDocument:LifecyclePolicyDocument (isRes=false)
    schema_test.go:38: Dangling reference: aws:elasticbeanstalk/application:Application (isRes=true)
    schema_test.go:38: Dangling reference: aws:elasticbeanstalk/applicationVersion:ApplicationVersion (isRes=true)
    schema_test.go:38: Dangling reference: aws:iam/Role:Role (isRes=false)
    schema_test.go:38: Dangling reference: aws:iam/documents:PolicyDocument (isRes=false)
    schema_test.go:38: Dangling reference: aws:iam/group:Group (isRes=true)
    schema_test.go:38: Dangling reference: aws:iam/instanceProfile:InstanceProfile (isRes=true)
    schema_test.go:38: Dangling reference: aws:iam/role:Role (isRes=true)
    schema_test.go:38: Dangling reference: aws:iam/user:User (isRes=true)
    schema_test.go:38: Dangling reference: aws:index/aRN:ARN (isRes=false)
    schema_test.go:38: Dangling reference: aws:index/region:Region (isRes=false)
    schema_test.go:38: Dangling reference: aws:iot/policy:Policy (isRes=true)
    schema_test.go:38: Dangling reference: aws:lambda/function:Function (isRes=true)
    schema_test.go:38: Dangling reference: aws:rds/engineType:EngineType (isRes=false)
    schema_test.go:38: Dangling reference: aws:s3/bucket:Bucket (isRes=true)
    schema_test.go:38: Dangling reference: aws:s3/routingRules:RoutingRule (isRes=false)
    schema_test.go:38: Dangling reference: aws:sns/topic:Topic (isRes=true)

The remediation for https://github.com/pulumi/pulumi-aws/issues/2565 we're looking at right now produces a side-by-side schema that has the references corrected, that does not fix the SDKs though (intentionally).

Let me have a quick look at the repro to see if there's any other remediation ideas.

t0yv0 commented 1 month ago

As a workaround it should be possible to unit test in languages other than Node where the overlays are not present perhaps, like Python.

t0yv0 commented 1 month ago

From 

        new aws.iam.RolePolicyAttachment("test-policy",
            {
                policyArn: "test",
                role: "test", //works
            },
        );

Another workaround would be to use the string-passing forms instead of the overlay union types correct?

flostadler commented 1 month ago

@t0yv0 For my use cases with the MLCs we cannot switch the language as they're already written in node. For other users I feel like the language decision shouldn't be made based on this, but rather what they're most comfortable using. I.e., re-writing their infra to another language isn't really anything I'd expect users to do in order to work around this bug.

Changing the code to use the string passing forms is definitely a viable workaround. Having to make code changes to get tests to work is not really intuitive for users though. Especially because the error message doesn't hint at this being the issue at all.

In terms of what dangling refs have the biggest impact, I'd say it's the iam resources (Role, InstanceProfile, et al.) as they're some of the most commonly used resources. At the same time this most likely means we cannot take a breaking change because they're so widely used.