Match rpc behavior (#228)

While working on
pulumi/pulumi-pulumiservice#258, I noticed that
native providers receive unknown values over the wire regardless of
configure options. This reflects that in the `rpc` middleware.

internal/key/key.go

@@ -29,3 +29,14 @@ var (
 	Logger      = logType{}
 	URN         = urnType{}
 )
+
+// ForceNoDetailedDiff acts as a side-channel in
+// [github.com/pulumi/pulumi-go-provider.DiffResponse].DetailedDiff to set HasDetailedDiff
+// to false.
+//
+// This is necessary for the
+// [github.com/pulumi/pulumi-go-provider/middleware/rpc.Provider], but should not be used
+// by outside providers, as they should support detailed diff in all cases.
+//
+// The key should never be exposed at the gRPC wire level.
+const ForceNoDetailedDiff = "__x-force-no-detailed-diff"

middleware/rpc/provider.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 
 	structpb "github.com/golang/protobuf/ptypes/struct"
+	"github.com/pulumi/pulumi-go-provider/internal/key"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
 	rpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
@@ -298,6 +299,8 @@ func diffResponse(resp *rpc.DiffResponse, err error) (p.DiffResponse, error) {
 		for _, replace := range resp.GetReplaces() {
 			detailedDiff[replace] = p.PropertyDiff{Kind: p.UpdateReplace}
 		}
+		detailedDiff[key.ForceNoDetailedDiff] = p.PropertyDiff{}
+
 	}
 	if len(detailedDiff) == 0 {
 		detailedDiff = nil
@@ -332,7 +335,7 @@ func (r runtime) propertyToRPC(m resource.PropertyMap) (*structpb.Struct, error)
 		r.configuration = &rpc.ConfigureResponse{}
 	}
 	s, err := plugin.MarshalProperties(m, plugin.MarshalOptions{
-		KeepUnknowns:     r.configuration.SupportsPreview,
+		KeepUnknowns:     true,
 		KeepSecrets:      r.configuration.AcceptSecrets,
 		KeepResources:    r.configuration.AcceptResources,
 		KeepOutputValues: r.configuration.AcceptOutputs,

provider.go

@@ -146,15 +146,28 @@ type DiffResponse struct {
 	DetailedDiff map[string]PropertyDiff
 }
 
+type diffChanges bool
+
+func (c diffChanges) rpc() rpc.DiffResponse_DiffChanges {
+	if c {
+		return rpc.DiffResponse_DIFF_SOME
+	}
+	return rpc.DiffResponse_DIFF_NONE
+}
+
 func (d DiffResponse) rpc() *rpc.DiffResponse {
+
+	hasDetailedDiff := true
+	if _, ok := d.DetailedDiff[key.ForceNoDetailedDiff]; ok {
+		hasDetailedDiff = false
+		delete(d.DetailedDiff, key.ForceNoDetailedDiff)
+	}
+
 	r := rpc.DiffResponse{
 		DeleteBeforeReplace: d.DeleteBeforeReplace,
-		Changes:             rpc.DiffResponse_DIFF_NONE,
+		Changes:             diffChanges(d.HasChanges).rpc(),
 		DetailedDiff:        detailedDiff(d.DetailedDiff).rpc(),
-		HasDetailedDiff:     true,
-	}
-	if d.HasChanges {
-		r.Changes = rpc.DiffResponse_DIFF_SOME
+		HasDetailedDiff:     hasDetailedDiff,
 	}
 	for k, v := range d.DetailedDiff {
 		switch v.Kind {

tests/grpc/wrap_rpc_test.go

@@ -0,0 +1,91 @@
+// Copyright 2024, Pulumi Corporation.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package grpc
+
+import (
+	"context"
+	"testing"
+
+	replay "github.com/pulumi/providertest/replay"
+	pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
+	"github.com/stretchr/testify/require"
+
+	p "github.com/pulumi/pulumi-go-provider"
+	"github.com/pulumi/pulumi-go-provider/middleware/rpc"
+)
+
+type rawRPCProvider struct {
+	pulumirpc.UnimplementedResourceProviderServer
+
+	diff func(context.Context, *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error)
+}
+
+func (r rawRPCProvider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error) {
+	return r.diff(ctx, req)
+}
+
+func wrapRPCProvider(t *testing.T, provider rawRPCProvider) pulumirpc.ResourceProviderServer {
+	s, err := p.RawServer("test", "1.0.0", rpc.Provider(provider))(nil)
+	require.NoError(t, err)
+	return s
+}
+
+// TestWrapRPCEmptyDiff ensures that hasDetailedDiff is correctly set even when there are
+// no listed changes.
+func TestWrapRPCEmptyDiff(t *testing.T) {
+	t.Run("no-detailed-diff", func(t *testing.T) {
+		replay.Replay(t, wrapRPCProvider(t, rawRPCProvider{
+			diff: func(context.Context, *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error) {
+				return &pulumirpc.DiffResponse{
+					Changes:         pulumirpc.DiffResponse_DIFF_SOME,
+					HasDetailedDiff: false,
+				}, nil
+			},
+		}), `{
+  "method": "/pulumirpc.ResourceProvider/Diff",
+  "request": {
+    "id": "vpc-4b82e033",
+    "urn": "urn:pulumi:testtags::tags-combinations-go::aws:ec2/defaultVpc:DefaultVpc::go-web-default-vpc",
+    "olds": {},
+    "news": {},
+    "oldInputs": {}
+  },
+  "response": {"changes": "DIFF_SOME"}
+}`)
+	})
+	t.Run("has-detailed-diff", func(t *testing.T) {
+		replay.Replay(t, wrapRPCProvider(t, rawRPCProvider{
+			diff: func(context.Context, *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error) {
+				return &pulumirpc.DiffResponse{
+					Changes:         pulumirpc.DiffResponse_DIFF_SOME,
+					HasDetailedDiff: true,
+				}, nil
+			},
+		}), `{
+  "method": "/pulumirpc.ResourceProvider/Diff",
+  "request": {
+    "id": "vpc-4b82e033",
+    "urn": "urn:pulumi:testtags::tags-combinations-go::aws:ec2/defaultVpc:DefaultVpc::go-web-default-vpc",
+    "olds": {},
+    "news": {},
+    "oldInputs": {}
+  },
+  "response": {
+    "changes": "DIFF_SOME",
+    "hasDetailedDiff": true
+  }
+}`)
+	})
+}

tests/rpc_test.go

@@ -230,10 +230,19 @@ func TestRPCConfigure(t *testing.T) {
 									Known:   true,
 									Element: resource.NewProperty("v1"),
 								}),
+								"unknown": resource.MakeComputed(
+									resource.NewProperty(""),
+								),
 							}, m)
 						} else {
 							assert.Equal(t, resource.PropertyMap{
 								"known": resource.NewProperty("v1"),
+								"output": resource.MakeComputed(
+									resource.NewProperty(""),
+								),
+								"unknown": resource.MakeComputed(
+									resource.NewProperty(""),
+								),
 							}, m)
 						}
 
@@ -737,6 +746,14 @@ func testRPCCheck(
 	})
 }
 
