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

Unable to destroy (panic) stack when using ArchivePaths in Go #143

Closed
aureq opened this issue Oct 13, 2022 · 6 comments · Fixed by pulumi/pulumi#11053 or #150
Closed

Unable to destroy (panic) stack when using ArchivePaths in Go #143

aureq opened this issue Oct 13, 2022 · 6 comments · Fixed by pulumi/pulumi#11053 or #150
Assignees
@dixler
Labels
impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Milestone
0.80

Comments

@aureq
Copy link
Member

aureq commented Oct 13, 2022

What happened?

As reported internally here, the customer is unable to delete their stack when a pulumi-command uses ArchivePaths: pulumi.StringArray{pulumi.String("*.go")}.

The code provided below, generates this panic.

panic: fatal: An assertion has failed: source is a pointer
    goroutine 52 [running]:
    github.com/pulumi/pulumi/sdk/v3/go/common/util/contract.failfast(...)
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.3/go/common/util/contract/failfast.go:23
    github.com/pulumi/pulumi/sdk/v3/go/common/util/contract.Assertf(0xa0?, {0xd8d097?, 0xbd6aad?}, {0x0?, 0x8?, 0xc9b180?})
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.3/go/common/util/contract/assert.go:33 +0xed
    github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper.(*mapper).DecodeValue(0xc00040d260, 0xc00055c178?, {0xeda548?, 0xd14900}, {0xbd6aad, 0x7}, {0xc000419640?, 0xc00055c178?}, 0x1)
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.3/go/common/util/mapper/mapper_decode.go:97 +0x42c
    github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper.(*mapper).Decode(0xc00040d260, 0x0?, {0xca6ee0?, 0xc00055c120?})
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.3/go/common/util/mapper/mapper_decode.go:55 +0x8db
    github.com/pulumi/pulumi-command/provider/pkg/provider.(*commandProvider).Delete(0xc00009d630, {0xecda88?, 0xc0005549f0?}, 0xc00040d1a0)
        /home/runner/work/pulumi-command/pulumi-command/provider/pkg/provider/provider.go:408 +0x5a5
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Delete_Handler.func1({0xecda88, 0xc0005549f0}, {0xd10680?, 0xc00040d1a0})
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.3/proto/go/provider.pb.go:3800 +0x78
    github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc.OpenTracingServerInterceptor.func1({0xecda88, 0xc0005544e0}, {0xd10680, 0xc00040d1a0}, 0xc0004405c0, 0xc00040ee70)
        /home/runner/go/pkg/mod/github.com/grpc-ecosystem/grpc-opentracing@v0.0.0-20180507213350-8e809c8a8645/go/otgrpc/server.go:57 +0x3f9
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Delete_Handler({0xd54f00?, 0xc00009d630}, {0xecda88, 0xc0005544e0}, 0xc00040d140, 0xc0001d0a00)
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.3/proto/go/provider.pb.go:3802 +0x138
    google.golang.org/grpc.(*Server).processUnaryRPC(0xc0002648c0, {0xed3420, 0xc0003bc340}, 0xc00043c360, 0xc00028ab40, 0x145cb28, 0x0)
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.47.0/server.go:1283 +0xcfe
    google.golang.org/grpc.(*Server).handleStream(0xc0002648c0, {0xed3420, 0xc0003bc340}, 0xc00043c360, 0x0)
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.47.0/server.go:1620 +0xa2f
    google.golang.org/grpc.(*Server).serveStreams.func1.2()
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.47.0/server.go:922 +0x98
    created by google.golang.org/grpc.(*Server).serveStreams.func1
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.47.0/server.go:920 +0x28a

Steps to reproduce

  1. Create a new pulumi-go project
  2. place the code below in your main.go
  3. deploy your pulumi app with pulumi up --yes (it works)
  4. destroy your stack with pulumi destroy --yes (a panic is raised)

Expected Behavior

The stack is deleted as expected.

Actual Behavior

A panic is raised and the stack destruction is interrupted.

Output of pulumi about

CLI          
Version      3.42.0
Go Version   go1.19.1
Go Compiler  gc

Plugins
NAME     VERSION
command  0.5.2
go       unknown

Host     
OS       debian
Version  11.5
Arch     x86_64

This project is written in go: executable='/usr/local/bin/go' version='go version go1.19.2 linux/amd64'

Current Stack: case-xray-001

TYPE                      URN
pulumi:pulumi:Stack       urn:pulumi:case-xray-001::zendesk::pulumi:pulumi:Stack::zendesk-case-xray-001
pulumi:providers:command  urn:pulumi:case-xray-001::zendesk::pulumi:providers:command::default
command:local:Command     urn:pulumi:case-xray-001::zendesk::command:local:Command::stdin


Found no pending operations associated with case-xray-001

Backend        
Name           pulumi.com
URL            https://app.pulumi.com/aureq
User           aureq
Organizations  aureq, team-ce, menfin, demo, pulumi

