Fix missing outputs in all languages

[danielrbradley]

Register missing state to make it available to each language

Fixes #12

[danielrbradley]

Currently strugging to get the integration tests to pass as trying to install the dev version of the provider hangs the CLI. @lukehoban is your approval for the overall approach or have you managed to run the tests and see them pass?

[lukehoban]

Automatically detect new package version instead of manually using pulumi plugin install resource aws-apigateway ...

You should just need to do a make install and then ensure ./bin is on your PATH so that the provider is picked up from what was built. Cc @pulumi/platform-integrations for help with getting this setup in the same way we got all other packages (which ideally should be pulled into the boilerplates so that it is setup by default in new packages). We should not be manually plug-in installing - as that would user a different interface plug-in than the one that we are building/testing.

[danielrbradley]

A side issue which is blocking the Go implementation is that the types for the output sub-properties are missing their fields (either that or my go-fu is lacking in how to access the fields).

Here's the only fields available on the Api property:

It would correlate with either the types being defined references from another package, or that fact they're complex outputs and not just strings etc.

Here's a snippet of the generated code:

type StageOutput struct{ *pulumi.OutputState }

func (StageOutput) ElementType() reflect.Type {
    return reflect.TypeOf((*Stage)(nil))
}

The Stage type is referenced correctly from the other package, but it appears the StageOutput type lacks the extension of the Stage type.

[lukehoban]

A side issue which is blocking the Go implementation is that the types for the output sub-properties are missing their fields (either that or my go-fu is lacking in how to access the fields).

Yes - this is known and expected - though not ideal. As things currently stand - you need to write code like this:

ctx.Export("stageName", restAPI.Stage.ApplyT(func(stage *awsapigateway.Stage) pulumi.StringOutput {
    return stage.StageName
}))

Or if you want to pass this value into something typed as pulumi.String, use the approach in https://github.com/pulumi/pulumi/issues/6073#issuecomment-809669645. This is not specific to this component, it's a general aspect of how MLC works (and how the Pulumi Go API works).

[danielrbradley]

When testing python locally, creating an API, I'm seeing the following error:

AssertionError: Unexpected type; expected a value of type `<class 'pulumi_aws.apigateway.deployment.Deployment'>` but got a value of type `<class 'dict'>` at resource `api`, property `deployment`: {'restApi': 'jjs1tz8fvh', 'urn': 'urn:pulumi:dev::aws-apigateway-py-routes::apigateway:index:RestAPI$aws:apigateway/deployment:Deployment::api', 'createdDate': '2021-12-02T10:50:00Z', 'description': '', 'invokeUrl': 'https://jjs1tz8fvh.execute-api.eu-west-2.amazonaws.com/', 'variables': {'version': '6a3a48e0'}, 'executionArn': 'arn:aws:execute-api:eu-west-2:616138583583:jjs1tz8fvh/', 'id': 'zsbxsc', 'stageName': ''}

This appears to be a failure when trying to decode the nested types. This previously didn't cause an error when these properties were not present in the state - the presence of the value triggers the error.

This is potentially a worse situation that we have right now as it makes the library unusable in Python - this failure occurs even if we don't use the output values.

[justinvp]

When testing python locally, creating an API, I'm seeing the following error

You're running into this because of the unwrapping when setting the state. These are typed as resources in the schema and should be set in the state as those resources to be serialized as resource references. Instead, you're converting the resources into plain objects through those .applys. When Python is then deserializing the value, it's raising an error because the Python type annotation is a resource, but the actual value is a dictionary, when it should be a resource reference.

[danielrbradley]

Python is then deserializing the value, it's raising an error because the Python type annotation is a resource, but the actual value is a dictionary, when it should be a resource reference.

This makes sense - I can see two possible routes here:

1. Fix the underlying way the Typescript provider encodes the resource classes so it includes all fields correctly. No idea how involved this would be.
2. Temporary workaround to manually include the type information .. seeing some references to signatures which I'm guessing is the mechanism used when decoding the properties recursively?

[danielrbradley]

It looks like the original issue with serializing the types is broken in serializeProperty: https://github.com/pulumi/pulumi/blob/master/sdk/nodejs/runtime/rpc.ts#L302

1. Output<OurSubResource> is converted into a Promise

   ```typescript
   const value = await serializeProperty(`${ctx}.id`, prop.promise(), promiseDeps, {
       keepOutputValues: false,
   });
   ```

2. The Promise<OurSubResource> is awaited and unwrapped

   ```typescript
   const subctx = `Promise<${ctx}>`;
   return serializeProperty(subctx,
       await debuggablePromise(prop, `serializeProperty.await(${subctx})`), dependentResources, opts);
   ```

3. OurSubResource is detected as a CustomResource. The id is serialized. The monitor does appear to support resource references, so the id is returned (note here says this is for backward compatibility).

   ```typescript
   const id = await serializeProperty(`${ctx}.id`, prop.id, dependentResources ...);
   
   if (await monitorSupportsResourceReferences()) {
       // snip
   }
   // Else, return the id for backward compatibility.
   return id;
   ```

This would align with the original behaviour seen - where the id was serialized for each output resource. Still need to definitively prove this is the source - if the monitor is returning false here or perhaps it's flowing through a different route.

[justinvp]

I spent a little time debugging this and the problem appears to be a weird issue with the way yarn link @pulumi/aws-apigateway works from a TypeScript program.

Before I get to that, I changed how the state is returned to not do the unwrapping:

    return {
        urn: restAPI.urn,
        state: {
            url: restAPI.url,
            api: restAPI.api,
            deployment: restAPI.deployment,
            stage: restAPI.stage,
            apiPolicy: restAPI.apiPolicy,
        },
    };

With that, running the following Python program works as expected:

import json
import pulumi
import pulumi_aws as aws
import pulumi_aws_apigateway as apigateway

role = aws.iam.Role("mylambda-role",
    assume_role_policy=json.dumps({
        "Version": "2012-10-17",
        "Statement": [{
            "Effect": "Allow",
            "Principal": { "Service": "lambda.amazonaws.com" },
            "Action": "sts:AssumeRole"
        }]
    })
)

policy = aws.iam.RolePolicy("mylambda-policy",
    role=role.id,
    policy=json.dumps({
        "Version": "2012-10-17",
        "Statement": [{
            "Action": ["logs:*", "cloudwatch:*"],
            "Resource": "*",
            "Effect": "Allow",
        }],
    }))

f = aws.lambda_.Function("mylambda",
    runtime=aws.lambda_.Runtime.PYTHON3D8,
    code=pulumi.AssetArchive({
        ".": pulumi.FileArchive("./handler"),
    }),
    timeout=300,
    handler="handler.handler",
    role=role.arn,
    opts=pulumi.ResourceOptions(depends_on=[policy]),
)

api = apigateway.RestAPI('api', routes=[
    apigateway.RouteArgs(path="/", method=apigateway.Method.GET, event_handler=f),
    apigateway.RouteArgs(path="/www", method=apigateway.Method.GET, local_path="www", index=False),
    apigateway.RouteArgs(path="/integration", target=apigateway.TargetArgs(uri="https://www.google.com", type=apigateway.IntegrationType.HTTP_PROXY))
])

api_key = aws.apigateway.ApiKey("api-key")

usage_plan = aws.apigateway.UsagePlan("usage-plan",
    api_stages= [aws.apigateway.UsagePlanApiStageArgs(
        api_id = api.api.id,
        stage = api.stage.stage_name,
    )],
)
aws.apigateway.UsagePlanKey("usage-plan-key",
    key_id = api_key.id,
    key_type = "API_KEY",
    usage_plan_id = usage_plan.id,
)

pulumi.export('url', api.url)

Now, getting back to the TypeScript program... When I run an initial pulumi preview, I get the following errors:

    error: aws:apigateway/usagePlan:UsagePlan resource 'usage-plan' has a problem: Missing required argument: The argument "api_stages.0.api_id" is required, but no definition was found.. Examine values at 'UsagePlan.ApiStages'.
    error: aws:apigateway/usagePlan:UsagePlan resource 'usage-plan' has a problem: Missing required argument: The argument "api_stages.0.stage" is required, but no definition was found.. Examine values at 'UsagePlan.ApiStages'.

Is this originally what wasn't working for you?

The root problem is along the lines of what you describe above, though, the resources are being serialized correctly as resource references. The problem is during deserialization: we try to "rehydrate" the resource from the resource ref (creating an instance of the resource and filling in its state):

https://github.com/pulumi/pulumi/blob/eb735c7106fcbb48eb3e0edaab3d0baea47d2f0e/sdk/nodejs/runtime/rpc.ts#L631-L633

But a resourceModule for theses resource refs isn't found, so it's falling back to deserializing the resources as their urn. So the resource types from aws aren't being registered. This is strange because our program explicitly has an import * as aws from "@pulumi/aws"; at the top, which should result in all the aws resources being registered.

Well, they are being registered from the program. The problem is there are two copies of @pulumi/pulumi being used by the TypeScript program (not counting the copy used by the MLC provider).

1. The copy being used by the program is in: ./examples/simple/node_modules/@pulumi/pulumi.

2. The copy being used by apigateway.RestAPI is in: ./sdk/nodejs/node_modules/@pulumi/pulumi. This is because I used yarn link @pulumi/aws-apigateway in ./examples/simple to use the locally built package from my program.

The resource registrations are present in (1) because the program imports @pulumi/aws. However, the resource registrations are not present in (2).

Despite ./sdk/nodejs/restAPI.ts having a import * as pulumiAws from "@pulumi/aws"; at the top, if I look at the compiled restAPI.js, it doesn't actually include a require("@pulumi/aws");, since it was only using @pulumi/aws for type definitions. Since it isn't imported at runtime, the resource registrations aren't happening here. If I manually insert a require("@pulumi/aws"); in restAPI.js, then the program works as expected.

So what can we do to address the problem? We either need to come up with a way where the test is only going to be using a single copy of @pulumi/pulumi for both the local @pulumi/aws-apigateway and the program (possibly by changing @pulumi/pulumi in @pulumi/aws-gateway's package.json to be a peerDependency rather than a dependency; though I haven't had a chance to try this yet), or come up with a way to force RestAPI to require("@pulumi/aws") at runtime to ensure the resource registrations happen in @pulumi/aws-apigateway's copy of @pulumi/pulumi.

[danielrbradley]

I can confirm the Python works perfectly with the change. After some struggle I've also confirmed that the Go version works fine with the change.

I've tried making the pulumi packages peer dependencies, but my local testing still appears to not have the values. Were you indicating this is likely only a problem for Typescript when testing locally or would this likely have the same problem if released? If it's the prior then would you feel comfortable cutting the release with just the minimal change and go from there? The worst case scenario of releasing as-is would be that Typescript still has the bug, but Python and Go are then fixed (I'm yet to put the test together for dotnet too).

[danielrbradley]

@justinvp you happy with this going in as-is? Is my reading of your comment correct that this mainly affects when being referenced locally or will this require further changes to get the published TS version working?

[justinvp]

you happy with this going in as-is?

I'm fine with it. The issue should only happen when attempting to test the Node.js package locally with yarn link and shouldn't affect anyone installing the package from npm. I think we can just temporarily skip any Node.js tests that check these outputs until we have a good solution. But we should be able to test the other languages. And should be able to have a basic Node.js sanity test that doesn't check those outputs.