Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
flostadler opened this issue Sep 30, 2024 · 4 comments
Open

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

flostadler opened this issue Sep 30, 2024 · 4 comments
Labels
kind/bug Some behavior is incorrect or out of spec

Comments

@flostadler
Copy link
Contributor

flostadler commented Sep 30, 2024 •
edited
Loading

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: #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).
@flostadler flostadler added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team and removed needs-triage Needs attention from the triage team labels Sep 30, 2024
@t0yv0
Copy link
Member

t0yv0 commented Oct 1, 2024

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 #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
Copy link
Member

t0yv0 commented Oct 1, 2024

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
Copy link
Member

t0yv0 commented Oct 1, 2024

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
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@t0yv0 @flostadler