Dependencies:
NAME                                  VERSION
github.com/pulumi/pulumi-command/sdk  0.5.2
github.com/pulumi/pulumi/sdk/v3       3.39.3

Pulumi locates its logs in /tmp by default

Additional context

main.go

package main

import (
	"github.com/pulumi/pulumi-command/sdk/go/command/local"
	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
)

func main() {
	pulumi.Run(func(ctx *pulumi.Context) error {

		random, err := local.NewCommand(ctx, "stdin", &local.CommandArgs{
			Create:       pulumi.String("head -n 1"),
			Stdin:        pulumi.String("the quick brown fox\njumped over\nthe lazy dog"),
			ArchivePaths: pulumi.StringArray{pulumi.String("*.go")},
		})
		if err != nil {
			return err
		}

		ctx.Export("output", random.Stdout)
		ctx.Export("assets", random.Assets)
		return nil
	})
}

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).
@aureq aureq added kind/bug Some behavior is incorrect or out of spec impact/panic This bug represents a panic or unexpected crash needs-triage Needs attention from the triage team p1 A bug severe enough to be the next item assigned to an engineer labels Oct 13, 2022
@thomas11
Copy link
Contributor

Some debugging notes: when commandProvider.Delete is called, it fails in decoder.Decode(inputs). Decode iterates over the properties of the command struct, and archive is of type pointer, which fails this assertion in DecodeValue:

// So long as the target element is a pointer, we have a pointer to pointer; dig through until we bottom out
// on the non-pointer type that matches the source.  This assumes the source isn't itself a pointer!
contract.Assertf(vsrc.Type().Kind() != reflect.Ptr, "source is a pointer")

@viveklak viveklak removed the needs-triage Needs attention from the triage team label Oct 15, 2022
@viveklak viveklak added this to the 0.79 milestone Oct 15, 2022
@Frassle
Copy link
Member

Frassle commented Oct 17, 2022

and archive is of type pointer, which fails this assertion in DecodeValue:

Bit backwards there. That assert is checking that the value being decoded into the command struct is not a pointer. Which suggests that UnmarshalProperties is sometimes returning a pointer for a string array. Oddly the decode sequence for Create looks exactly the same (it uses MapI but that's just a helper method for the same Decode call that Delete does) but it doesn't panic.

I'll see if I can repro this with engine diagnostics and see what's being sent over the wire.

@Frassle
Copy link
Member

Frassle commented Oct 17, 2022

Ah it's the archive result itself:
archive={&{ 0d6088d06bae9908d3c98b82f68ad29838c9b2e86807b27a0a7dda27c8d86d56 map[main.go:0xc001302640] }}
That's one of the outputs of the command provider, and so it's fed back into Delete and then the mapper chokes on it because it can't handle an archive pointer.

@iwahbe
Copy link
Member

iwahbe commented Oct 17, 2022

Its worth noting that the same problem happens on update as well. It is masked if no changes are made to the resource inputs, but the problem is still there. We can test this by running pulumi up on the example given, changing the Stdin string, and running pulumi up again. We get the same error.

@iwahbe iwahbe mentioned this issue Oct 17, 2022
Merged
3 tasks
bors bot added a commit to pulumi/pulumi that referenced this issue Oct 17, 2022
@bors @iwahbe
Merge #11053
267a050 
11053: Allow decoding *asset and *archive values r=iwahbe a=iwahbe

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

We handle ptr sources in deserialization. This is necessary since `pulumi.New*Asset` and `pulumi.New*Archive` return `*resource.Asset` and `*resource.Archive` respectively.

This will fix pulumi/pulumi-command#143 when it is picked up.

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [X] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [X] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Ian Wahbe <ian@wahbe.com>
bors bot added a commit to pulumi/pulumi that referenced this issue Oct 18, 2022
@bors @iwahbe
Merge #11053
ed46a19 
11053: Allow decoding *asset and *archive values r=iwahbe a=iwahbe

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

We handle ptr sources in deserialization. This is necessary since `pulumi.New*Asset` and `pulumi.New*Archive` return `*resource.Asset` and `*resource.Archive` respectively.

This will fix pulumi/pulumi-command#143 when it is picked up.

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [X] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [X] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Ian Wahbe <ian@wahbe.com>
@AaronFriel
Copy link
Member

@iwahbe this should be closed by pulumi/pulumi#11053?

@AaronFriel
Copy link
Member

Oh, hm, as it statically links to Pu/pu we likely want to update go.mod and cut a release.

@dixler dixler modified the milestones: 0.79, 0.80 Oct 21, 2022
@dixler dixler mentioned this issue Oct 26, 2022
Merged
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dixler dixler

Labels
impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.80
8 participants
@thomas11 @AaronFriel @viveklak @aureq @Frassle @iwahbe @dixler @pulumi-bot