Fix crashes around secrets with 'undefined' value. (#3069)

CHANGELOG.md

@@ -17,6 +17,9 @@ CHANGELOG
 - Fix a bug in the Python SDK that caused crashes when using asynchronous data sources.
   [#3056](https://github.com/pulumi/pulumi/pull/3056)
 
+- Fix crash when exporting secrets from a pulumi app
+  [#2962](https://github.com/pulumi/pulumi/issues/2962)
+
 ## 0.17.28 (2019-08-05)
 
 - Retry renaming a temporary folder during plugin installation

sdk/nodejs/runtime/rpc.ts

@@ -291,13 +291,15 @@ export async function serializeProperty(ctx: string, prop: Input<any>, dependent
         // so we must compare to the literal true instead of just doing await prop.isSecret.
         const isSecret = await prop.isSecret === true;
         const value = await serializeProperty(`${ctx}.id`, prop.promise(), dependentResources);
+
         if (!isKnown) {
             return unknownValue;
         }
         if (isSecret && await monitorSupportsSecrets()) {
             return {
                 [specialSigKey]: specialSecretSig,
-                value: value,
+                // coerce 'undefined' to 'null' as required by the protobuf system.
+                value: value === undefined ? null : value,
             };
         }
         return value;

sdk/nodejs/runtime/stack.ts

@@ -96,17 +96,7 @@ async function massage(prop: any, seenObjects: Set<any>): Promise<any> {
     }
 
     if (Output.isInstance(prop)) {
-        // If the output itself is a secret, we don't want to lose the secretness by returning the underlying
-        // value. So instead, we massage the underlying value and then wrap it back up in an Output which is
-        // marked as secret.
-        const isSecret = await (prop.isSecret || Promise.resolve(false));
-        const value = await massage(await prop.promise(), seenObjects);
-
-        if (isSecret) {
-            return secret(value);
-        }
-
-        return value;
+        return prop.apply(v => massage(v, seenObjects));
     }
 
     // from this point on, we have complex objects.  If we see them again, we don't want to emit

sdk/nodejs/tests/runtime/props.spec.ts

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 import * as assert from "assert";
-import { Inputs, runtime } from "../../index";
+import { Inputs, runtime, secret } from "../../index";
 import { asyncTest } from "../util";
 
 
@@ -41,6 +41,20 @@ describe("runtime", () => {
             assert.equal(result.id, "foo");
             assert.equal(result.urn, "bar");
         }));
+        it("marshals secrets correctly", asyncTest(async () => {
+            runtime._setTestModeEnabled(true);
+            const inputs: Inputs = {
+                "secret1": secret(1),
+                "secret2": secret(undefined),
+            };
+            // Serialize and then deserialize all the properties, checking that they round-trip as expected.
+            const transfer = gstruct.Struct.fromJavaScript(
+                await runtime.serializeProperties("test", inputs));
+            const result = runtime.deserializeProperties(transfer);
+            assert.equal(result.secret1, 1);
+            assert.equal(result.secret2, undefined);
+            runtime._setTestModeEnabled(false);
+        }));
     });
 
     describe("deserializeProperty", () => {