pulumi / examples

Infrastructure, containers, and serverless apps to AWS, Azure, GCP, and Kubernetes... all deployed with Pulumi
https://www.pulumi.com
Apache License 2.0
2.33k stars 874 forks source link

TestAccAwsPyEc2Provisioners test failing #1491

Closed t0yv0 closed 10 months ago

t0yv0 commented 11 months ago

What happened?

TestAccAwsPyEc2Provisioners test is failing in CI on master.

I've looked into this and here's the problem. There's two versions of this program, one in TS and one in Python.

With recent changes to pulumi-command, if you are on 0.8.2, the following program:

// Execute a basic command on our server.
const catConfig = new command.remote.Command("cat-config", {
    triggers: [changeToken],
    connection,
    create: "cat myapp.conf",
}, { dependsOn: cpConfig });

export const publicIp = server.publicIp;
export const publicHostName = server.publicDns;
export const catConfigStdout = catConfig.stdout;

now marks catConfig.Stdout as a secret output whereas it didn't before.

Now, Node test pins Pulumi-command 0.0.3 or some such, while Python test floats to the latest reference. Therefore Node test passes but Python test fails.

The specific failure is a panic when reading the secret value in ProgramTest Go code. There's an assert that tries to convert catConfigStdout to a string, but it is now a map in Go land. AFAIK Go ProgramTest cannot yet peek inside secret values returned from the stack.

Expected Behavior

Passing test.

Steps to reproduce

See above.

Output of pulumi about

N/A

Additional context

We had some discussion and dug up this change https://github.com/pulumi/pulumi-go-provider/pull/105/files#diff-4bd4b3f761d7688f63a5a8e766057562d08e3df2f51cfa88e604cb4e353fd042R318 as possible root cause.

Need to decide if this semantic is reasonable for Command and we want to keep it or revert back. If keeping this semantic, we need to adjust the test here to not fail on receiving a secret.

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

scottslowe commented 10 months ago

1492 pins the Python version to fix the test failure

t0yv0 commented 10 months ago

Hi @scottslowe thanks so much, forgive me for not finding time to contribute a workaround here. it makes total sense for you to keep this repo with passing tests and pinning the version is 👍

I've created an issue for my team so we can close this one without losing track of the actual breaking change. I don't think we got to a good conclusion of whether the new behavior is correct or needs to be reverted in Pulumi-command. https://github.com/pulumi/pulumi-command/issues/256

Thanks again and sorry for the disruption.