Fix accept secrets=false to configure provider with nested secrets (#641

)

Fixes #640 panic when passing nested secret values to provider config

Our current design for handling program-originating secrets in bridged providers is to basically let the engine (Pulumi CLI) handle it for us. Bridged providers return "acceptsSecrets=false" to the engine, which then does not pass secret bits to Configure or Create but instead passes plain data and does some heuristic matching on the resource/provider outputs to make sure secret data stays secret.

Unfortunately due to how Configure was written in dockerHybridProvider struct here which multiplexes traffic between bridged and native providers, this information got lost and the docker provider returned acceptsSecrets=true to the engine, while not protecting the bridged provider from unexpected secrets. This caused a panic deep in the bridged provider when a secret was passed.

The fix is to ensure the hybrid provider returns the same options as the bridgedProvider, with a notable exception of supportsPreview - which currently is supported in the bridged but not the native provider, and has to be disabled when mixing them into the hybrid provider.

examples/examples_dotnet_test.go

@@ -124,6 +124,15 @@ func TestDockerContainerRegistryDotnet(t *testing.T) {
 	integration.ProgramTest(t, &test)
 }
 
+func TestSecretsInExplicitProvider(t *testing.T) {
+	test := getCsharpBaseOptions(t).With(integration.ProgramTestOptions{
+		Dir:         path.Join(getCwd(t), "test-secrets-in-explicit-provider", "csharp"),
+		Quick:       true,
+		SkipRefresh: true,
+	})
+	integration.ProgramTest(t, &test)
+}
+
 func getCsharpBaseOptions(t *testing.T) integration.ProgramTestOptions {
 	base := getBaseOptions()
 	baseCsharp := base.With(integration.ProgramTestOptions{

examples/examples_yaml_test.go

@@ -1,3 +1,4 @@
+// Copyright 2016-2023, Pulumi Corporation.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -16,11 +17,14 @@
 package examples
 
 import (
-	"github.com/pulumi/pulumi/pkg/v3/testing/integration"
-	"github.com/stretchr/testify/assert"
+	"encoding/json"
 	"os"
 	"path"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/pulumi/pulumi/pkg/v3/testing/integration"
 )
 
 func TestUnknownInputsYAML(t *testing.T) {
@@ -54,8 +58,13 @@ func TestSecretsYAML(t *testing.T) {
 			imgName, ok := stack.Outputs["imageName"]
 			assert.True(t, ok)
 			assert.NotEmpty(t, imgName)
-			assert.Equal(t, "pulumibot/test-secrets:yaml", imgName)
 
+			// imgName is going to be a secret value encoded as a map. Currently ProgramTest lacks the
+			// capacity to decrypt it and check the contents. For now we simply ensure that the secret
+			// information is not present plaintext in the JSON expansion of this value.
+			imgNameStr, err := json.Marshal(imgName)
+			assert.NoError(t, err)
+			assert.NotContains(t, imgNameStr, "pulumibot/test-secrets:yaml")
 		},
 	})
 }

examples/test-secrets-in-explicit-provider/csharp/Program.cs

@@ -0,0 +1,24 @@
+using Pulumi;
+using System.Collections.Generic;
+
+return await Deployment.RunAsync(() =>
+{
+   var provider = new Pulumi.Docker.Provider("docker", new Pulumi.Docker.ProviderArgs
+   {
+      Host = "host",
+      RegistryAuth = new List<Pulumi.Docker.Inputs.ProviderRegistryAuthArgs>
+      {
+         new Pulumi.Docker.Inputs.ProviderRegistryAuthArgs
+         {
+            Address = "somewhere.org",
+            Username = "some-user",
+            Password = "some-password"
+         }
+      }
+   });
+
+   return new Dictionary<string, object?>
+   {
+      ["outputKey"] = "outputValue"
+   };
+});

examples/test-secrets-in-explicit-provider/csharp/Pulumi.yaml

@@ -0,0 +1,3 @@
+name: docker-640
+runtime: dotnet
+description: A minimal C# Pulumi program reproducing pulumi/pulumi-docker/issues/640

examples/test-secrets-in-explicit-provider/csharp/docker-640.csproj

@@ -0,0 +1,14 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>net6.0</TargetFramework>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Pulumi" Version="3.*" />
+    <PackageReference Include="Pulumi.Docker" Version="4.*" />
+  </ItemGroup>
+
+</Project>

provider/hybrid.go

@@ -3,8 +3,10 @@ package provider
 import (
 	"context"
 	"fmt"
+
 	"github.com/golang/protobuf/ptypes/empty"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/util/logging"
 	rpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
 	"google.golang.org/grpc/codes"
@@ -66,17 +68,30 @@ func (dp dockerHybridProvider) DiffConfig(ctx context.Context, request *rpc.Diff
 	}, nil
 }
 
-func (dp dockerHybridProvider) Configure(ctx context.Context, request *rpc.ConfigureRequest) (
-	*rpc.ConfigureResponse, error) {
-	var myResp *rpc.ConfigureResponse
-	for _, prov := range []rpc.ResourceProviderServer{dp.bridgedProvider, dp.nativeProvider} {
-		resp, err := prov.Configure(ctx, request)
-		if err != nil {
-			return nil, err
-		}
-		myResp = resp
+func (dp dockerHybridProvider) Configure(
+	ctx context.Context,
+	request *rpc.ConfigureRequest,
+) (*rpc.ConfigureResponse, error) {
+	// Native provider returns empty response and error from Configure, just call it to propagate the information.
+	r, err := dp.nativeProvider.Configure(ctx, request)
+	if err != nil {
+		return nil, fmt.Errorf("Docker native provider returned an unexpected error from Configure: %w", err)
 	}
-	return myResp, nil
+
+	contract.Assertf(!r.AcceptOutputs, "Unexpected AcceptOutputs=true from Docker native provider Configure")
+	contract.Assertf(!r.AcceptResources, "Unexpected AcceptResources=true from Docker native provider Configure")
+	contract.Assertf(!r.AcceptSecrets, "Unexpected AcceptSecrets=true from Docker native provider Configure")
+	contract.Assertf(!r.SupportsPreview, "Unexpected SupportsPreview=true from Docker native provider Configure")
+
+	// For the most part delegate Configure handling to the bridged provider.
+	resp, err := dp.bridgedProvider.Configure(ctx, request)
+
+	// With one important exception: the hybrid provider cannot support preview because Create on the native
+	// provider is not ready to accept partial data with unknowns when called in preview mode. This limits the
+	// ability of the bridged provider to do best-effort processing and validation in preview. An alternate design
+	// would return SupportsPreview=true here but shield the native provider from it.
+	resp.SupportsPreview = false
+	return resp, err
 }
 
 func (dp dockerHybridProvider) Invoke(ctx context.Context, request *rpc.InvokeRequest) (*rpc.InvokeResponse, error) {

provider/provider.go

@@ -21,6 +21,7 @@ import (
 	"github.com/pulumi/pulumi/pkg/v3/resource/provider"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/util/logging"
 	rpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
 	"google.golang.org/grpc/codes"
@@ -77,9 +78,7 @@ func (p *dockerNativeProvider) Configure(_ context.Context, req *rpc.ConfigureRe
 	for key, val := range req.GetVariables() {
 		p.config[strings.TrimPrefix(key, "docker:config:")] = val
 	}
-	return &rpc.ConfigureResponse{
-		AcceptSecrets: true,
-	}, nil
+	return &rpc.ConfigureResponse{}, nil
 }
 
 // Invoke dynamically executes a built-in function in the provider.
@@ -288,6 +287,10 @@ func diffUpdates(updates map[resource.PropertyKey]resource.ValueDiff) map[string
 
 // Create allocates a new instance of the provided resource and returns its unique ID afterwards.
 func (p *dockerNativeProvider) Create(ctx context.Context, req *rpc.CreateRequest) (*rpc.CreateResponse, error) {
+	contract.Assertf(!req.GetPreview(), "Internal error in pulumi-docker: "+
+		"dockerNativeProvider Create should not be called during preview "+
+		"as it currently does not support partial data or recognizing unknowns.")
+
 	urn := resource.URN(req.GetUrn())
 	label := fmt.Sprintf("%s.Create(%s)", p.name, urn)
 	logging.V(9).Infof("%s executing", label)