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.

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

TestAccAwsPyEc2Provisioners test failing #1491

Closed
t0yv0 opened this issue Aug 29, 2023 · 2 comments · Fixed by #1497
Closed

TestAccAwsPyEc2Provisioners test failing #1491

t0yv0 opened this issue Aug 29, 2023 · 2 comments · Fixed by #1497
Assignees
Labels
area/examples kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed size/S Estimated effort to complete (1-2 days).
Milestone

Comments

@t0yv0
Copy link
Member

t0yv0 commented Aug 29, 2023

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

@t0yv0 t0yv0 added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Aug 29, 2023
@scottslowe scottslowe added area/examples and removed needs-triage Needs attention from the triage team labels Sep 1, 2023
@scottslowe
Copy link
Contributor

#1492 pins the Python version to fix the test failure

@t0yv0
Copy link
Member Author

t0yv0 commented Sep 1, 2023

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.
pulumi/pulumi-command#256

Thanks again and sorry for the disruption.

@cnunciato cnunciato self-assigned this Sep 12, 2023
@cnunciato cnunciato added this to the 0.94 milestone Sep 12, 2023
@cnunciato cnunciato added the size/S Estimated effort to complete (1-2 days). label Sep 12, 2023
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/examples kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed size/S Estimated effort to complete (1-2 days).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants