Passing malformed data to apimachinery causes panic

[hausdorff]

This is relatively unlikely for TS users (because types), but could in theory accidentally be done by Python and TS users. The panic looks like:

  pulumi:pulumi:Stack (exposed-deployment-exp-dev):
    panic: cannot deep copy *resource.Asset
    goroutine 370 [running]:
    github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.DeepCopyJSONValue(0x2230b00, 0xc0007d56d0, 0xc00088044c, 0x4)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/converter.go:474 +0x572
    github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.DeepCopyJSONValue(0x20bd200, 0xc0007d8c30, 0xc0008804c0, 0xa)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/converter.go:458 +0x149
    github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.DeepCopyJSONValue(0x20bd200, 0xc0007d8c00, 0x0, 0x0)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/converter.go:458 +0x149
    github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.DeepCopyJSON(...)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/converter.go:443
    github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured.(*Unstructured).DeepCopy(0xc0011cc108, 0xc00102a000)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go:146 +0x76
    github.com/pulumi/pulumi-kubernetes/pkg/provider.withLastAppliedConfig(0xc0011cc108, 0xc0011cc108, 0x0, 0xc0007d8c00)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/pkg/provider/provider.go:1309 +0x85
    github.com/pulumi/pulumi-kubernetes/pkg/provider.(*kubeProvider).Create(0xc000172070, 0x2506c00, 0xc0007d88d0, 0xc0002ff200, 0xc000172070, 0x20e5501, 0xc0002ff340)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/pkg/provider/provider.go:735 +0x39d
    github.com/pulumi/pulumi-kubernetes/vendor/github.com/pulumi/pulumi/sdk/proto/go._ResourceProvider_Create_Handler.func1(0x2506c00, 0xc0007d88d0, 0x21e3e00, 0xc0002ff200, 0x2221980, 0x3330858, 0x2506c00, 0xc0007d88d0)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/github.com/pulumi/pulumi/sdk/proto/go/provider.pb.go:1516 +0x86
    github.com/pulumi/pulumi-kubernetes/vendor/github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc.OpenTracingServerInterceptor.func1(0x2506c00, 0xc0007d8330, 0x21e3e00, 0xc0002ff200, 0xc0003003e0, 0xc000300400, 0x0, 0x0, 0x24ca8e0, 0xc0001851d0)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc/server.go:61 +0x360
    github.com/pulumi/pulumi-kubernetes/vendor/github.com/pulumi/pulumi/sdk/proto/go._ResourceProvider_Create_Handler(0x22306e0, 0xc000172070, 0x2506c00, 0xc0007d8330, 0xc0007d5630, 0xc00044c040, 0x2506c00, 0xc0007d8330, 0xc001328000, 0x1f1)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/github.com/pulumi/pulumi/sdk/proto/go/provider.pb.go:1518 +0x158
    github.com/pulumi/pulumi-kubernetes/vendor/google.golang.org/grpc.(*Server).processUnaryRPC(0xc000001c80, 0x2521760, 0xc0004b6780, 0xc00089c300, 0xc000316150, 0x33027d0, 0x0, 0x0, 0x0)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/google.golang.org/grpc/server.go:972 +0x470
    github.com/pulumi/pulumi-kubernetes/vendor/google.golang.org/grpc.(*Server).handleStream(0xc000001c80, 0x2521760, 0xc0004b6780, 0xc00089c300, 0x0)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/google.golang.org/grpc/server.go:1252 +0xda6
    github.com/pulumi/pulumi-kubernetes/vendor/google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc0004fe000, 0xc000001c80, 0x2521760, 0xc0004b6780, 0xc00089c300)
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/google.golang.org/grpc/server.go:691 +0x9f
    created by github.com/pulumi/pulumi-kubernetes/vendor/google.golang.org/grpc.(*Server).serveStreams.func1
    	/Users/alex/go/src/github.com/pulumi/pulumi-kubernetes/vendor/google.golang.org/grpc/server.go:689 +0xa1

    error: update failed

Resources:
    1 unchanged

Duration: 5s

[hausdorff]

In the near term we certainly shouldn't panic. I'll fix that. @lukehoban @pgavlin what are your thoughts on supporting this longer term? Worth doing? And if so, how do you envision this working?

[lukehoban]

What did you do to hit this?

[hausdorff]

@lukehoban sorry—you pass any Asset into any field in any Kubernetes resource definition. The result is a panic. This will be fixed today.

Also—why did we remove the P1? It seems like we should commit to fixing this ASAP.

[hausdorff]

Oh. Passing anything that's not JSON-compatible will actually cause a panic. Not just an Asset. The code is here, and a related issue is here. I wonder why they panic there instead of returning an error.

[hausdorff]

Alright, after careful thought, I think the only time we would encounter an error like this is when we pass an Asset/Archive to the provider.

To recap:

• We construct a Kubernetes *unstructured.Unstructured from the map[string]interface{} representation of a Kubernetes resource—this is provided to the provider from the Pulumi program.
• This apimachinery panic occurs this map[string]interface{} contains a value that is not one of its blessed JSON types.
• The only time a Pulumi program can pass a value that is not one of these blessed JSON types is when the value is either an Asset or an Archive (EDIT: or secrets; perhaps some other things). (Specifically, the value types possible as I understand them are: Bool, Number, String, Array, Asset, Archive, Object.)

Hence, merging this PR should ameliorate of of this issue in the short term. Questions/thoughts:

• Will we ever introduce new object types, and if so, how should we protect against this in the future?
• I'd prefer to not have to have validation code in the providers that crawls around in the property maps to detect anything that is not one of these "blessed" types—but even more than that I'd prefer that apimachinery simply not panic! :)

cc @lukehoban @pgavlin @lblackstone for visibility. We're going to merge the PR that we think addresses this issue tonight, but I will keep this open for another day or two just so it has time to get visibility, percolate, etc.

[hausdorff]

Talking a bit to @ellismg I believe the above is correct modulo the secrets—which he is handling.

[lukehoban]

Let's close this out if there's no known work remaining here. If we find new issues, we can open new issues to track.