Allow opt-out of adding stdout/stderr env variables for remote command (#451)

# Context

At the moment, stdout and stderr of previous runs are passed as
environment variables on future invocations. When stdout or stderr is
too large, this cause an error.

# Proposed change

#355 fixed it for local commands, this fixed it for remote commands. See
#285 for full details on the issue

See this comment for details around local vs remote functionality :
#285 (comment)

---------

Co-authored-by: Thomas Kappler <[email protected]>
Co-authored-by: Martin Lehmann <[email protected]>

Author: 3 people

provider/cmd/pulumi-resource-command/schema.json

@@ -368,6 +368,11 @@
     "command:remote:Command": {
       "description": "A command to run on a remote host.\nThe connection is established via ssh.",
       "properties": {
+        "addPreviousOutputInEnv": {
+          "type": "boolean",
+          "description": "If the previous command's stdout and stderr (as generated by the prior create/update) is\ninjected into the environment of the next run as PULUMI_COMMAND_STDOUT and PULUMI_COMMAND_STDERR.\nDefaults to true.",
+          "default": true
+        },
         "connection": {
           "$ref": "#/types/command:remote:Connection",
           "description": "The parameters with which to connect to the remote host.",
@@ -424,6 +429,11 @@
         "stdout"
       ],
       "inputProperties": {
+        "addPreviousOutputInEnv": {
+          "type": "boolean",
+          "description": "If the previous command's stdout and stderr (as generated by the prior create/update) is\ninjected into the environment of the next run as PULUMI_COMMAND_STDOUT and PULUMI_COMMAND_STDERR.\nDefaults to true.",
+          "default": true
+        },
         "connection": {
           "$ref": "#/types/command:remote:Connection",
           "description": "The parameters with which to connect to the remote host.",

provider/pkg/provider/remote/command.go

@@ -37,10 +37,11 @@ type CommandInputs struct {
 	// pulumi:"optional" specifies that a field is optional. This must be a pointer.
 	// provider:"replaceOnChanges" specifies that the resource will be replaced if the field changes.
 	// provider:"secret" specifies that a field should be marked secret.
-	Stdin       *string           `pulumi:"stdin,optional"`
-	Logging     *Logging          `pulumi:"logging,optional"`
-	Connection  *Connection       `pulumi:"connection" provider:"secret"`
-	Environment map[string]string `pulumi:"environment,optional"`
+	Stdin                  *string           `pulumi:"stdin,optional"`
+	Logging                *Logging          `pulumi:"logging,optional"`
+	Connection             *Connection       `pulumi:"connection" provider:"secret"`
+	Environment            map[string]string `pulumi:"environment,optional"`
+	AddPreviousOutputInEnv *bool             `pulumi:"addPreviousOutputInEnv,optional"`
 }
 
 // Implementing Annotate lets you provide descriptions and default values for arguments and they will
@@ -55,6 +56,11 @@ outputs as secret via 'additionalSecretOutputs'. Defaults to logging both stdout
 Note that this only works if the SSH server is configured to accept these variables via AcceptEnv.
 Alternatively, if a Bash-like shell runs the command on the remote host, you could prefix the command itself
 with the variables in the form 'VAR=value command'.`)
+	a.Describe(&c.AddPreviousOutputInEnv,
+		`If the previous command's stdout and stderr (as generated by the prior create/update) is
+injected into the environment of the next run as PULUMI_COMMAND_STDOUT and PULUMI_COMMAND_STDERR.
+Defaults to true.`)
+	a.SetDefault(&c.AddPreviousOutputInEnv, true)
 }
 
 // The properties for a remote Command resource.

provider/pkg/provider/remote/commandOutputs.go

@@ -48,23 +48,25 @@ func (c *CommandOutputs) run(ctx context.Context, cmd string, logging *Logging)
 		}
 	}
 
-	// Set remote Stdout and Stderr environment variables optimistically, but log and continue if they fail.
-	if c.Stdout != "" {
-		err := session.Setenv(util.PULUMI_COMMAND_STDOUT, c.Stdout)
-		if err != nil {
-			// Set remote Stdout var optimistically, but warn and continue on failure.
-			//
-			//nolint:errcheck
-			logAndWrapSetenvErr(diag.Warning, util.PULUMI_COMMAND_STDOUT, ctx, err)
+	if c.AddPreviousOutputInEnv == nil || *c.AddPreviousOutputInEnv {
+		// Set remote Stdout and Stderr environment variables optimistically, but log and continue if they fail.
+		if c.Stdout != "" {
+			err := session.Setenv(util.PULUMI_COMMAND_STDOUT, c.Stdout)
+			if err != nil {
+				// Set remote Stdout var optimistically, but warn and continue on failure.
+				//
+				//nolint:errcheck
+				logAndWrapSetenvErr(diag.Warning, util.PULUMI_COMMAND_STDOUT, ctx, err)
+			}
 		}
-	}
-	if c.Stderr != "" {
-		err := session.Setenv(util.PULUMI_COMMAND_STDERR, c.Stderr)
-		if err != nil {
-			// Set remote STDERR var optimistically, but warn and continue on failure.
-			//
-			//nolint:errcheck
-			logAndWrapSetenvErr(diag.Warning, util.PULUMI_COMMAND_STDERR, ctx, err)
+		if c.Stderr != "" {
+			err := session.Setenv(util.PULUMI_COMMAND_STDERR, c.Stderr)
+			if err != nil {
+				// Set remote STDERR var optimistically, but warn and continue on failure.
+				//
+				//nolint:errcheck
+				logAndWrapSetenvErr(diag.Warning, util.PULUMI_COMMAND_STDERR, ctx, err)
+			}
 		}
 	}
 

provider/pkg/provider/util/testutil/test_ssh_server.go

@@ -0,0 +1,44 @@
+package testutil
+
+import (
+	"fmt"
+	"net"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/gliderlabs/ssh"
+	"github.com/stretchr/testify/require"
+)
+
+type TestSshServer struct {
+	Host string
+	Port int64
+}
+
+// NewTestSshServer creates a new in-process SSH server with the specified handler.
+// The server is bound to an arbitrary free port, and automatically closed
+// during test cleanup.
+func NewTestSshServer(t *testing.T, handler ssh.Handler) TestSshServer {
+	const host = "127.0.0.1"
+
+	listener, err := net.Listen("tcp", fmt.Sprintf("%s:%d", host, 0))
+	require.NoErrorf(t, err, "net.Listen()")
+
+	port, err := strconv.ParseInt(strings.Split(listener.Addr().String(), ":")[1], 10, 64)
+	require.NoErrorf(t, err, "parse address %s allocated port number as int", listener.Addr())
+
+	server := ssh.Server{Handler: handler}
+	go func() {
+		// "Serve always returns a non-nil error."
+		_ = server.Serve(listener)
+	}()
+	t.Cleanup(func() {
+		_ = server.Close()
+	})
+
+	return TestSshServer{
+		Host: host,
+		Port: port,
+	}
+}

provider/tests/go.mod

@@ -6,6 +6,7 @@ replace github.com/pulumi/pulumi-command/provider => ../
 
 require (
 	github.com/blang/semver v3.5.1+incompatible
+	github.com/gliderlabs/ssh v0.3.7
 	github.com/pulumi/pulumi-command/provider v0.0.0-00010101000000-000000000000
 	github.com/pulumi/pulumi-go-provider v0.17.0
 	github.com/pulumi/pulumi/sdk/v3 v3.117.0
@@ -18,6 +19,7 @@ require (
 	github.com/ProtonMail/go-crypto v1.0.0 // indirect
 	github.com/aead/chacha20 v0.0.0-20180709150244-8b13a72661da // indirect
 	github.com/agext/levenshtein v1.2.3 // indirect
+	github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be // indirect
 	github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect
 	github.com/atotto/clipboard v0.1.4 // indirect
 	github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect

provider/tests/provider_test.go

@@ -2,6 +2,8 @@ package tests
 
 import (
 	"fmt"
+	"github.com/gliderlabs/ssh"
+	"strings"
 	"testing"
 
 	"github.com/blang/semver"
@@ -13,6 +15,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	command "github.com/pulumi/pulumi-command/provider/pkg/provider"
+	"github.com/pulumi/pulumi-command/provider/pkg/provider/util/testutil"
 	"github.com/pulumi/pulumi-command/provider/pkg/version"
 )
 
@@ -223,6 +226,104 @@ func TestRemoteCommand(t *testing.T) {
 	})
 }
 
+func TestRemoteCommandStdoutStderrFlag(t *testing.T) {
+	// Start a local SSH server that writes the PULUMI_COMMAND_STDOUT environment variable
+	// on the format "PULUMI_COMMAND_STDOUT=<value>" to the client using stdout.
+	const (
+		createCommand = "arbitrary create command"
+	)
+
+	sshServer := testutil.NewTestSshServer(t, func(session ssh.Session) {
+		// Find the PULUMI_COMMAND_STDOUT environment variable
+		var envVar string
+		for _, v := range session.Environ() {
+			if strings.HasPrefix(v, "PULUMI_COMMAND_STDOUT=") {
+				envVar = v
+				break
+			}
+		}
+
+		response := fmt.Sprintf("Response{%s}", envVar)
+		_, err := session.Write([]byte(response))
+		require.NoErrorf(t, err, "session.Write(%s)", response)
+	})
+
+	cmd := provider()
+	urn := urn("remote", "Command", "dial")
+
+	// Run a create against an in-memory provider, assert it succeeded, and return the created property map.
+	connection := resource.NewObjectProperty(resource.PropertyMap{
+		"host":           resource.NewStringProperty(sshServer.Host),
+		"port":           resource.NewNumberProperty(float64(sshServer.Port)),
+		"user":           resource.NewStringProperty("arbitrary-user"), // unused but prevents nil panic
+		"perDialTimeout": resource.NewNumberProperty(1),                // unused but prevents nil panic
+	})
+
+	// The state that we expect a non-preview create to return.
+	//
+	// We use this as the final expect for create and the old state during update.
+	initialState := resource.PropertyMap{
+		"connection":             connection,
+		"create":                 resource.PropertyValue{V: createCommand},
+		"stderr":                 resource.PropertyValue{V: ""},
+		"stdout":                 resource.PropertyValue{V: "Response{}"},
+		"addPreviousOutputInEnv": resource.NewBoolProperty(true),
+	}
+
+	t.Run("create", func(t *testing.T) {
+		createResponse, err := cmd.Create(p.CreateRequest{
+			Urn: urn,
+			Properties: resource.PropertyMap{
+				"connection":             connection,
+				"create":                 resource.NewStringProperty(createCommand),
+				"addPreviousOutputInEnv": resource.NewBoolProperty(true),
+			},
+		})
+		require.NoError(t, err)
+		require.Equal(t, initialState, createResponse.Properties)
+	})
+
+	// Run an update against an in-memory provider, assert it succeeded, and return
+	// the new property map.
+	update := func(addPreviousOutputInEnv bool) resource.PropertyMap {
+		resp, err := cmd.Update(p.UpdateRequest{
+			ID:   "echo1234",
+			Urn:  urn,
+			Olds: initialState.Copy(),
+			News: resource.PropertyMap{
+				"connection":             connection,
+				"create":                 resource.NewStringProperty(createCommand),
+				"addPreviousOutputInEnv": resource.NewBoolProperty(addPreviousOutputInEnv),
+			},
+		})
+		require.NoError(t, err)
+		return resp.Properties
+	}
+
+	t.Run("update-actual-with-std", func(t *testing.T) {
+		assert.Equal(t, resource.PropertyMap{
+			"connection": connection,
+			"create":     resource.PropertyValue{V: createCommand},
+			"stderr":     resource.PropertyValue{V: ""},
+			// Running with addPreviousOutputInEnv=true sets the environment variable:
+			"stdout":                 resource.PropertyValue{V: "Response{PULUMI_COMMAND_STDOUT=Response{}}"},
+			"addPreviousOutputInEnv": resource.PropertyValue{V: true},
+		}, update(true))
+	})
+
+	t.Run("update-actual-without-std", func(t *testing.T) {
+		assert.Equal(t, resource.PropertyMap{
+			"connection": connection,
+			"create":     resource.PropertyValue{V: createCommand},
+			"stderr":     resource.PropertyValue{V: ""},
+			// Running without addPreviousOutputInEnv does not set the environment variable:
+			"stdout":                 resource.PropertyValue{V: "Response{}"},
+			"addPreviousOutputInEnv": resource.PropertyValue{V: false},
+		}, update(false))
+	})
+
+}
+
 // Ensure that we correctly apply defaults to `connection.port`.
 //
 // User issue is https://github.com/pulumi/pulumi-command/issues/248.
@@ -251,6 +352,7 @@ func TestRegress248(t *testing.T) {
 			"dialErrorLimit": pNumber(10),
 			"perDialTimeout": pNumber(15),
 		}),
+		"addPreviousOutputInEnv": resource.NewBoolProperty(true),
 	}, resp.Inputs)
 }
 

sdk/dotnet/Remote/Command.cs

@@ -16,6 +16,14 @@ namespace Pulumi.Command.Remote
     [CommandResourceType("command:remote:Command")]
     public partial class Command : global::Pulumi.CustomResource
     {
+        /// <summary>
+        /// If the previous command's stdout and stderr (as generated by the prior create/update) is
+        /// injected into the environment of the next run as PULUMI_COMMAND_STDOUT and PULUMI_COMMAND_STDERR.
+        /// Defaults to true.
+        /// </summary>
+        [Output("addPreviousOutputInEnv")]
+        public Output<bool?> AddPreviousOutputInEnv { get; private set; } = null!;
+
         /// <summary>
         /// The parameters with which to connect to the remote host.
         /// </summary>
@@ -139,6 +147,14 @@ public static Command Get(string name, Input<string> id, CustomResourceOptions?
 
     public sealed class CommandArgs : global::Pulumi.ResourceArgs
     {
+        /// <summary>
+        /// If the previous command's stdout and stderr (as generated by the prior create/update) is
+        /// injected into the environment of the next run as PULUMI_COMMAND_STDOUT and PULUMI_COMMAND_STDERR.
+        /// Defaults to true.
+        /// </summary>
+        [Input("addPreviousOutputInEnv")]
+        public Input<bool>? AddPreviousOutputInEnv { get; set; }
+
         [Input("connection", required: true)]
         private Input<Inputs.ConnectionArgs>? _connection;
 
@@ -221,6 +237,7 @@ public InputList<object> Triggers
 
         public CommandArgs()
         {
+            AddPreviousOutputInEnv = true;
         }
         public static new CommandArgs Empty => new CommandArgs();
     }