+func noDetailedDiff(m map[string]p.PropertyDiff) map[string]p.PropertyDiff {
+	if m == nil {
+		m = map[string]p.PropertyDiff{}
+	}
+	m["__x-force-no-detailed-diff"] = p.PropertyDiff{}
+	return m
+}
+
 // testRPCDiff tests a check function against a series of inputs.
 func testRPCDiff(
 	t *testing.T,
@@ -769,6 +786,7 @@ func testRPCDiff(
 		require.NoError(t, err)
 		assert.Equal(t, p.DiffResponse{
 			DeleteBeforeReplace: true,
+			DetailedDiff:        noDetailedDiff(nil),
 		}, resp)
 	})
 
@@ -818,7 +836,8 @@ func testRPCDiff(
 
 		require.NoError(t, err)
 		assert.Equal(t, p.DiffResponse{
-			HasChanges: false,
+			HasChanges:   false,
+			DetailedDiff: noDetailedDiff(nil),
 		}, resp)
 
 	})
@@ -838,11 +857,11 @@ func testRPCDiff(
 		require.NoError(t, err)
 		assert.Equal(t, p.DiffResponse{
 			HasChanges: true,
-			DetailedDiff: map[string]p.PropertyDiff{
+			DetailedDiff: noDetailedDiff(map[string]p.PropertyDiff{
 				"r1": {Kind: p.UpdateReplace},
 				"r2": {Kind: p.UpdateReplace},
 				"f3": {Kind: p.Update},
-			},
+			}),
 		}, resp)
 	})
 	t.Run("error", func(t *testing.T) {