From 7175ccdec9d505ff94c25aed91d489fa5582ef07 Mon Sep 17 00:00:00 2001
From: Ian Wahbe <ian@wahbe.com>
Date: Fri, 28 Oct 2022 15:13:22 -0700
Subject: [PATCH] Make IsSecret threadsafe

---
 ...ck-issecret-until-secretness-is-known.yaml |  4 +++
 sdk/go/pulumi/types.go                        | 27 ++++++++++++++++++-
 sdk/go/pulumi/types_test.go                   | 19 +++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 changelog/pending/20221028--sdk-go-yaml--block-issecret-until-secretness-is-known.yaml

diff --git a/changelog/pending/20221028--sdk-go-yaml--block-issecret-until-secretness-is-known.yaml b/changelog/pending/20221028--sdk-go-yaml--block-issecret-until-secretness-is-known.yaml
new file mode 100644
index 000000000000..5a6b9437d934
--- /dev/null
+++ b/changelog/pending/20221028--sdk-go-yaml--block-issecret-until-secretness-is-known.yaml
@@ -0,0 +1,4 @@
+changes:
+- type: fix
+  scope: sdk/go,yaml
+  description: Block IsSecret until secretness is known
diff --git a/sdk/go/pulumi/types.go b/sdk/go/pulumi/types.go
index 19b4157ec35f..4c4b7533a88e 100644
--- a/sdk/go/pulumi/types.go
+++ b/sdk/go/pulumi/types.go
@@ -519,8 +519,33 @@ func (o *OutputState) ApplyTWithContext(ctx context.Context, applier interface{}
 }
 
 // IsSecret returns a bool representing the secretness of the Output
+//
+// IsSecret may return an inaccurate results if the Output is unknowable (during a
+// preview) or contains an error.
 func IsSecret(o Output) bool {
-	return o.getState().secret
+	_, _, secret, _, _ := o.getState().await(context.Background())
+	// We intentionally ignore both the `known` and `error` values returned by `await`:
+	//
+	// If a value is not known, it is possible that we will return the wrong result. This
+	// is unavoidable. Consider the example:
+	//
+	// ```go
+	// bucket, _ := s3.Bucket("bucket", &s3.BucketArgs{})
+	// unknowable := bucket.Bucket.ApplyT(func(b string) OutputString {
+	//   if strings.ContainsRune(b, '9') {
+	//     return ToSecret(String(b))
+	//   else {
+	//     return String(b)
+	//   }
+	// })
+	// ```
+	//
+	// Until we resolve values from the cloud, we can't know the correct value of
+	// `IsSecret(unknowable)`. We have the same problem for outputs with non-nil errors.
+	//
+	// This is tolerable because users will never be able to retrieve values (secret or
+	// otherwise) that are unknown or erred.
+	return secret
 }
 
 // Unsecret will unwrap a secret output as a new output with a resolved value and no secretness
diff --git a/sdk/go/pulumi/types_test.go b/sdk/go/pulumi/types_test.go
index 1114c53504ee..68db440891f9 100644
--- a/sdk/go/pulumi/types_test.go
+++ b/sdk/go/pulumi/types_test.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -618,7 +619,25 @@ func TestSecretApply(t *testing.T) {
 			break
 		}
 	}
+}
+
+// Test that secretness is properly bubbled up with all/apply that delays its execution.
+func TestSecretApplyDelayed(t *testing.T) {
+	t.Parallel()
 
+	// We run multiple tests here to increase the likelihood of a hypothetical race
+	// condition triggering. As with all concurrency tests, its not a 100% guarantee.
+	for i := 0; i < 10 && !t.Failed(); i++ {
+		t.Run("", func(t *testing.T) {
+			t.Parallel()
+			s1 := String("foo").ToStringOutput().ApplyT(func(s string) StringOutput {
+				time.Sleep(time.Millisecond * 5)
+				return ToSecret(String("bar")).(StringOutput)
+			})
+			// assert that s1 is secret.
+			assert.True(t, IsSecret(s1))
+		})
+	}
 }
 
 func TestNil(t *testing.T) {